Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 28 Oct 2014 11:46:49 -0400
From:      John Baldwin <jhb@freebsd.org>
To:        Konstantin Belousov <kostikbel@gmail.com>
Cc:        freebsd-arch@freebsd.org
Subject:   Re: RfC: fueword(9) and casueword(9)
Message-ID:  <201410281146.49370.jhb@freebsd.org>
In-Reply-To: <20141027165557.GC1877@kib.kiev.ua>
References:  <20141021094539.GA1877@kib.kiev.ua> <2048849.GkvWliFbyg@ralph.baldwin.cx> <20141027165557.GC1877@kib.kiev.ua>

next in thread | previous in thread | raw e-mail | index | archive | help
On Monday, October 27, 2014 12:55:57 pm Konstantin Belousov wrote:
> On Mon, Oct 27, 2014 at 11:17:51AM -0400, John Baldwin wrote:
> > On Tuesday, October 21, 2014 07:23:06 PM Konstantin Belousov wrote:
> > > On Wed, Oct 22, 2014 at 01:41:12AM +1100, Bruce Evans wrote:
> > > > A new API should try to fix these __DEVOLATILE() abominations.  I think it
> > > > is safe, and even correct, to declare the pointers as volatile const void
> > > > *, since the functions really can handle volatile data, unlike copyin().
> > > > 
> > > > Atomic op functions are declared as taking pointers to volatile for
> > > > similar reasons.  Often they are applied to non-volatile data, but
> > > > adding a qualifier is type-safe and doesn't cost efficiency since the
> > > > pointer access is is not known to the compiler.  (The last point is not
> > > > so clear -- the compiler can see things in the functions since they are
> > > > inline asm.  fueword() isn't inline so its (in)efficiency is not changed.)
> > > > 
> > > > The atomic read functions are not declared as taking pointers to const.
> > > > The __DECONST() abomination might be used to work around this bug.
> > > 
> > > I prefer to not complicate the fetch(9) KPI due to the mistakes in the
> > > umtx structures definitions.  I think that it is bug to mark the lock
> > > words with volatile.  I want the fueword(9) interface to be as much
> > > similar to fuword(9), in particular, volatile seems to be not needed.
> > 
> > I agree with Bruce here.  casuword() already accepts volatile.  I also
> > think umtx is correct in marking the field as volatile.  They are subject
> > to change without the compiler's knowledge albeit by other threads
> > rather than signal handlers.  Having them marked volatile doesn't really
> > matter for the kernel, but the header is also used in userland and is
> > relevant in sem_new.c, etc.
> 
> You agree with making fueword() accept volatile const void * as the
> address ?  Or do you agree with the existence of the volatile type
> qualifier for the lock field of umtx structures ?

I agree with both (I thought Bruce only asserted the first).

> I definitely do not want to make fueword() different from fuword() in
> this aspect.  If changing both fueword() and fuword() to take volatile
> const * address, this should be different patch.

I also agree that fuword() and fueword() should take identical arguments,
so if this change is made it should be a separate patch (and should include
suword()).

-- 
John Baldwin



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201410281146.49370.jhb>