sched/wait: Standardize wait_bit_queue naming
authorIngo Molnar <mingo@kernel.org>
Sun, 5 Mar 2017 10:35:27 +0000 (11:35 +0100)
committerIngo Molnar <mingo@kernel.org>
Tue, 20 Jun 2017 10:18:29 +0000 (12:18 +0200)
So wait-bit-queue head variables are often named:

struct wait_bit_queue *q

... which is a bit ambiguous and super confusing, because
they clearly suggest wait-queue head semantics and behavior
(they rhyme with the old wait_queue_t *q naming), while they
are extended wait-queue _entries_, not heads!

They are misnomers in two ways:

 - the 'wait_bit_queue' leaves open the question of whether
   it's an entry or a head

 - the 'q' parameter and local variable naming falsely implies
   that it's a 'queue' - while it's an entry.

This resulted in sometimes confusing cases such as:

finish_wait(wq, &q->wait);

where the 'q' is not a wait-queue head, but a wait-bit-queue entry.

So improve this all by standardizing wait-bit-queue nomenclature
similar to wait-queue head naming:

struct wait_bit_queue   => struct wait_bit_queue_entry
q => wbq_entry

Which makes it all a much clearer:

struct wait_bit_queue_entry *wbq_entry

... and turns the former confusing piece of code into:

finish_wait(wq_head, &wbq_entry->wq_entry;

which IMHO makes it apparently clear what we are doing,
without having to analyze the context of the code: we are
adding a wait-queue entry to a regular wait-queue head,
which entry is embedded in a wait-bit-queue entry.

I'm not a big fan of acronyms, but repeating wait_bit_queue_entry
in field and local variable names is too long, so Hopefully it's
clear enough that 'wq_' prefixes stand for wait-queues, while
'wbq_' prefixes stand for wait-bit-queues.

Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
include/linux/wait.h
kernel/sched/wait.c

index 1c8add685f2251b136da08aad31490de0f57fcc9..fc7c32d821200fc9ad2cbc083d6bc258f7ca7367 100644 (file)
@@ -36,7 +36,7 @@ struct wait_bit_key {
        unsigned long           timeout;
 };
 
-struct wait_bit_queue {
+struct wait_bit_queue_entry {
        struct wait_bit_key     key;
        struct wait_queue_entry wq_entry;
 };
@@ -207,8 +207,8 @@ void __wake_up_sync_key(struct wait_queue_head *wq_head, unsigned int mode, int
 void __wake_up_locked(struct wait_queue_head *wq_head, unsigned int mode, int nr);
 void __wake_up_sync(struct wait_queue_head *wq_head, unsigned int mode, int nr);
 void __wake_up_bit(struct wait_queue_head *, void *, int);
-int __wait_on_bit(struct wait_queue_head *, struct wait_bit_queue *, wait_bit_action_f *, unsigned);
-int __wait_on_bit_lock(struct wait_queue_head *, struct wait_bit_queue *, wait_bit_action_f *, unsigned);
+int __wait_on_bit(struct wait_queue_head *, struct wait_bit_queue_entry *, wait_bit_action_f *, unsigned);
+int __wait_on_bit_lock(struct wait_queue_head *, struct wait_bit_queue_entry *, wait_bit_action_f *, unsigned);
 void wake_up_bit(void *, int);
 void wake_up_atomic_t(atomic_t *);
 int out_of_line_wait_on_bit(void *, int, wait_bit_action_f *, unsigned);
@@ -989,7 +989,7 @@ int wake_bit_function(struct wait_queue_entry *wq_entry, unsigned mode, int sync
 #define DEFINE_WAIT(name) DEFINE_WAIT_FUNC(name, autoremove_wake_function)
 
 #define DEFINE_WAIT_BIT(name, word, bit)                               \
-       struct wait_bit_queue name = {                                  \
+       struct wait_bit_queue_entry name = {                            \
                .key = __WAIT_BIT_KEY_INITIALIZER(word, bit),           \
                .wq_entry = {                                           \
                        .private        = current,                      \
index f1ba0625b8bef8e177e42a5f3c2cf880ce63fc0e..95e6d3820cbac0492b5c933342558e0a35b3656f 100644 (file)
@@ -394,8 +394,7 @@ EXPORT_SYMBOL(woken_wake_function);
 int wake_bit_function(struct wait_queue_entry *wq_entry, unsigned mode, int sync, void *arg)
 {
        struct wait_bit_key *key = arg;
-       struct wait_bit_queue *wait_bit
-               = container_of(wq_entry, struct wait_bit_queue, wq_entry);
+       struct wait_bit_queue_entry *wait_bit = container_of(wq_entry, struct wait_bit_queue_entry, wq_entry);
 
        if (wait_bit->key.flags != key->flags ||
                        wait_bit->key.bit_nr != key->bit_nr ||
@@ -412,17 +411,17 @@ EXPORT_SYMBOL(wake_bit_function);
  * permitted return codes. Nonzero return codes halt waiting and return.
  */
 int __sched
-__wait_on_bit(struct wait_queue_head *wq_head, struct wait_bit_queue *q,
+__wait_on_bit(struct wait_queue_head *wq_head, struct wait_bit_queue_entry *wbq_entry,
              wait_bit_action_f *action, unsigned mode)
 {
        int ret = 0;
 
        do {
-               prepare_to_wait(wq_head, &q->wq_entry, mode);
-               if (test_bit(q->key.bit_nr, q->key.flags))
-                       ret = (*action)(&q->key, mode);
-       } while (test_bit(q->key.bit_nr, q->key.flags) && !ret);
-       finish_wait(wq_head, &q->wq_entry);
+               prepare_to_wait(wq_head, &wbq_entry->wq_entry, mode);
+               if (test_bit(wbq_entry->key.bit_nr, wbq_entry->key.flags))
+                       ret = (*action)(&wbq_entry->key, mode);
+       } while (test_bit(wbq_entry->key.bit_nr, wbq_entry->key.flags) && !ret);
+       finish_wait(wq_head, &wbq_entry->wq_entry);
        return ret;
 }
 EXPORT_SYMBOL(__wait_on_bit);
@@ -450,15 +449,15 @@ int __sched out_of_line_wait_on_bit_timeout(
 EXPORT_SYMBOL_GPL(out_of_line_wait_on_bit_timeout);
 
 int __sched
-__wait_on_bit_lock(struct wait_queue_head *wq_head, struct wait_bit_queue *q,
+__wait_on_bit_lock(struct wait_queue_head *wq_head, struct wait_bit_queue_entry *wbq_entry,
                        wait_bit_action_f *action, unsigned mode)
 {
        int ret = 0;
 
        for (;;) {
-               prepare_to_wait_exclusive(wq_head, &q->wq_entry, mode);
-               if (test_bit(q->key.bit_nr, q->key.flags)) {
-                       ret = action(&q->key, mode);
+               prepare_to_wait_exclusive(wq_head, &wbq_entry->wq_entry, mode);
+               if (test_bit(wbq_entry->key.bit_nr, wbq_entry->key.flags)) {
+                       ret = action(&wbq_entry->key, mode);
                        /*
                         * See the comment in prepare_to_wait_event().
                         * finish_wait() does not necessarily takes wwq_head->lock,
@@ -466,11 +465,11 @@ __wait_on_bit_lock(struct wait_queue_head *wq_head, struct wait_bit_queue *q,
                         * smp_mb__after_atomic() before wake_up_page().
                         */
                        if (ret)
-                               finish_wait(wq_head, &q->wq_entry);
+                               finish_wait(wq_head, &wbq_entry->wq_entry);
                }
-               if (!test_and_set_bit(q->key.bit_nr, q->key.flags)) {
+               if (!test_and_set_bit(wbq_entry->key.bit_nr, wbq_entry->key.flags)) {
                        if (!ret)
-                               finish_wait(wq_head, &q->wq_entry);
+                               finish_wait(wq_head, &wbq_entry->wq_entry);
                        return 0;
                } else if (ret) {
                        return ret;
@@ -538,7 +537,7 @@ static int wake_atomic_t_function(struct wait_queue_entry *wq_entry, unsigned mo
                                  void *arg)
 {
        struct wait_bit_key *key = arg;
-       struct wait_bit_queue *wait_bit = container_of(wq_entry, struct wait_bit_queue, wq_entry);
+       struct wait_bit_queue_entry *wait_bit = container_of(wq_entry, struct wait_bit_queue_entry, wq_entry);
        atomic_t *val = key->flags;
 
        if (wait_bit->key.flags != key->flags ||
@@ -554,25 +553,25 @@ static int wake_atomic_t_function(struct wait_queue_entry *wq_entry, unsigned mo
  * return codes halt waiting and return.
  */
 static __sched
-int __wait_on_atomic_t(struct wait_queue_head *wq_head, struct wait_bit_queue *q,
+int __wait_on_atomic_t(struct wait_queue_head *wq_head, struct wait_bit_queue_entry *wbq_entry,
                       int (*action)(atomic_t *), unsigned mode)
 {
        atomic_t *val;
        int ret = 0;
 
        do {
-               prepare_to_wait(wq_head, &q->wq_entry, mode);
-               val = q->key.flags;
+               prepare_to_wait(wq_head, &wbq_entry->wq_entry, mode);
+               val = wbq_entry->key.flags;
                if (atomic_read(val) == 0)
                        break;
                ret = (*action)(val);
        } while (!ret && atomic_read(val) != 0);
-       finish_wait(wq_head, &q->wq_entry);
+       finish_wait(wq_head, &wbq_entry->wq_entry);
        return ret;
 }
 
 #define DEFINE_WAIT_ATOMIC_T(name, p)                                  \
-       struct wait_bit_queue name = {                                  \
+       struct wait_bit_queue_entry name = {                            \
                .key = __WAIT_ATOMIC_T_KEY_INITIALIZER(p),              \
                .wq_entry = {                                           \
                        .private        = current,                      \