Re: [PATCH 07/10] Add __pthread_set_abort_hook export

[Torvald Riegel <triegel at redhat dot com>]

On Fri, 2013-01-11 at 23:33 +0100, Andi Kleen wrote:
> On Fri, Jan 11, 2013 at 10:23:50PM +0100, Torvald Riegel wrote:
> > On Thu, 2013-01-10 at 12:19 -0800, Andi Kleen wrote:
> > > From: Andi Kleen <ak@linux.intel.com>
> > > 
> > > For some debugging scenarios it is useful to catch aborts. Looking
> > > at the abort code can be the only way to get information out of a
> > > memory transaction.
> > 
> > If it's purely for debugging purposes through external debugging tools,
> > a systemtap probe or something similar might also be sufficient (or
> > better).
> 
> It's not. Out of band monitoring is very different from in-band.

So what's the in-band monitoring use case?

> > 
> > > This exports an __pthread_set_abort_hook function that can set
> > > this abort hook.
> > 
> > If we'll have this function, it should at least be called
> > __pthread_set_hle_abort_hook or similar (otherwise, which "abort"?).
> 
> The name is not good because there is no HLE used
> (you seem to have this generic termninology problem in your email)

Nope, I've just been aware of lock elision before Haswell was announced
(see my other mail).

And yes, what you implement is the concept of eliding locks using HW
help, so I'd use the hardware lock elision name coined for this concept.

> 
> I'm not aware of any other aborts in pthreads, so it's unique.

But that doesn't mean that there won't be any other in the future.  This
is especially of concern because you just report one unsigned value as
parameter.

What are the precise uses cases for this hook?

> There may be better names, but I think my current one is reasonable
> and better than your proposal.
> 
> > > +typedef void (*__pthread_abort_hook_t) (unsigned);
> > 
> > If this is to be used for "debugging"/testing/logging code inside of
> > programs, is just the TSX abort code sufficient?  I suppose the lock
> > being touched would be useful too (if this abort hook is just used for
> > HLE).
> 
> The hook is designed to catch any transaction.
> The lock could be a rwlock/mutex/spinlock (if we elide them)
> or even something which does not fall back to a lock.
> So passing a lock would be ill-defined.

Then extend it so that it's properly defined.

> If someone wants to know the place they are in they can use 
> backtrace()

Which brings us back to my initial question: What's the use case?  I
don't see the hook being useful for the use cases I can imagine, and I
don't see how the hook would be better than systemtap probes or the like
if external tools are used anyway.


Torvald