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

[Peter Zijlstra]

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'.

Paul?

> Fixes: 56180dd20c19 ("futex: Use RCU-based per-CPU reference counting instead of rcuref_t")
> Assisted-by: kres:claude-opus-5
> Signed-off-by: Chris Mason <mason@xxxxxxxxxx>
> ---
>  kernel/futex/core.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/futex/core.c b/kernel/futex/core.c
> index 128c5752f225..05619eb8329c 100644
> --- a/kernel/futex/core.c
> +++ b/kernel/futex/core.c
> @@ -209,10 +209,14 @@ static bool __futex_pivot_hash(struct mm_struct *mm, struct futex_private_hash *
>  		futex_rehash_private(fph, new);
>  	}
>  	new->state = FR_PERCPU;
> -	scoped_guard(rcu) {
> -		mmph->batches = get_state_synchronize_rcu();
> -		rcu_assign_pointer(mmph->hash, new);
> -	}
> +	rcu_assign_pointer(mmph->hash, new);
> +	/*
> +	 * Pairs with futex_ref_drop(): ->batches must be sampled at or after
> +	 * the rcu_assign_pointer() above, so any grace-period satisfying
> +	 * poll_state_synchronize_rcu() provably started once no reader could
> +	 * still load the retired fph. Both stores are done under mmph->lock.
> +	 */
> +	mmph->batches = get_state_synchronize_rcu();
>  	kvfree_rcu(fph, rcu);
>  	return true;
>  }
> -- 
> 2.53.0-Meta
>