Re: [PATCH v4 06/11] futex: Allow to re-allocate the private hash bucket.

Sebastian Andrzej Siewior
2024-12-12T16:16:50Z

On 2024-12-10 23:27:32 [+0100], Thomas Gleixner wrote:
> On Tue, Dec 03 2024 at 17:42, Sebastian Andrzej Siewior wrote:
> > +static void futex_put_old_hb_p(struct futex_hash_bucket_private *hb_p)
> > +{
> > + unsigned int slots = hb_p->hash_mask + 1;
> > + struct futex_hash_bucket *hb;
> > + DEFINE_WAKE_Q(wake_q);
> > + unsigned int i;
> > +
> > + for (i = 0; i < slots; i++) {
> > + struct futex_q *this;
> > +
> > + hb = &hb_p->queues[i];
> > +
> > + spin_lock(&hb->lock);
> > + plist_for_each_entry(this, &hb->chain, list)
> > + wake_q_add(&wake_q, this->task);
> > + spin_unlock(&hb->lock);
> > + }
> > + futex_hash_priv_put(hb_p);
> > +
> > + wake_up_q(&wake_q);
>
> So you wake up all queued waiters and let themself requeue on the new
> hash.
>
> How is that safe against the following situation:
>
> CPU 0 CPU 1
> hb_p_old = mm->futex_hash_bucket; hbp = mm->futex_hash_bucket;
> mm->futex_hash_bucket = new;
> // Referrence count succeeds!
> rcuref_get(&hpb->refcnt);
> futex_put_old_hb_p();
> // Queues on old hash and
> // is lost forever
> queue(hbp);

This does not happen. futex_q_lock() check if the hb is valid after
locking the HB it obtained. So if the HB is still valid then
futex_put_old_hb_p() will see/ wake it. If it is not then it will drop
the lock and try again.

However looking at your proposal below, it has some benefits. Let me
implement this instead.

Sebastian