This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Add PTHREAD_MUTEX_NORMAL_INT
- From: Torvald Riegel <triegel at redhat dot com>
- To: Andi Kleen <ak at linux dot jf dot intel dot com>
- Cc: Rich Felker <dalias at aerifal dot cx>, Andi Kleen <andi at firstfloor dot org>, "Carlos O'Donell" <carlos at redhat dot com>, libc-alpha at sourceware dot org
- Date: Wed, 26 Jun 2013 00:04:10 +0200
- Subject: Re: [PATCH] Add PTHREAD_MUTEX_NORMAL_INT
- References: <1372105055-31254-1-git-send-email-andi at firstfloor dot org> <51C8AB50 dot 80108 at redhat dot com> <20130624203147 dot GO5643 at tassilo dot jf dot intel dot com> <51C8B797 dot 7080503 at redhat dot com> <20130624232607 dot GT6123 at two dot firstfloor dot org> <51C9B7D0 dot 5000207 at redhat dot com> <20130625170656 dot GB6123 at two dot firstfloor dot org> <20130625173203 dot GL29800 at brightrain dot aerifal dot cx> <20130625204013 dot GS5643 at tassilo dot jf dot intel dot com>
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