Message-ID: <20250312151634.2183278-18-bigeasy@linutronix.de>
Date: Wed, 12 Mar 2025 16:16:30 +0100
From: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
To: linux-kernel@...r.kernel.org
Cc: 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>,
	Peter Zijlstra <peterz@...radead.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Valentin Schneider <vschneid@...hat.com>,
	Waiman Long <longman@...hat.com>,
	Sebastian Andrzej Siewior <bigeasy@...utronix.de>
Subject: [PATCH v10 17/21] futex: Untangle and naming

From: Peter Zijlstra <peterz@...radead.org>

Untangle the futex_private_hash::users increment from finding the hb.

  hb = __futex_hash(key) /* finds the hb */
  hb = futex_hash(key)   /* finds the hb and inc users */

Use __futex_hash() for re-hashing, notably allowing to rehash into the
global hash.

This gets us:

  hb = futex_hash(key) /* gets hb and inc users */
  futex_hash_get(hb)   /* inc users */
  futex_hash_put(hb)   /* dec users */

But then we have:

  fph = futex_get_private_hash()  /* get fph and inc */
  futex_put_private_hash()        /* dec */

Which doesn't match naming, so change to:

  fph = futex_private_hash()    /* get and inc */
  futex_private_hash_get(fph)   /* inc */
  futex_private_hash_put(fph)   /* dec */

Add a CLASS for the private_hash, to clean up some trivial wrappers.

Additional random renaming that happened while mucking about with the
code.

Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
---
 kernel/futex/core.c     | 170 +++++++++++++++++++++++-----------------
 kernel/futex/futex.h    |  27 +++++--
 kernel/futex/waitwake.c |  25 ++----
 3 files changed, 124 insertions(+), 98 deletions(-)

diff --git a/kernel/futex/core.c b/kernel/futex/core.c
index 37c3e020f2f03..1c00890cc4fb5 100644
--- a/kernel/futex/core.c
+++ b/kernel/futex/core.c
@@ -124,25 +124,34 @@ static inline bool futex_key_is_private(union futex_key *key)
 	return !(key->both.offset & (FUT_OFF_INODE | FUT_OFF_MMSHARED));
 }
 
-static struct futex_hash_bucket *futex_hash_private(union futex_key *key,
-						    struct futex_hash_bucket *fhb,
-						    u32 hash_mask)
+static struct futex_hash_bucket *
+__futex_hash(union futex_key *key, struct futex_private_hash *fph);
+
+#ifdef CONFIG_FUTEX_PRIVATE_HASH
+static struct futex_hash_bucket *
+__futex_hash_private(union futex_key *key, struct futex_private_hash *fph)
 {
 	u32 hash;
 
+	if (!futex_key_is_private(key))
+		return NULL;
+
+	if (!fph)
+		fph = rcu_dereference(key->private.mm->futex_phash);
+	if (!fph || !fph->hash_mask)
+		return NULL;
+
 	hash = jhash2((void *)&key->private.address,
 		      sizeof(key->private.address) / 4,
 		      key->both.offset);
-	return &fhb[hash & hash_mask];
+	return &fph->queues[hash & fph->hash_mask];
 }
 
-#ifdef CONFIG_FUTEX_PRIVATE_HASH
-static void futex_rehash_current_users(struct futex_private_hash *old,
-				       struct futex_private_hash *new)
+static void futex_rehash_private(struct futex_private_hash *old,
+				 struct futex_private_hash *new)
 {
 	struct futex_hash_bucket *hb_old, *hb_new;
 	unsigned int slots = old->hash_mask + 1;
-	u32 hash_mask = new->hash_mask;
 	unsigned int i;
 
 	for (i = 0; i < slots; i++) {
@@ -158,7 +167,7 @@ static void futex_rehash_current_users(struct futex_private_hash *old,
 
 			WARN_ON_ONCE(this->lock_ptr != &hb_old->lock);
 
-			hb_new = futex_hash_private(&this->key, new->queues, hash_mask);
+			hb_new = __futex_hash(&this->key, new);
 			futex_hb_waiters_inc(hb_new);
 			/*
 			 * The new pointer isn't published yet but an already
@@ -173,8 +182,8 @@ static void futex_rehash_current_users(struct futex_private_hash *old,
 	}
 }
 
-static bool futex_assign_new_hash(struct mm_struct *mm,
-				  struct futex_private_hash *new)
+static bool __futex_pivot_hash(struct mm_struct *mm,
+			       struct futex_private_hash *new)
 {
 	struct futex_private_hash *fph;
 
@@ -194,14 +203,27 @@ static bool futex_assign_new_hash(struct mm_struct *mm,
 			return false;
 		}
 
-		futex_rehash_current_users(fph, new);
+		futex_rehash_private(fph, new);
 	}
 	rcu_assign_pointer(mm->futex_phash, new);
 	kvfree_rcu(fph, rcu);
 	return true;
 }
 
-struct futex_private_hash *futex_get_private_hash(void)
+static void futex_pivot_hash(struct mm_struct *mm)
+{
+	scoped_guard (mutex, &mm->futex_hash_lock) {
+		struct futex_private_hash *fph;
+
+		fph = mm->futex_phash_new;
+		if (fph) {
+			mm->futex_phash_new = NULL;
+			__futex_pivot_hash(mm, fph);
+		}
+	}
+}
+
+struct futex_private_hash *futex_private_hash(void)
 {
 	struct mm_struct *mm = current->mm;
 	/*
@@ -225,41 +247,73 @@ struct futex_private_hash *futex_get_private_hash(void)
 		if (rcuref_get(&fph->users))
 			return fph;
 	}
-	scoped_guard (mutex, &mm->futex_hash_lock) {
-		struct futex_private_hash *fph;
-
-		fph = mm->futex_phash_new;
-		if (fph) {
-			mm->futex_phash_new = NULL;
-			futex_assign_new_hash(mm, fph);
-		}
-	}
+	futex_pivot_hash(mm);
 	goto again;
 }
 
-void futex_put_private_hash(struct futex_private_hash *fph)
+bool futex_private_hash_get(struct futex_private_hash *fph)
 {
-	/* Ignore return value, last put is verified via rcuref_is_dead() */
-	if (rcuref_put(&fph->users)) {
-		;
+	return rcuref_get(&fph->users);
+}
+
+void futex_private_hash_put(struct futex_private_hash *fph)
+{
+	/*
+	 * Ignore the result; the DEAD state is picked up
+	 * when rcuref_get() starts failing via rcuref_is_dead().
+	 */
+	bool __maybe_unused ignore = rcuref_put(&fph->users);
+}
+
+struct futex_hash_bucket *futex_hash(union futex_key *key)
+{
+	struct futex_private_hash *fph;
+	struct futex_hash_bucket *hb;
+
+again:
+	scoped_guard (rcu) {
+		hb = __futex_hash(key, NULL);
+		fph = hb->priv;
+
+		if (!fph || futex_private_hash_get(fph))
+			return hb;
 	}
+	futex_pivot_hash(key->private.mm);
+	goto again;
 }
 
-static struct futex_private_hash *futex_get_private_hb(union futex_key *key)
+void futex_hash_get(struct futex_hash_bucket *hb)
 {
-	if (!futex_key_is_private(key))
-		return NULL;
+	struct futex_private_hash *fph = hb->priv;
 
-	return futex_get_private_hash();
+	if (!fph)
+		return;
+	WARN_ON_ONCE(!futex_private_hash_get(fph));
 }
 
-#else
+void futex_hash_put(struct futex_hash_bucket *hb)
+{
+	struct futex_private_hash *fph = hb->priv;
 
-static struct futex_private_hash *futex_get_private_hb(union futex_key *key)
+	if (!fph)
+		return;
+	futex_private_hash_put(fph);
+}
+
+#else /* !CONFIG_FUTEX_PRIVATE_HASH */
+
+static inline struct futex_hash_bucket *
+__futex_hash_private(union futex_key *key, struct futex_private_hash *fph)
 {
 	return NULL;
 }
-#endif
+
+struct futex_hash_bucket *futex_hash(union futex_key *key)
+{
+	return __futex_hash(key, NULL);
+}
+
+#endif /* CONFIG_FUTEX_PRIVATE_HASH */
 
 /**
  * futex_hash - Return the hash bucket in the global hash
@@ -269,14 +323,15 @@ static struct futex_private_hash *futex_get_private_hb(union futex_key *key)
  * corresponding hash bucket in the global hash. If the FUTEX is private and
  * a local hash table is privated then this one is used.
  */
-struct futex_hash_bucket *__futex_hash(union futex_key *key)
+static struct futex_hash_bucket *
+__futex_hash(union futex_key *key, struct futex_private_hash *fph)
 {
-	struct futex_private_hash *fph;
+	struct futex_hash_bucket *hb;
 	u32 hash;
 
-	fph = futex_get_private_hb(key);
-	if (fph)
-		return futex_hash_private(key, fph->queues, fph->hash_mask);
+	hb = __futex_hash_private(key, fph);
+	if (hb)
+		return hb;
 
 	hash = jhash2((u32 *)key,
 		      offsetof(typeof(*key), both.offset) / 4,
@@ -284,34 +339,6 @@ struct futex_hash_bucket *__futex_hash(union futex_key *key)
 	return &futex_queues[hash & futex_hashmask];
 }
 
-#ifdef CONFIG_FUTEX_PRIVATE_HASH
-/**
- * futex_hash_get - Get an additional reference for the local hash.
- * @hb:		    ptr to the private local hash.
- *
- * Obtain an additional reference for the already obtained hash bucket. The
- * caller must already own an reference.
- */
-void futex_hash_get(struct futex_hash_bucket *hb)
-{
-	struct futex_private_hash *fph = hb->priv;
-
-	if (!fph)
-		return;
-
-	WARN_ON_ONCE(!rcuref_get(&fph->users));
-}
-
-void futex_hash_put(struct futex_hash_bucket *hb)
-{
-	struct futex_private_hash *fph = hb->priv;
-
-	if (!fph)
-		return;
-	futex_put_private_hash(fph);
-}
-#endif
-
 /**
  * futex_setup_timer - set up the sleeping hrtimer.
  * @time:	ptr to the given timeout value
@@ -1152,7 +1179,6 @@ static void compat_exit_robust_list(struct task_struct *curr)
 static void exit_pi_state_list(struct task_struct *curr)
 {
 	struct list_head *next, *head = &curr->pi_state_list;
-	struct futex_private_hash *fph;
 	struct futex_pi_state *pi_state;
 	union futex_key key = FUTEX_KEY_INIT;
 
@@ -1166,7 +1192,7 @@ static void exit_pi_state_list(struct task_struct *curr)
 	 * on the mutex.
 	 */
 	WARN_ON(curr != current);
-	fph = futex_get_private_hash();
+	guard(private_hash)();
 	/*
 	 * We are a ZOMBIE and nobody can enqueue itself on
 	 * pi_state_list anymore, but we have to be careful
@@ -1229,8 +1255,6 @@ static void exit_pi_state_list(struct task_struct *curr)
 		raw_spin_lock_irq(&curr->pi_lock);
 	}
 	raw_spin_unlock_irq(&curr->pi_lock);
-	if (fph)
-		futex_put_private_hash(fph);
 }
 #else
 static inline void exit_pi_state_list(struct task_struct *curr) { }
@@ -1410,9 +1434,7 @@ static int futex_hash_allocate(unsigned int hash_slots)
 			 * mm->futex_hash_lock which we currently hold and last
 			 * put is verified via rcuref_is_dead().
 			 */
-			if (rcuref_put(&mm->futex_phash->users)) {
-				;
-			}
+			futex_private_hash_put(mm->futex_phash);
 		}
 
 		if (mm->futex_phash_new) {
@@ -1433,7 +1455,7 @@ static int futex_hash_allocate(unsigned int hash_slots)
 		 * Will set mm->futex_phash_new on failure;
 		 * futex_get_private_hash() will try again.
 		 */
-		futex_assign_new_hash(mm, fph);
+		__futex_pivot_hash(mm, fph);
 	}
 	kvfree(hb_tofree);
 	return 0;
diff --git a/kernel/futex/futex.h b/kernel/futex/futex.h
index 5b6b58e8a7008..8eba9982bcae1 100644
--- a/kernel/futex/futex.h
+++ b/kernel/futex/futex.h
@@ -204,23 +204,38 @@ extern struct hrtimer_sleeper *
 futex_setup_timer(ktime_t *time, struct hrtimer_sleeper *timeout,
 		  int flags, u64 range_ns);
 
-extern struct futex_hash_bucket *__futex_hash(union futex_key *key);
+extern struct futex_hash_bucket *futex_hash(union futex_key *key);
+
 #ifdef CONFIG_FUTEX_PRIVATE_HASH
 extern void futex_hash_get(struct futex_hash_bucket *hb);
 extern void futex_hash_put(struct futex_hash_bucket *hb);
-extern struct futex_private_hash *futex_get_private_hash(void);
-extern void futex_put_private_hash(struct futex_private_hash *fph);
+
+extern struct futex_private_hash *futex_private_hash(void);
+extern bool futex_private_hash_get(struct futex_private_hash *fph);
+extern void futex_private_hash_put(struct futex_private_hash *fph);
 
 #else /* !CONFIG_FUTEX_PRIVATE_HASH */
 static inline void futex_hash_get(struct futex_hash_bucket *hb) { }
 static inline void futex_hash_put(struct futex_hash_bucket *hb) { }
-static inline struct futex_private_hash *futex_get_private_hash(void) { return NULL; }
-static inline void futex_put_private_hash(struct futex_private_hash *fph) { }
+
+static inline struct futex_private_hash *futex_private_hash(void)
+{
+	return NULL;
+}
+static inline bool futex_private_hash_get(struct futex_private_hash *fph)
+{
+	return false;
+}
+static inline void futex_private_hash_put(struct futex_private_hash *fph) { }
 #endif
 
 DEFINE_CLASS(hb, struct futex_hash_bucket *,
 	     if (_T) futex_hash_put(_T),
-	     __futex_hash(key), union futex_key *key);
+	     futex_hash(key), union futex_key *key);
+
+DEFINE_CLASS(private_hash, struct futex_private_hash *,
+	     if (_T) futex_private_hash_put(_T),
+	     futex_private_hash(), void);
 
 /**
  * futex_match - Check whether two futex keys are equal
diff --git a/kernel/futex/waitwake.c b/kernel/futex/waitwake.c
index 0d150453a0b41..74647f6bf75de 100644
--- a/kernel/futex/waitwake.c
+++ b/kernel/futex/waitwake.c
@@ -400,12 +400,18 @@ int futex_unqueue_multiple(struct futex_vector *v, int count)
  *  -  0 - Success
  *  - <0 - -EFAULT, -EWOULDBLOCK or -EINVAL
  */
-static int __futex_wait_multiple_setup(struct futex_vector *vs, int count, int *woken)
+int futex_wait_multiple_setup(struct futex_vector *vs, int count, int *woken)
 {
 	bool retry = false;
 	int ret, i;
 	u32 uval;
 
+	/*
+	 * Make sure to have a reference on the private_hash such that we
+	 * don't block on rehash after changing the task state below.
+	 */
+	guard(private_hash)();
+
 	/*
 	 * Enqueuing multiple futexes is tricky, because we need to enqueue
 	 * each futex on the list before dealing with the next one to avoid
@@ -491,23 +497,6 @@ static int __futex_wait_multiple_setup(struct futex_vector *vs, int count, int *
 	return 0;
 }
 
-int futex_wait_multiple_setup(struct futex_vector *vs, int count, int *woken)
-{
-	struct futex_private_hash *fph;
-	int ret;
-
-	/*
-	 * Assume to have a private futex and acquire a reference on the private
-	 * hash to avoid blocking on mm_struct::futex_hash_bucket during rehash
-	 * after changing the task state.
-	 */
-	fph = futex_get_private_hash();
-	ret = __futex_wait_multiple_setup(vs, count, woken);
-	if (fph)
-		futex_put_private_hash(fph);
-	return ret;
-}
-
 /**
  * futex_sleep_multiple - Check sleeping conditions and sleep
  * @vs:    List of futexes to wait for
-- 
2.47.2

Powered by blists - more mailing lists