linux-kernel - Re: [PATCH v8 08/15] futex: Prepare for reference counting of the process private hash end of operation.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250204094922.GS7145@noisy.programming.kicks-ass.net>
Date: Tue, 4 Feb 2025 10:49:22 +0100
From: Peter Zijlstra <peterz@...radead.org>
To: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
Cc: linux-kernel@...r.kernel.org,
	André Almeida <andrealmeid@...lia.com>,
	Darren Hart <dvhart@...radead.org>,
	Davidlohr Bueso <dave@...olabs.net>, Ingo Molnar <mingo@...hat.com>,
	Juri Lelli <juri.lelli@...hat.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Valentin Schneider <vschneid@...hat.com>,
	Waiman Long <longman@...hat.com>
Subject: Re: [PATCH v8 08/15] futex: Prepare for reference counting of the
 process private hash end of operation.

On Mon, Feb 03, 2025 at 02:59:28PM +0100, Sebastian Andrzej Siewior wrote:

> @@ -555,11 +558,12 @@ struct futex_hash_bucket *futex_q_lock(struct futex_q *q)
>  	return hb;
>  }
>  
> -void futex_q_unlock(struct futex_hash_bucket *hb)
> +void futex_q_unlock_put(struct futex_hash_bucket *hb)
>  	__releases(&hb->lock)
>  {
>  	futex_hb_waiters_dec(hb);
>  	spin_unlock(&hb->lock);
> +	futex_hash_put(hb);
>  }

Here you don't

> @@ -288,23 +289,29 @@ extern void __futex_unqueue(struct futex_q *q);
>  extern void __futex_queue(struct futex_q *q, struct futex_hash_bucket *hb);
>  extern int futex_unqueue(struct futex_q *q);
>  
> +static inline void futex_hb_unlock_put(struct futex_hash_bucket *hb)
> +{
> +	spin_unlock(&hb->lock);
> +	futex_hash_put(hb);
> +}
> +
>  /**
> - * futex_queue() - Enqueue the futex_q on the futex_hash_bucket
> + * futex_queue_put() - Enqueue the futex_q on the futex_hash_bucket
>   * @q:	The futex_q to enqueue
>   * @hb:	The destination hash bucket
>   *
> - * The hb->lock must be held by the caller, and is released here. A call to
> - * futex_queue() is typically paired with exactly one call to futex_unqueue().  The
> - * exceptions involve the PI related operations, which may use futex_unqueue_pi()
> - * or nothing if the unqueue is done as part of the wake process and the unqueue
> - * state is implicit in the state of woken task (see futex_wait_requeue_pi() for
> - * an example).
> + * The hb->lock must be held by the caller, and is released here and the reference
> + * on the hb is dropped. A call to futex_queue_put() is typically paired with
> + * exactly one call to futex_unqueue(). The exceptions involve the PI related
> + * operations, which may use futex_unqueue_pi() or nothing if the unqueue is
> + * done as part of the wake process and the unqueue state is implicit in the
> + * state of woken task (see futex_wait_requeue_pi() for an example).
>   */
> -static inline void futex_queue(struct futex_q *q, struct futex_hash_bucket *hb)
> +static inline void futex_queue_put(struct futex_q *q, struct futex_hash_bucket *hb)
>  	__releases(&hb->lock)
>  {
>  	__futex_queue(q, hb);
> -	spin_unlock(&hb->lock);
> +	futex_hb_unlock_put(hb);
>  }

And here you do.


> @@ -380,11 +387,13 @@ double_lock_hb(struct futex_hash_bucket *hb1, struct futex_hash_bucket *hb2)
>  }
>  
>  static inline void
> -double_unlock_hb(struct futex_hash_bucket *hb1, struct futex_hash_bucket *hb2)
> +double_unlock_hb_put(struct futex_hash_bucket *hb1, struct futex_hash_bucket *hb2)
>  {
>  	spin_unlock(&hb1->lock);
>  	if (hb1 != hb2)
>  		spin_unlock(&hb2->lock);
> +	futex_hash_put(hb1);
> +	futex_hash_put(hb2);
>  }
>  

This seems horribly inconsistent and makes my head hurt. Where are the
matching gets for double_lock_hb() ?