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] Add PTHREAD_MUTEX_NORMAL_INT


On Tue, 2013-06-25 at 13:40 -0700, Andi Kleen wrote:
> The other patches don't add any new versions, just two new bits
> to two existing APIs (enable/disable elision for mutexes and
> rwlocks respectively)

And that's one of the contentious issues that still exist, and has been
noted in several reviews already (and not just recently...).  In
particular, there's (1) the question of whether we want to have any
additional bits at all, and (2) the question whether these bits should
affect semantics, or be pure performance hints.

Given that we don't have consensus about this, but can arguably get the
90% without it (now that we have a solution for DEFAULT vs. NORMAL), it
seems better to me to not have any such bits in the first round.  The
feedback from the first round should help us in doing the right thing
regarding any such bits in the second round.

Personally, I'd rather like to see the env var tuning patches go in than
the two bits: The env vars are not part of the ABI (and we don't have
the same fragmentation concerns etc.), they would just affect
performance not semantics and so would be easier to ignore, and I
suppose that they are useful for non-experts and without recompilation,
whereas the elision-bits likely require more experimentation and
trial-and-error effort on behalf of users.  But given that we don't seem
to have consensus for the env vars yet, I guess they'll have to wait
too.

> Also all of these patches have been already reviewed by multiple
> people, including Carlos.

Sure, but they didn't get approved, did they?  You addressed some of the
review comments, but didn't address nor discuss others.

> 
> > Also, the tuning per-lock is not nearly as impactful as adding mutex
> > type constants or changing the meaning of existing ones in subtle
> > ways. Adding these new tuning functions will not affect any
> > application that does not use them, i.e. any current application and
> > any conforming POSIX application. On the other hand, changing NORMAL
> > will affect many applications.
> 
> FWIW i spent quite some time on debian codesearch and koders.com 
> and I determined that NORMAL or pthread_mutexattr_settype(DEFAULT)
> is not widely used at all. That was the only reason I agreed to
> disable elision for NORMAL and plain settype() on old binaries.

That would actually be an argument *for* using a new internal type for
NORMAL: If it's rarely used, you won't ever take the branches for these
types, and they are marked as unlikely branches in my patch, for
example.  Thus, the impact of a new internal type on performance should
be very low.


Torvald


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