This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH v2] Add and use new glibc-internal futex API.


On Fri, 2015-06-19 at 14:27 -0700, Roland McGrath wrote:
> > But we have plenty of futex calls that are not used for PThreads
> > synchronization data structures.  I don't think it clarifies the code if
> > for those, we can't simply pass in FUTEX_PRIVATE, but have to first do
> > another call that transforms it to whatever futex_wake or such would
> > like to see internally.  So now we have those calls littered throughout
> > the code, and it would be cleanest to call this on every callsite.  That
> > just seems wrong.
> 
> What I was calling "internal form" is exactly FUTEX_PRIVATE et al--the
> arguments to futex-internal.h functions.  I never proposed that direct
> uses of futex-internal.h would do anything different.

Ah -- so we're not as much in disagreement as I thought we might be.

> (For NaCl, the
> FUTEX_{PRIVATE,SHARED} macros might well just have the same value,
> since the values would never actually be examined.)

> It sort of seems like you missed this paragraph, though you quoted it:
> > > If we did need to handle this case, then I think the best thing would be
> > > to have another sysdeps call that transforms an internal form stored by
> > > *_setpshared into the internal form actually used in futex_wake et al.
> > > Then pthread_mutex_init et al would use that call instead of simply
> > > copying the field from attributes object to synchronization object.  (I
> > > don't have any objection to structuring it this way now even though all
> > > the sysdeps implementations today would in fact just copy the value.)
> 
> As I said there, if we were to handle the "degrade to shared"
> situation, then there could be a third form (a second internal form).
> For such a configuration, that form would be the only one that needs
> to distinguish requested-private from requested-shared just so that
> *_getpshared can work.  In configurations without that need (which is
> all we have today), then presumably the sysdeps call mentioned above
> would simply use "final" internal form (i.e. FUTEX_* values passed to
> futex-internal.h functions) as the intermediate internal form
> (i.e. what's stored in attributes objects).
> 
> Since we don't need to handle that case today, I think this is all
> moot.  For now, just having final internal form (FUTEX_* values) be
> what's stored in attributes objects as well as synchronization objects
> will work fine.

If we had sufficient space in the attribute structures to just store an
int that is just FUTEX_PRIVATE or FUTEX_SHARED, I'd agree that this
would work for now.  (And if we don't consider the "degrade to shared"
case.)

However, on x86 for example, the attributes structs are 4 bytes for
condvar, barrier, and mutex and 8 bytes for rwlock.  Barrier just has a
shared attribute; condvars need to store shared and which clock.  Mutex
attributes use several bits (kind, shared, robust, ...).  rwlock has
kind and shared.  Thus, only barrier and rwlock could use an int just
for the internal form of shared.

We could try to make the internal form compatible with the the other
things that condvar and mutex need to put into the same int, but then
we're restricting the actual values the futex API can use.

We have one such restriction already (which I need to document):
FUTEX_PRIVATE must be zero, because the initializers for rwlock,
condvar, and mutex all have zeros for the shared field (or bit).


In the interest of making progress on this, I suggest that we treat the
attribute initialization separately.  I'm going to work with you towards
bringing that into a shape that we're both happy with.  I agree that
eventually, we'll want to use FUTEX_PRIVATE / FUTEX_SHARED to store
shared or not in data structures such as pthread_barrier.  However,
right now, we can't yet do that everywhere, or not easily in this patch:
* barrier still has assembly implementations that expect other values;
* for condvar I have posted a new algorithm, and we'd do the conversion
in that patch;
* mutex encodes shared together with other bits, and we have the
dependencies on what the lll_* code does;
* for rwlock, a new algorithm is WIP.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]