All of lore.kernel.org
 help / color / mirror / Atom feed
From: Wu Fengguang <fengguang.wu@intel.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Jan Kara <jack@suse.cz>,
	linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
	Johannes Weiner <hannes@cmpxchg.org>,
	Wu Fengguang <fengguang.wu@intel.com>
Cc: Christoph Hellwig <hch@lst.de>, Jan Engelhardt <jengelh@medozas.de>
Cc: LKML <linux-kernel@vger.kernel.org>
Subject: [PATCH 5/5] writeback: check skipped pages on WB_SYNC_ALL
Date: Wed, 10 Nov 2010 10:35:05 +0800	[thread overview]
Message-ID: <20101110024224.144021908@intel.com> (raw)
In-Reply-To: 20101110023500.404859581@intel.com

[-- Attachment #1: writeback-warn-sync-skipped_pages.patch --]
[-- Type: text/plain, Size: 1238 bytes --]

In WB_SYNC_ALL mode, filesystems are not expected to skip dirty pages on
temporal lock contentions or non fatal errors, otherwise sync() will
return without actually syncing the skipped pages. Add a check to
catch possible redirty_page_for_writepage() callers that violate this
expectation.

I'd recommend to keep this check in -mm tree for some time and fixup the
possible warnings before pushing it to upstream.

If some FS triggers this warning and it's non-trivial to fix the FS,
we'll have to work out a sync retry scheme for skipped pages.

Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
---
 fs/fs-writeback.c |    6 ++++++
 1 file changed, 6 insertions(+)

--- linux-next.orig/fs/fs-writeback.c	2010-11-10 07:04:43.000000000 +0800
+++ linux-next/fs/fs-writeback.c	2010-11-10 07:11:03.000000000 +0800
@@ -527,6 +527,12 @@ static int writeback_sb_inodes(struct su
 			 * buffers.  Skip this inode for now.
 			 */
 			redirty_tail(inode);
+			/*
+			 * There's no logic to retry skipped pages for sync(),
+			 * filesystems are assumed not to skip dirty pages on
+			 * temporal lock contentions or non fatal errors.
+			 */
+			WARN_ON_ONCE(wbc->sync_mode == WB_SYNC_ALL);
 		}
 		spin_unlock(&inode_lock);
 		iput(inode);



WARNING: multiple messages have this Message-ID (diff)
From: Wu Fengguang <fengguang.wu@intel.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Jan Kara <jack@suse.cz>,
	linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
	Johannes Weiner <hannes@cmpxchg.org>,
	Wu Fengguang <fengguang.wu@intel.com>
Cc: Christoph Hellwig <hch@lst.de>, Jan Engelhardt <jengelh@medozas.de>
Cc: LKML <linux-kernel@vger.kernel.org>
Subject: [PATCH 5/5] writeback: check skipped pages on WB_SYNC_ALL
Date: Wed, 10 Nov 2010 10:35:05 +0800	[thread overview]
Message-ID: <20101110024224.144021908@intel.com> (raw)
In-Reply-To: 20101110023500.404859581@intel.com

[-- Attachment #1: writeback-warn-sync-skipped_pages.patch --]
[-- Type: text/plain, Size: 1534 bytes --]

In WB_SYNC_ALL mode, filesystems are not expected to skip dirty pages on
temporal lock contentions or non fatal errors, otherwise sync() will
return without actually syncing the skipped pages. Add a check to
catch possible redirty_page_for_writepage() callers that violate this
expectation.

I'd recommend to keep this check in -mm tree for some time and fixup the
possible warnings before pushing it to upstream.

If some FS triggers this warning and it's non-trivial to fix the FS,
we'll have to work out a sync retry scheme for skipped pages.

Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
---
 fs/fs-writeback.c |    6 ++++++
 1 file changed, 6 insertions(+)

--- linux-next.orig/fs/fs-writeback.c	2010-11-10 07:04:43.000000000 +0800
+++ linux-next/fs/fs-writeback.c	2010-11-10 07:11:03.000000000 +0800
@@ -527,6 +527,12 @@ static int writeback_sb_inodes(struct su
 			 * buffers.  Skip this inode for now.
 			 */
 			redirty_tail(inode);
+			/*
+			 * There's no logic to retry skipped pages for sync(),
+			 * filesystems are assumed not to skip dirty pages on
+			 * temporal lock contentions or non fatal errors.
+			 */
+			WARN_ON_ONCE(wbc->sync_mode == WB_SYNC_ALL);
 		}
 		spin_unlock(&inode_lock);
 		iput(inode);


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

WARNING: multiple messages have this Message-ID (diff)
From: Wu Fengguang <fengguang.wu@intel.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Jan Kara <jack@suse.cz>,
	linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
	Johannes Weiner <hannes@cmpxchg.org>,
	Wu Fengguang <fengguang.wu@intel.com>,
	Christoph Hellwig <hch@lst.de>,
	Jan Engelhardt <jengelh@medozas.de>,
	LKML <linux-kernel@vger.kernel.org>
Subject: [PATCH 5/5] writeback: check skipped pages on WB_SYNC_ALL
Date: Wed, 10 Nov 2010 10:35:05 +0800	[thread overview]
Message-ID: <20101110024224.144021908@intel.com> (raw)
In-Reply-To: 20101110023500.404859581@intel.com

[-- Attachment #1: writeback-warn-sync-skipped_pages.patch --]
[-- Type: text/plain, Size: 1534 bytes --]

In WB_SYNC_ALL mode, filesystems are not expected to skip dirty pages on
temporal lock contentions or non fatal errors, otherwise sync() will
return without actually syncing the skipped pages. Add a check to
catch possible redirty_page_for_writepage() callers that violate this
expectation.

I'd recommend to keep this check in -mm tree for some time and fixup the
possible warnings before pushing it to upstream.

If some FS triggers this warning and it's non-trivial to fix the FS,
we'll have to work out a sync retry scheme for skipped pages.

Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
---
 fs/fs-writeback.c |    6 ++++++
 1 file changed, 6 insertions(+)

--- linux-next.orig/fs/fs-writeback.c	2010-11-10 07:04:43.000000000 +0800
+++ linux-next/fs/fs-writeback.c	2010-11-10 07:11:03.000000000 +0800
@@ -527,6 +527,12 @@ static int writeback_sb_inodes(struct su
 			 * buffers.  Skip this inode for now.
 			 */
 			redirty_tail(inode);
+			/*
+			 * There's no logic to retry skipped pages for sync(),
+			 * filesystems are assumed not to skip dirty pages on
+			 * temporal lock contentions or non fatal errors.
+			 */
+			WARN_ON_ONCE(wbc->sync_mode == WB_SYNC_ALL);
 		}
 		spin_unlock(&inode_lock);
 		iput(inode);


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  parent reply	other threads:[~2010-11-10  2:45 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-11-10  2:35 [PATCH 0/5] writeback livelock fixes v2 Wu Fengguang
2010-11-10  2:35 ` Wu Fengguang
2010-11-10  2:35 ` [PATCH 1/5] writeback: integrated background writeback work Wu Fengguang
2010-11-10  2:35   ` Wu Fengguang
2010-11-10  2:35   ` Wu Fengguang
2010-11-10  2:35 ` [PATCH 2/5] writeback: trace wakeup event for background writeback Wu Fengguang
2010-11-10  2:35   ` Wu Fengguang
2010-11-10  2:35   ` Wu Fengguang
2010-11-10  2:35 ` [PATCH 3/5] writeback: stop background/kupdate works from livelocking other works Wu Fengguang
2010-11-10  2:35   ` Wu Fengguang
2010-11-10  2:35   ` Wu Fengguang
2010-11-10  3:55   ` Wu Fengguang
2010-11-10  3:55     ` Wu Fengguang
2010-11-10 16:26     ` Jan Kara
2010-11-10 16:26       ` Jan Kara
2010-11-10  2:35 ` [PATCH 4/5] writeback: avoid livelocking WB_SYNC_ALL writeback Wu Fengguang
2010-11-10  2:35   ` Wu Fengguang
2010-11-10  2:35   ` Wu Fengguang
2010-11-10  2:35 ` Wu Fengguang [this message]
2010-11-10  2:35   ` [PATCH 5/5] writeback: check skipped pages on WB_SYNC_ALL Wu Fengguang
2010-11-10  2:35   ` Wu Fengguang
  -- strict thread matches above, loose matches on Subject: below --
2010-11-08 23:09 [PATCH 0/5] writeback livelock fixes Wu Fengguang
2010-11-08 23:09 ` [PATCH 5/5] writeback: check skipped pages on WB_SYNC_ALL Wu Fengguang
2010-11-08 23:09   ` Wu Fengguang
2010-11-09 22:47   ` Andrew Morton
2010-11-09 22:47     ` Andrew Morton
2010-11-09 23:16     ` Wu Fengguang
2010-11-09 23:16       ` Wu Fengguang

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=20101110024224.144021908@intel.com \
    --to=fengguang.wu@intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=hannes@cmpxchg.org \
    --cc=jack@suse.cz \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-mm@kvack.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.