Re: [PATCH v2 2/2] bcache: Move journal work to new background wq

[Kai Krakow <kai@kaishome.de>]

Hi Coly!

With v2, I added WQ_MEM_RECLAIM to the background wq. But with it, I
consistently get 50% worse boot times (20 -> 30s), when set the value
to 0 again, boot times are better again. The desktop also reacts to
clicks earlier right after boot.

Is there memory reclaim pressure during boot?

As far as I understand the code, this would trigger an immediate
journal flush then under memory reclaim because this background wq is
only used to reschedule journal flush some time in the future (100ms?)
if there's nothing to write just now:

>         } else if (!w->dirty) {
>                 w->dirty = true;
> -               schedule_delayed_work(&c->journal.work,
> -                                     msecs_to_jiffies(c->journal_delay_ms));
> +               queue_delayed_work(bch_background_wq, &c->journal.work,
> +                                  msecs_to_jiffies(c->journal_delay_ms));
>                 spin_unlock(&c->journal.lock);
>         } else {

Do you have an explanation? Will it be safe to return to the previous
init using no WQ_MEM_RECLAIM for background wq? Like this:

-       bch_background_wq = alloc_workqueue("bch_background",
WQ_MEM_RECLAIM, 0);
+       bch_background_wq = alloc_workqueue("bch_background", 0, 0);

Maybe we should find a better name for this - not bch_background?

Thanks,
Kai

Am Do., 28. Jan. 2021 um 11:50 Uhr schrieb Kai Krakow <kai@kaishome.de>:
>
> 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.
>
> Cc: Coly Li <colyli@suse.de>
> Signed-off-by: Kai Krakow <kai@kaishome.de>
> ---
>  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..70565ed5732d7 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_background_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..942e97dd17554 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_background_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 dc4fe7eeda815..5206c037c13f3 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_background_wq;
>  struct workqueue_struct *bch_journal_wq;
>
>
> @@ -2822,6 +2823,8 @@ static void bcache_exit(void)
>                 destroy_workqueue(bcache_wq);
>         if (bch_journal_wq)
>                 destroy_workqueue(bch_journal_wq);
> +       if (bch_background_wq)
> +               destroy_workqueue(bch_background_wq);
>
>         if (bcache_major)
>                 unregister_blkdev(bcache_major, "bcache");
> @@ -2884,6 +2887,10 @@ static int __init bcache_init(void)
>         if (bch_btree_init())
>                 goto err;
>
> +       bch_background_wq = alloc_workqueue("bch_background", WQ_MEM_RECLAIM, 0);
> +       if (!bch_background_wq)
> +               goto err;
> +
>         bch_journal_wq = alloc_workqueue("bch_journal", WQ_MEM_RECLAIM, 0);
>         if (!bch_journal_wq)
>                 goto err;
> --
> 2.26.2
>