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

Started by Kai Krakow on Jan 29, 2021

open the first message on its own page

Message details and related content
Am Fr., 29. Jan. 2021 um 17:01 Uhr schrieb Coly Li [off-list ref]:
On 1/29/21 12:41 AM, Kai Krakow wrote:
quoted
quoted
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.
quoted
quoted
quoted
        } 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
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help