On Tue 10-05-11 10:14:21, Wu Fengguang wrote:
> On Sat, May 07, 2011 at 12:33:12AM +0800, Jan Kara wrote:
> > On Fri 06-05-11 11:08:23, Wu Fengguang wrote:
> > > With the more aggressive "keep writeback as long as we wrote something"
> > > logic in wb_writeback(), the "use LONG_MAX .nr_to_write" trick in commit
> > > b9543dac5bbc ("writeback: avoid livelocking WB_SYNC_ALL writeback") is
> > > no longer enough to stop sync livelock.
> > >
> > > The fix is to explicitly update .dirtied_when on synced inodes, so that
> > > they are no longer considered for writeback in the next round.
> > The changelog doesn't make sense now when the patch is in the second
> > place in the series... Also as we discussed in the previous iteration of
> > the patches, I thought you'll move the condition before mapping_tagged()
> > test. I.e. the code would be (comment updated):
> > if (!(inode->i_state & I_FREEING)) {
> > /*
> > * Sync livelock prevention: Each inode is tagged and
> > * synced in one shot. So we can unconditionally update its
> > * dirty time to prevent syncing it again. Note that time
> > * ordering of b_dirty list will be kept because the
> > * following code either removes the inode from b_dirty
> > * or calls redirty_tail().
> > */
> > if (wbc->sync_mode == WB_SYNC_ALL || wbc->tagged_sync)
> > inode->dirtied_when = jiffies;
> > if (mapping_tagged(mapping, PAGECACHE_TAG_DIRTY)) {
>
> It looks good, thanks! Here is the updated patch with your
> reviewed-by. Note that I retains a modified paragraph in the changelog
> to clarify its relationship to the other two patches.
Thanks. It's fine now.
Honza
> Thanks,
> Fengguang
> ---
> Subject: writeback: update dirtied_when for synced inode to prevent livelock
> Date: Wed Apr 27 19:05:21 CST 2011
>
> Explicitly update .dirtied_when on synced inodes, so that they are no
> longer considered for writeback in the next round.
>
> We'll do more aggressive "keep writeback as long as we wrote something"
> logic in wb_writeback(). Then the "use LONG_MAX .nr_to_write" trick in
> commit b9543dac5b ("writeback: avoid livelocking WB_SYNC_ALL writeback")
> will no longer be enough to stop sync livelock.
>
> Reviewed-by: Jan Kara <jack@xxxxxxx>
> Signed-off-by: Wu Fengguang <fengguang.wu@xxxxxxxxx>
> ---
> fs/fs-writeback.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> --- linux-next.orig/fs/fs-writeback.c 2011-05-10 09:50:07.000000000 +0800
> +++ linux-next/fs/fs-writeback.c 2011-05-10 10:03:00.000000000 +0800
> @@ -419,6 +419,15 @@ writeback_single_inode(struct inode *ino
> spin_lock(&inode->i_lock);
> inode->i_state &= ~I_SYNC;
> if (!(inode->i_state & I_FREEING)) {
> + /*
> + * Sync livelock prevention. Each inode is tagged and synced in
> + * one shot, so we can unconditionally update its dirty time to
> + * prevent syncing it again. Note that time ordering of b_dirty
> + * list will be kept because the following code either removes
> + * the inode from b_dirty or calls redirty_tail().
> + */
> + if (wbc->sync_mode == WB_SYNC_ALL || wbc->tagged_sync)
> + inode->dirtied_when = jiffies;
> if (mapping_tagged(mapping, PAGECACHE_TAG_DIRTY)) {
> /*
> * We didn't write back all the pages. nfs_writepages()
--
Jan Kara <jack@xxxxxxx>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
