From: Jens Axboe <axboe@kernel.dk>
To: John Stoffel <john@quad.stoffel.home>
Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
hannes@cmpxchg.org, jack@suse.cz, torvalds@linux-foundation.org
Subject: Re: [PATCH 0/12 v3] Writeback improvements
Date: Thu, 28 Sep 2017 15:39:49 +0200 [thread overview]
Message-ID: <0ad8b55b-1712-51f4-ac89-a838bfe3b81e@kernel.dk> (raw)
In-Reply-To: <20170928131949.GA4384@quad.stoffel.home>
On 09/28/2017 03:19 PM, John Stoffel wrote:
> On Wed, Sep 27, 2017 at 02:13:47PM -0600, Jens Axboe wrote:
>> We've had some issues with writeback in presence of memory reclaim
>> at Facebook, and this patch set attempts to fix it up. The real
>> functional change for that issue is patch 10. The rest are cleanups,
>> as well as the removal of doing non-range cyclic writeback. The users
>> of that was sync_inodes_sb() and wakeup_flusher_threads(), both of
>> which writeback all of the dirty pages.
>
> So does this patch set make things faster? Less bursty? Does it make
> writeout take longer, but with less spikes? What is the performance
> impact of this change? I hate to be a pain, but this just smacks of
> arm waving and I'm sure FB doesn't make changes without data... :-)
See patch 10, this isn't arm waving at all. The whole point is that
you can have millions of writeback work items, which don't do anything.
See not only are we wasting a full core of doing nothing, it's bad
enough that we can trigger softlockups since it's just sitting there
in a loop doing that.
It's all explained in that patch...
>> The basic idea is that we have callers that call
>> wakeup_flusher_threads() with nr_pages == 0. This means 'writeback
>> everything'. For memory reclaim situations, we can end up queuing
>> a TON of these kinds of writeback units. This can cause softlockups
>> and further memory issues, since we allocate huge amounts of
>> struct wb_writeback_work to handle this writeback. Handle this
>> situation more gracefully.
>
> Do you push back on the callers or slow them down? Why do we even
> allow callers to flush everything?
Ehm, because we have to? There are cases where flushing everything
makes sense. Laptop mode is one of them, the problematic case
here is memory reclaim. To clean dirty pages, you have to kick
the flusher threads.
--
Jens Axboe
prev parent reply other threads:[~2017-09-28 13:39 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-27 20:13 Jens Axboe
2017-09-27 20:13 ` [PATCH 01/12] buffer: have alloc_page_buffers() use __GFP_NOFAIL Jens Axboe
2017-09-28 14:08 ` Nikolay Borisov
2017-10-02 15:02 ` Jan Kara
2017-09-27 20:13 ` [PATCH 02/12] buffer: grow_dev_page() should use __GFP_NOFAIL for all cases Jens Axboe
2017-09-28 14:11 ` Nikolay Borisov
2017-09-28 18:12 ` Jens Axboe
2017-10-03 12:10 ` Jan Kara
2017-10-03 12:25 ` Jan Kara
2017-10-03 14:36 ` Jens Axboe
2017-10-03 15:52 ` Jan Kara
2017-09-27 20:13 ` [PATCH 03/12] buffer: eliminate the need to call free_more_memory() in __getblk_slow() Jens Axboe
2017-09-28 14:12 ` Nikolay Borisov
2017-10-03 12:22 ` Jan Kara
2017-09-27 20:13 ` [PATCH 04/12] fs: kill 'nr_pages' argument from wakeup_flusher_threads() Jens Axboe
2017-09-27 20:13 ` [PATCH 05/12] writeback: switch wakeup_flusher_threads() to cyclic writeback Jens Axboe
2017-10-03 12:43 ` Jan Kara
2017-09-27 20:13 ` [PATCH 06/12] writeback: provide a wakeup_flusher_threads_bdi() Jens Axboe
2017-09-27 20:13 ` [PATCH 07/12] writeback: pass in '0' for nr_pages writeback in laptop mode Jens Axboe
2017-09-27 20:13 ` [PATCH 08/12] writeback: make wb_start_writeback() static Jens Axboe
2017-09-27 20:13 ` [PATCH 09/12] writeback: move nr_pages == 0 logic to one location Jens Axboe
2017-09-27 20:13 ` [PATCH 10/12] writeback: only allow one inflight and pending full flush Jens Axboe
2017-09-28 21:41 ` Andrew Morton
2017-09-28 21:44 ` Linus Torvalds
2017-09-29 0:17 ` Jens Axboe
2017-09-29 5:21 ` Amir Goldstein
2017-10-03 16:06 ` Matthew Wilcox
2017-10-03 16:11 ` Jens Axboe
2017-10-03 17:03 ` Matthew Wilcox
2017-09-29 0:15 ` Jens Axboe
2017-09-27 20:13 ` [PATCH 11/12] writeback: make sync_inodes_sb() use range cyclic writeback Jens Axboe
2017-09-27 20:13 ` [PATCH 12/12] writeback: kill off ->range_cycle option Jens Axboe
2017-09-27 20:22 ` Jens Axboe
2017-09-28 13:19 ` [PATCH 0/12 v3] Writeback improvements John Stoffel
2017-09-28 13:39 ` Jens Axboe [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=0ad8b55b-1712-51f4-ac89-a838bfe3b81e@kernel.dk \
--to=axboe@kernel.dk \
--cc=hannes@cmpxchg.org \
--cc=jack@suse.cz \
--cc=john@quad.stoffel.home \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®