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

[Kai Krakow <kai@kaishome.de>]

Am Fr., 29. Jan. 2021 um 17:01 Uhr schrieb Coly Li <colyli@suse.de>:
>
> On 1/29/21 12:41 AM, Kai Krakow wrote:
> >> 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?)

> For a typical 1000HZ jiffies, 100ms is extended 1 jiffy by
> msecs_to_jiffies().

Ah, you mean in the sense of lagging at least 1 jiffy behind because
work is dispatched asynchronously?

BTW: I'm using a 300 Hz system for my desktop, that's usually good
enough and maybe even a better choice for 60 Hz applications, as 300
divides easily by typical refresh rates (25, 30, 50, 60). But this is
useful information for the xpadneo driver I'm developing. Thanks.

> >>>         } 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 {
> >
> > This would mean we start performing worse under memory reclaim...
>
> A journal write buffer is 8 pages, for 4KB kernel page size, it won't be
> a large occupation.

As far as I can see the called routine would only spawn the actual
writes in a closure anyways. So if this was used for memory reclaim,
effects would lag behind anyways.

Still, I'm seeing a huge difference if this queue gets allocated with
`WQ_MEM_RECLAIM`. It works fine for most filesystems but for btrfs
there are probably at least twice that many outstanding requests.

But I don't think we need to discuss whether it should run under
memory reclaim, when the original implementation using `system_wq`
didn't do that in the first place. I was just curious and wanted to
understand the context better.

I think it's important to design carefully to not have vastly
different behavior whether we had a 100 Hz or a 1000 Hz kernel. For
example, my server builds usually run a 100 Hz kernel.

Thanks,
Kai