Re: [PATCH] futex: sample poll cookie after publishing new hash

[Paul E. McKenney]

On Fri, Sep 25, 2026 at 05:58:13PM +0000, Chris Mason wrote:
> On Thu Aug 20, 2026 at 10:40 AM UTC, Peter Zijlstra wrote:
> > On Mon, Aug 17, 2026 at 06:01:42PM -0700, Chris Mason wrote:
> >> futex_ref_drop() may only skip its grace period when one has already
> >> elapsed since the current private hash was published:
> >>
> >>     kernel/futex/core.c:futex_ref_drop
> >>         if (poll_state_synchronize_rcu(mm->futex.phash.batches)) {
> >>                 /*
> >>                  * There was a grace-period, we can begin now.
> >>                  */
> >>                 __futex_ref_atomic_begin(fph);
> >>                 return;
> >>         }
> >>
> >> The cookie it polls is sampled one statement before that publication:
> >>
> >>     kernel/futex/core.c:__futex_pivot_hash
> >>         new->state = FR_PERCPU;
> >>         scoped_guard(rcu) {
> >>                 mmph->batches = get_state_synchronize_rcu();
> >>                 rcu_assign_pointer(mmph->hash, new);
> >>         }
> >>         kvfree_rcu(fph, rcu);
> >>
> >> get_state_synchronize_rcu() anchors its guarantee at the snapshot, so
> >> the cookie is cleared by the first grace period that starts from there
> >> on, including one starting between the two stores which never waited
> >> for a reader that loaded the old hash after it began.
> >>
> >>     CPU 0 (resize)                    CPU 1 (futex_hash)
> >>     ==============                    ==================
> >>     __futex_pivot_hash()
> >>       batches = get_state_...()
> >>                                       grace period starts
> >>                                       guard(rcu)
> >>                                       fph = old hash
> >>       rcu_assign_pointer(hash, new)
> >>       kvfree_rcu(old hash)
> >>     futex_hash_allocate()
> >>       futex_ref_drop(new hash)
> >>         poll_state_...() -> true
> >>         __futex_ref_atomic_begin()
> >>           atomic = LONG_MAX
> >>                                       futex_ref_get(old hash) -> true
> >>                                       spin_lock(&fph->queues[i].lock)
> >>
> >> The reference count lives in the mm and has just been biased for the
> >> new generation, so the stalled reader pins and then locks the retired
> >> hash that is already queued for free, and its later put is charged
> >> against the live generation.
> >>
> >> Fix by sampling the cookie after rcu_assign_pointer() publishes the
> >> new hash. Drop the surrounding scoped_guard(rcu) while at it: it only
> >> delayed completion of the prematurely anchored grace-period and serves
> >> no purpose once the cookie is sampled after publication. The writer
> >> side is serialized by mm->futex.phash.lock and neither
> >> rcu_assign_pointer() nor kvfree_rcu() requires a read-side section.
> >
> > God, how I hate reading AI output :-(
> >
> > Anyway, the thinking was that by holding rcu_read_lock(), the current
> > RCU-GP cannot change and the cookie and assignment are effectively
> > 'atomic'.
> 
> >From what I can tell there are a few ways for new grace periods to start
> while we're holding rcu_read_lock(), synchronize_rcu_expedited() if no
> GP is currently in flight being the easiest?
> 
> Anyway, this BUG_ON() fires for me:
> 
> diff --git a/kernel/futex/core.c b/kernel/futex/core.c
> index a061f54b6..18d51c145 100644
> --- a/kernel/futex/core.c
> +++ b/kernel/futex/core.c
> @@ -215,6 +215,14 @@ static bool __futex_pivot_hash(struct mm_struct *mm, struct futex_private_hash *
>  	new->state = FR_PERCPU;
>  	scoped_guard(rcu) {
>  		mmph->batches = get_state_synchronize_rcu();
> +		/*
> +		 * Fires only if a grace period started after the cookie
> +		 * above was taken, although rcu_read_lock() is held. That
> +		 * grace period then satisfies the stored cookie, yet it began
> +		 * before the new hash is published below.
> +		 */
> +		BUG_ON(!same_state_synchronize_rcu(mmph->batches,
> +						   get_state_synchronize_rcu()));
>  		rcu_assign_pointer(mmph->hash, new);
>  	}
>  	kvfree_rcu(fph, rcu);
> 
> -chris

This matches my understanding of RCU.  Although rcu_read_lock() will
prevent a *new* RCU grace period from ending, it will not prevent an *old*
one from ending or a new one from starting.

							Thanx, Paul