Re: [PATCH 09/14] Add a new pthread_mutexattr_setelision_np interface.

[Andi Kleen <andi at firstfloor dot org>]

> Roland objects to this patch principally because the API should
> be about behaviour guarantees and performance characteristics
> that the application can describe.

Elision has a firm abstract behavior model (much firmer than
most performance critical functions in glibc imho)
I describe it a bit in the manual, but can expand it if it helps.

> The fact is that elision is an
> implementation detail of the underlying lock type. The library
> should be deciding amongst the implementation details given
> information it knows about the lock.

The library already does that (that is what the "adaptive elision
algorithm" is all about). But we don't have enough experience with
the algorithm yet to be sure that it works well in all cases.
Also for experimenting with the algorithms there needs to be some
way to turn it on/off per lock. That is what the interface does.

> 
> What API would allow us to describe the lock in such a way that
> glibc could make the decision to use or not use elision?

Well in theory you could describe abort reasons one by one
(may do syscalls all the time, may conflict all the time etc.)
And say do not elide if you know that you would abort for this bit.
But TSX does not guarantee aborts in all these cases, that 
is just what the current implementation does. And POWER, zSeries
etc. may do something different.

Could be done, but would be likely awkward.

Also I looked through existing glibc nterfaces 
for precedence, and as far as I can see this interface is doing exactly
the same as mallopt() does:
- pure tuning hint
- no not undefined program should be affected
- allows to tune some internals
- it's all hint, if the internal implementation changes it just ignores
the hint.

I presume we could define an indexed name space like mallopt() 
if that helps?

Can you please ask Roland to explain why mallopt() is ok and 
pthread_{rwlock,mutex}attr_setelision_np() is not and what the
differences are?

> I will review this patch anyway because I feel that the review
> of the patch is instructive and because this patch could get
> picked up by a downstream that desires to add these interfaces
> and maintain the as an experimental interface.

That would fragment the glibc ABI. Is that what you're suggesting?

> >      __pthread_mutexattr_settype;
> > +    pthread_mutexattr_setelision_np;
> 
> This doesn't look like the nptl/Version file I have on master,
> so I figure you have some additional changes that should be
> reverted.

This is relative to the earlier NORMAL patch, which adds new symbols there.

I'll send a new patch tomorrow with the update you asked for.

> 
> You don't need this? 
> 
> If you add the interfaces, then you add their support to the testsuite.

I don't have the capability to maintain that many versions
of the test suite files.  I'll remove all the tests related to this.

-Andi