About the memory barrier in BSD libc

[Fengwei yin]

On Wed, Apr 25, 2012 at 10:12 PM, John Baldwin <jhb at freebsd.org> wrote:
> On Wednesday, April 25, 2012 3:05:54 am Fengwei yin wrote:
>> On Wed, Apr 25, 2012 at 2:26 PM, Konstantin Belousov
>> <kostikbel at gmail.com> wrote:
>> > On Wed, Apr 25, 2012 at 11:25:40AM +0800, Fengwei yin wrote:
>> >> On Wed, Apr 25, 2012 at 2:13 AM, Konstantin Belousov
>> >> <kostikbel at gmail.com> wrote:
>> >> > On Tue, Apr 24, 2012 at 07:00:33PM +0100, Martin Simmons wrote:
>> >> >> >>>>> On Tue, 24 Apr 2012 19:58:42 +0300, Konstantin Belousov said:
>> >> >> >
>> >> >> > +
>> >> >> > +   /*
>> >> >> > +    * Lock the spinlock used to protect __sglue list walk in
>> >> >> > +    * __sfp().  The __sfp() uses fp->_flags == 0 test as an
>> >> >> > +    * indication of the unused FILE.
>> >> >> > +    *
>> >> >> > +    * Taking the lock prevents possible compiler or processor
>> >> >> > +    * reordering of the writes performed before the final _flags
>> >> >> > +    * cleanup, making sure that we are done with the FILE before
>> >> >> > +    * it is considered available.
>> >> >> > +    */
>> >> >> > +   STDIO_THREAD_LOCK();
>> >> >> >     fp->_flags = 0;         /* Release this FILE for reuse. */
>> >> >> > +   STDIO_THREAD_UNLOCK();
>> >> >> >     FUNLOCKFILE(fp);
>> >> >> >     return (r);
>> >> >>
>> >> >> Is that assumption about reordering correct?  I.e. is FreeBSD's spinlock a
>> >> >> two-way barrier?
>> >> >>
>> >> >> It isn't unusual for the locking primitive to be a one-way barrier, i.e. (from
>> >> >> Linux kernel memory-barriers.txt) "Memory operations that occur before a LOCK
>> >> >> operation may appear to happen after it completes."  See also acq and rel in
>> >> >> atomic(9).
>> >> > Taking the lock prevents the __sfp from iterating the list until the
>> >> > spinlock is released. Since release makes sure that all previous memory
>> >> > operations become visible, the acquire in the spinlock lock provides
>> >> > the neccesary guarentee.
>> >>
>> >> IMHO, the lock to me is too heavy here. What about this patch?
>> >>
>> >> NOTE: patch just show thoughts. I didn't even check build checking.
>> > Yes, it might be correct. But FreeBSD does prefer the acq/rel barriers
>> > over the rmb/wmb.
>> >
>>
>> There is no stand alone acq/rel APIs (Sorry, as new guy to FreeBSD,
>> don't know too much APIs).  They are bound to atomic operations.
>> And yes, atomic_acq/rel should work also.
>
> Yes, you would want to use atomic_store_rel() or some such.  Often doing
> so is much clearer than stand-alone membar's as it indicates which write
> has special ordering.
>
Thanks a lot for clarification.

>> > Also, the lock is not that heavy right there, and the committed patch
>> > provides mostly zero overhead for non-threaded case.
>>
>> But lock could introduced contention in SMP case which could be avoid
>> with rmb/wmb.
>
> Seriously, if you are using stdio, you've already given up on performance
> enough to not care about contention for fclose() vs fopen().
>
This makes sense.

> --
> John Baldwin