All of lore.kernel.org
 help / color / mirror / Atom feed
From: Wu Fengguang <fengguang.wu@intel.com>
To: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Jan Kara <jack@suse.cz>, Dave Chinner <david@fromorbit.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: [PATCH] writeback: fix BDI_WRITTEN accounting disturbing bdi->completions
Date: Thu, 1 Sep 2011 22:40:41 +0800	[thread overview]
Message-ID: <20110901144041.GA5681@localhost> (raw)

[-- Attachment #1: Type: text/plain, Size: 1459 bytes --]

Peter,

This is an amazing bug. I'm not sure how the accounting goes wrong in
some tricky way. But you can compare the exact bdi proportions pattern
before/after patch. The gray "bdi setpoint" lines are vastly different.

---
When increasing BDI_WRITTEN together with bdi->completions inside the
same local irq disabling block, bdi_thresh is found to go wild in the
1 disk + 1 usb stick writeback test case. Fix it by moving BDI_WRITTEN
accounting out.

Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
---
 mm/page-writeback.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

--- linux.orig/mm/page-writeback.c	2011-09-01 21:43:45.312000020 +0800
+++ linux/mm/page-writeback.c	2011-09-01 21:46:10.656000006 +0800
@@ -230,7 +230,6 @@ int dirty_bytes_handler(struct ctl_table
  */
 static inline void __bdi_writeout_inc(struct backing_dev_info *bdi)
 {
-	__inc_bdi_stat(bdi, BDI_WRITTEN);
 	__prop_inc_percpu_max(&vm_completions, &bdi->completions,
 			      bdi->max_prop_frac);
 }
@@ -239,6 +238,7 @@ void bdi_writeout_inc(struct backing_dev
 {
 	unsigned long flags;
 
+	inc_bdi_stat(bdi, BDI_WRITTEN);
 	local_irq_save(flags);
 	__bdi_writeout_inc(bdi);
 	local_irq_restore(flags);
@@ -1577,6 +1577,7 @@ int test_clear_page_writeback(struct pag
 						PAGECACHE_TAG_WRITEBACK);
 			if (bdi_cap_account_writeback(bdi)) {
 				__dec_bdi_stat(bdi, BDI_WRITEBACK);
+				__inc_bdi_stat(bdi, BDI_WRITTEN);
 				__bdi_writeout_inc(bdi);
 			}
 		}

[-- Attachment #2: xfs-1dd-4k-8p-2948M-20:10-3.1.0-rc1+balance_dirty_pages-pages.png --]
[-- Type: image/png, Size: 82188 bytes --]

[-- Attachment #3: xfs-1dd-4k-8p-2948M-20:10-3.1.0-rc4-fixed+balance_dirty_pages-pages.png --]
[-- Type: image/png, Size: 55013 bytes --]

             reply	other threads:[~2011-09-01 14:40 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-09-01 14:40 Wu Fengguang [this message]
2011-09-01 16:29 ` [PATCH] writeback: fix BDI_WRITTEN accounting disturbing bdi->completions Jan Kara
2011-09-02  2:52   ` 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=20110901144041.GA5681@localhost \
    --to=fengguang.wu@intel.com \
    --cc=a.p.zijlstra@chello.nl \
    --cc=akpm@linux-foundation.org \
    --cc=david@fromorbit.com \
    --cc=jack@suse.cz \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.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.