Re: [PATCH v3 3/3] bcache: Move journal work to new background wq

Am Fr., 29. Jan. 2021 um 17:06 Uhr schrieb Coly Li [off-list ref]:

On 1/29/21 7:28 AM, Kai Krakow wrote:

quoted

This is potentially long running and not latency sensitive, let's get
it out of the way of other latency sensitive events.

As observed in the previous commit, the `system_wq` comes easily
congested by bcache, and this fixes a few more stalls I was observing
every once in a while.

Let's not make this `WQ_MEM_RECLAIM` as it showed to reduce performance
of boot and file system operations in my tests. Also, without
`WQ_MEM_RECLAIM`, I no longer see desktop stalls. This matches the
previous behavior as `system_wq` also does no memory reclaim:

quoted

// workqueue.c:
system_wq = alloc_workqueue("events", 0, 0);

Your text is convinced. I am OK with this patch. One more request, could
you please add a comment at alloc_workqueue() to explain why setting
flags as 0 and no WQ_MEM_RECLAIM. It should be helpful to others who
don't read our discussion.

I'll post a v4.

quoted

Cc: Coly Li <redacted>
Signed-off-by: Kai Krakow <redacted>
---
 drivers/md/bcache/bcache.h  | 1 +
 drivers/md/bcache/journal.c | 4 ++--
 drivers/md/bcache/super.c   | 7 +++++++
 3 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/md/bcache/bcache.h b/drivers/md/bcache/bcache.h
index b1ed16c7a5341..e8bf4f752e8be 100644
--- a/drivers/md/bcache/bcache.h
+++ b/drivers/md/bcache/bcache.h

@@ -1001,6 +1001,7 @@ void bch_write_bdev_super(struct cached_dev *dc, struct closure *parent);

 extern struct workqueue_struct *bcache_wq;
 extern struct workqueue_struct *bch_journal_wq;
+extern struct workqueue_struct *bch_flush_wq;
 extern struct mutex bch_register_lock;
 extern struct list_head bch_cache_sets;

diff --git a/drivers/md/bcache/journal.c b/drivers/md/bcache/journal.c
index aefbdb7e003bc..c6613e8173337 100644
--- a/drivers/md/bcache/journal.c
+++ b/drivers/md/bcache/journal.c

@@ -932,8 +932,8 @@ atomic_t *bch_journal(struct cache_set *c,
              journal_try_write(c);
      } else if (!w->dirty) {
              w->dirty = true;
-             schedule_delayed_work(&c->journal.work,
-                                   msecs_to_jiffies(c->journal_delay_ms));
+             queue_delayed_work(bch_flush_wq, &c->journal.work,
+                                msecs_to_jiffies(c->journal_delay_ms));
              spin_unlock(&c->journal.lock);
      } else {
              spin_unlock(&c->journal.lock);

diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index 77c5d8b6d4316..817b36c39b4fc 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c

@@ -49,6 +49,7 @@ static int bcache_major;
 static DEFINE_IDA(bcache_device_idx);
 static wait_queue_head_t unregister_wait;
 struct workqueue_struct *bcache_wq;
+struct workqueue_struct *bch_flush_wq;
 struct workqueue_struct *bch_journal_wq;

@@ -2821,6 +2822,8 @@ static void bcache_exit(void)
              destroy_workqueue(bcache_wq);
      if (bch_journal_wq)
              destroy_workqueue(bch_journal_wq);
+     if (bch_flush_wq)
+             destroy_workqueue(bch_flush_wq);
      bch_btree_exit();

      if (bcache_major)

@@ -2884,6 +2887,10 @@ static int __init bcache_init(void)
      if (!bcache_wq)
              goto err;

+     bch_flush_wq = alloc_workqueue("bch_flush", 0, 0);
+     if (!bch_flush_wq)
+             goto err;
+
      bch_journal_wq = alloc_workqueue("bch_journal", WQ_MEM_RECLAIM, 0);
      if (!bch_journal_wq)
              goto err;