All of lore.kernel.org
 help / color / mirror / Atom feed
From: Wu Fengguang <fengguang.wu@intel.com>
To: Jan Kara <jack@suse.cz>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>,
	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: Re: [PATCH] writeback: fix BDI_WRITTEN accounting disturbing bdi->completions
Date: Fri, 2 Sep 2011 10:52:55 +0800	[thread overview]
Message-ID: <20110902025255.GA28584@localhost> (raw)
In-Reply-To: <20110901162950.GD2070@quack.suse.cz>

On Fri, Sep 02, 2011 at 12:29:50AM +0800, Jan Kara wrote:
> On Thu 01-09-11 22:40:41, Wu Fengguang wrote:
> > 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.
>   I don't understand this - the patch is just NOP. The change in
> test_clear_page_writeback() does absolutely nothing and the change in
> bdi_writeout_inc() just changes:
>   	local_irq_save(flags);
>   	__inc_bdi_stat(bdi);
>   	__prop_inc_percpu_max(&vm_completions, &bdi->completions,
>   			      bdi->max_prop_frac);
>   	local_irq_restore(flags);
> 
> to:
>   	local_irq_save(flags);
>   	__inc_bdi_stat(bdi);
>   	local_irq_restore(flags);
>   	local_irq_save(flags);
>   	__prop_inc_percpu_max(&vm_completions, &bdi->completions,
>   			      bdi->max_prop_frac);
>   	local_irq_restore(flags);
> 
> So the difference must be in something else...

Yes you are right -- this patch is basically a NOP. I retried without
this patch and it's now working fine again. Something mysterious to
catch..

Thanks,
Fengguang

      reply	other threads:[~2011-09-02  2:53 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-09-01 14:40 [PATCH] writeback: fix BDI_WRITTEN accounting disturbing bdi->completions Wu Fengguang
2011-09-01 16:29 ` Jan Kara
2011-09-02  2:52   ` Wu Fengguang [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=20110902025255.GA28584@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.