In the commit mentioned below, fscache was converted from slow-work to
workqueue. slow_work_enqueue() and slow_work_sleep_till_thread_needed()
did not use a per-CPU workqueue. They choose from two global waitqueues
depending on the SLOW_WORK_VERY_SLOW bit which was not set so it always
one waitqueue.

I can't find out how it is ensured that a waiter on certain CPU is woken
up be the other side. My guess is that the timeout in schedule_timeout()
ensures that it does not wait forever (or a random wake up).

fscache_object_sleep_till_congested() must be invoked from preemptible
context in order for schedule() to work. In this case this_cpu_ptr()
should complain with CONFIG_DEBUG_PREEMPT enabled except the thread is
bound to one CPU.

wake_up() wakes only one waiter and I'm not sure if it is guaranteed
that only one waiter exists.

Replace the per-CPU waitqueue with one global waitqueue.

Fixes: 8b8edefa2fffb ("fscache: convert object to use workqueue instead of 
slow-work")
Reported-by: Gregor Beck <gregor.b...@gmail.com>
Signed-off-by: Sebastian Andrzej Siewior <bige...@linutronix.de>
---
v2: https://lore.kernel.org/all/20211029083839.xwwt7jgzru3kc...@linutronix.de/
Repost: 
https://lore.kernel.org/all/20211118165442.hekmz7xgisdzs...@linutronix.de/
Ping 1: 
https://lore.kernel.org/all/20211202205240.giqxuxqemlxxo...@linutronix.de/
|I noticed that -next gained commit
|   608bfec640edb ("fscache: Remove the contents of the fscache driver, pending 
rewrite")
|
|which removes slow_work_sleep_till_thread_needed() and the per-CPU
|variable. Since it looks like a bug, what happens stable wise?

Ping 2: https://lore.kernel.org/all/ybdiyn+wu1rn9...@linutronix.de/

 fs/fscache/internal.h |  1 -
 fs/fscache/main.c     |  6 ------
 fs/fscache/object.c   | 13 +++++--------
 3 files changed, 5 insertions(+), 15 deletions(-)

diff --git a/fs/fscache/internal.h b/fs/fscache/internal.h
index c3e4804b8fcbf..9edb87e11680b 100644
--- a/fs/fscache/internal.h
+++ b/fs/fscache/internal.h
@@ -81,7 +81,6 @@ extern unsigned fscache_debug;
 extern struct kobject *fscache_root;
 extern struct workqueue_struct *fscache_object_wq;
 extern struct workqueue_struct *fscache_op_wq;
-DECLARE_PER_CPU(wait_queue_head_t, fscache_object_cong_wait);
 
 extern unsigned int fscache_hash(unsigned int salt, unsigned int *data, 
unsigned int n);
 
diff --git a/fs/fscache/main.c b/fs/fscache/main.c
index 4207f98e405fd..85f8cf3a323d5 100644
--- a/fs/fscache/main.c
+++ b/fs/fscache/main.c
@@ -41,8 +41,6 @@ struct kobject *fscache_root;
 struct workqueue_struct *fscache_object_wq;
 struct workqueue_struct *fscache_op_wq;
 
-DEFINE_PER_CPU(wait_queue_head_t, fscache_object_cong_wait);
-
 /* these values serve as lower bounds, will be adjusted in fscache_init() */
 static unsigned fscache_object_max_active = 4;
 static unsigned fscache_op_max_active = 2;
@@ -138,7 +136,6 @@ unsigned int fscache_hash(unsigned int salt, unsigned int 
*data, unsigned int n)
 static int __init fscache_init(void)
 {
        unsigned int nr_cpus = num_possible_cpus();
-       unsigned int cpu;
        int ret;
 
        fscache_object_max_active =
@@ -161,9 +158,6 @@ static int __init fscache_init(void)
        if (!fscache_op_wq)
                goto error_op_wq;
 
-       for_each_possible_cpu(cpu)
-               init_waitqueue_head(&per_cpu(fscache_object_cong_wait, cpu));
-
        ret = fscache_proc_init();
        if (ret < 0)
                goto error_proc;
diff --git a/fs/fscache/object.c b/fs/fscache/object.c
index 6a675652129b2..7a972d144b546 100644
--- a/fs/fscache/object.c
+++ b/fs/fscache/object.c
@@ -798,6 +798,8 @@ void fscache_object_destroy(struct fscache_object *object)
 }
 EXPORT_SYMBOL(fscache_object_destroy);
 
+static DECLARE_WAIT_QUEUE_HEAD(fscache_object_cong_wait);
+
 /*
  * enqueue an object for metadata-type processing
  */
@@ -806,16 +808,12 @@ void fscache_enqueue_object(struct fscache_object *object)
        _enter("{OBJ%x}", object->debug_id);
 
        if (fscache_get_object(object, fscache_obj_get_queue) >= 0) {
-               wait_queue_head_t *cong_wq =
-                       &get_cpu_var(fscache_object_cong_wait);
 
                if (queue_work(fscache_object_wq, &object->work)) {
                        if (fscache_object_congested())
-                               wake_up(cong_wq);
+                               wake_up(&fscache_object_cong_wait);
                } else
                        fscache_put_object(object, fscache_obj_put_queue);
-
-               put_cpu_var(fscache_object_cong_wait);
        }
 }
 
@@ -833,16 +831,15 @@ void fscache_enqueue_object(struct fscache_object *object)
  */
 bool fscache_object_sleep_till_congested(signed long *timeoutp)
 {
-       wait_queue_head_t *cong_wq = this_cpu_ptr(&fscache_object_cong_wait);
        DEFINE_WAIT(wait);
 
        if (fscache_object_congested())
                return true;
 
-       add_wait_queue_exclusive(cong_wq, &wait);
+       add_wait_queue_exclusive(&fscache_object_cong_wait, &wait);
        if (!fscache_object_congested())
                *timeoutp = schedule_timeout(*timeoutp);
-       finish_wait(cong_wq, &wait);
+       finish_wait(&fscache_object_cong_wait, &wait);
 
        return fscache_object_congested();
 }
-- 
2.34.1


--
Linux-cachefs mailing list
Linux-cachefs@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-cachefs

Reply via email to