Re: [patch V2 2/7] futex: Hash private futexes per process

[Peter Zijlstra <peterz@infradead.org>]

On Fri, May 27, 2016 at 07:10:01PM +0200, Sebastian Andrzej Siewior wrote:
> On 2016-05-19 14:24:06 [+0200], Peter Zijlstra wrote:
> > On Thu, May 05, 2016 at 08:44:04PM -0000, Thomas Gleixner wrote:
> > > +static struct futex_hash_bucket *hash_futex(union futex_key *key)
> > > +{
> > > +#ifdef CONFIG_FUTEX_PRIVATE_HASH
> > > +	struct mm_struct *mm = current->mm;
> > > +	unsigned int slot;
> > > +
> > > +	/*
> > > +	 * Futexes which use the per process hash have the lower bits cleared
> > > +	 */
> > > +	if (key->both.offset & (FUT_OFF_INODE | FUT_OFF_MMSHARED))
> > > +		return hash_global_futex(key);
> > > +
> > > +	slot = hash_long(key->private.address, mm->futex_hash.hash_bits);
> > > +	return &mm->futex_hash.hash[slot];
> > 
> > Do we want the option to WARN if we get collisions in this per-process
> > hash?
> > 
> > Because afaiu there is no guarantee what so ever this doesn't happen,
> > and collisions here can create the very same priority inversions as are
> > possible in the global hash.
> > 
> > Less likely etc.. more contained since its only the threads of the one
> > process that get tangled up, but still possible.
> 
> Since the collision is contained the same process it is less dramatic.

Right, but can still cause significant malfunction inside the process.
So its not something to completely ignore. If your room sized CNC
machine gets the priorities of the logging thread and the motor control
thread confused bad things could happen.

> But how do you want to warn the user? A trace-event would be handy to
> dump the uaddr and slot.

So I think there's a number of cases:

 - PREALLOC_HASH finds a taken bucket; in this case we can simply return
   an error.
 - PREALLOC_HASH succeeds, but an on demand hash later hits the same
   bucket. This is harder; we could maybe mark all buckets taken by
   PREALLOC_HASH and allow for a signal when this collision hits. Dunno.

> The user would have to check the trace and
> figure out which slot was assigend to different uaddr. 

Yeah, that's not really workable, might work for debugging, but blergh.

> But due to ASLR
> the same application might result in a different behaviour on each run.

Yeah, ASLR makes this all somewhat non deterministic, which is why you
really don't want a silent collision for your PREALLOC_HASH buckets.
Because once every 100 runs it does weird,..

> However, it might be good for a indication about the size of the private
> hash…

Yeah, now if online resize wasn't such a pain ;-)