From: Christoph Hellwig <hch@infradead.org>
To: Jan Kara <jack@suse.cz>
Cc: Christoph Hellwig <hch@infradead.org>,
Curt Wohlgemuth <curtw@google.com>,
linux-fsdevel@vger.kernel.org, Al Viro <viro@ZenIV.linux.org.uk>
Subject: Re: [PATCH 6/6] vfs: Avoid unnecessary WB_SYNC_NONE writeback during sys_sync and reorder sync passes
Date: Thu, 20 Oct 2011 05:57:25 -0400 [thread overview]
Message-ID: <20111020095725.GF11291@infradead.org> (raw)
In-Reply-To: <1318020055-4450-7-git-send-email-jack@suse.cz>
On Fri, Oct 07, 2011 at 10:40:55PM +0200, Jan Kara wrote:
> wakeup_flusher_threads(0) will queue work doing complete writeback for each
> flusher thread. Thus there is not much point in submitting another work doing
> full inode WB_SYNC_NONE writeback by writeback_inodes_sb().
>
> After this change it does not make sense to call nonblocking ->sync_fs and
> block device flush before calling sync_inodes_sb() because
> wakeup_flusher_threads() is completely asynchronous and thus these functions
> would be called in parallel with inode writeback running which will effectively
> void any work they do. So we move sync_inodes_sb() call before these two
> functions.
> wakeup_flusher_threads(0);
> - iterate_supers(sync_inodes_one_sb, &nowait);
> + iterate_supers(sync_inodes_one_sb, &wait);
> iterate_supers(sync_fs_one_sb, &nowait);
> sync_all_bdevs(nowait);
> - iterate_supers(sync_inodes_one_sb, &wait);
> iterate_supers(sync_fs_one_sb, &wait);
> sync_all_bdevs(wait);
> if (unlikely(laptop_mode))
This looks a bit half-assed to me. Why do we still do the non-blocking
sync_all_bdevs call? This really only starts async writeback on the
block device inodes, which wakeup_flusher_threads already did.
Similarly I don't think the non-blocking ->sync_fs call really make
much sense anymore here.
Also we really need some good comment on why the order is
like the one chose here in this function.
next prev parent reply other threads:[~2011-10-20 9:57 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-10-07 20:40 [PATCH 0/6] Cleanup and improve sync (v3) Jan Kara
2011-10-07 20:40 ` [PATCH 1/6] vfs: Move noop_backing_dev_info check from sync into writeback Jan Kara
2011-10-20 9:48 ` Christoph Hellwig
2011-10-07 20:40 ` [PATCH 2/6] quota: Split dquot_quota_sync() to writeback and cache flushing part Jan Kara
2011-10-20 9:50 ` Christoph Hellwig
2011-10-07 20:40 ` [PATCH 3/6] quota: Move quota syncing to ->sync_fs method Jan Kara
2011-10-07 20:40 ` [PATCH 4/6] vfs: Reorder operations during sys_sync Jan Kara
2011-10-20 9:53 ` Christoph Hellwig
2011-10-20 23:57 ` Jan Kara
2011-10-07 20:40 ` [PATCH 5/6] vfs: Make sys_sync writeout also block device inodes Jan Kara
2011-10-20 9:54 ` Christoph Hellwig
2011-10-07 20:40 ` [PATCH 6/6] vfs: Avoid unnecessary WB_SYNC_NONE writeback during sys_sync and reorder sync passes Jan Kara
2011-10-20 9:57 ` Christoph Hellwig [this message]
2011-10-24 13:14 ` Jan Kara
2011-10-18 1:07 ` [PATCH 0/6] Cleanup and improve sync (v3) Jan Kara
2011-10-18 6:45 ` Christoph Hellwig
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=20111020095725.GF11291@infradead.org \
--to=hch@infradead.org \
--cc=curtw@google.com \
--cc=jack@suse.cz \
--cc=linux-fsdevel@vger.kernel.org \
--cc=viro@ZenIV.linux.org.uk \
/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;
as well as URLs for NNTP newsgroup(s).