This is the mail archive of the 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] Remove __PTHREAD_MUTEX_HAVE_ELISION undefined warning

On 08-04-2014 17:30, Carlos O'Donell wrote:
> On 03/26/2014 04:29 PM, Adhemerval Zanella wrote:
>> On 26-03-2014 14:50, Roland McGrath wrote:
>>> I don't understand what "support for 64 bits" or "support for 32 bits"
>>> means.  OK, I've looked at bits/pthreadtypes.h so I do understand.  But it
>>> seems pretty wrong to pretend this is a generic 32/64 sort of thing when it
>>> is really just about the x86-private layout of pthread_mutex_t.  It seems
>>> more proper to have bits/pthreadtypes.h just define __PTHREAD_SPINS.  
>>> That can be a separate cleanup if you want, and others may want to kibitz.
>>> But that might involve dropping the header you're adding here, so maybe you
>>> want to just resolve it now.
>>> If you want to go ahead with this change, then it's OK with the other nits
>>> above and this comment rewritten to describe concretely what the macro
>>> means.  In actual usage so far, it doesn't actually mean anything about
>>> elision support per se.  It just means something about how the fields of
>>> pthread_mutex_t are structured and hence what the initializer must look like.
>>> If that's all it's for, it should be made clear.
>> Cleanup up the whole __PTHREAD_SPINS seems the appropriated measure.  This patch
>> moves the __PTHREAD_SPINS definition to arch specific header since pthread_mutex_t
>> layout is also arch specific and does not make sense disassociate them.
>> This makes the definition of __PTHREAD_MUTEX_HAVE_ELISION not required.
> My opinion is that you should *not* wait for arch maintainers to review this
> patch. It's a fairly mechanical patch that should not break any configuration
> and should not cause any undue ABI changes. A simple grep'ing shows you won't
> have any problems.
> If you want to be nice send a follow-up email and TO all of the arch
> maintainers letting them know you made the change. This should cut through
> the noise and let them test at their own time and pace.
> I pulled the patch down, applied, and tested with it and it looks good
> for x86-64, no regressions.
> OK to checkin.
Pushed upstream as 01f8eac224421f07f28f91cc05db7459ea433ea4 and CCing all the arch

PS: sorry for the spam.

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