linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jan Kara <jack@suse.cz>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Dave Chinner <david@fromorbit.com>,
	linux-kernel@vger.kernel.org, xfs@oss.sgi.com,
	linux-fsdevel@vger.kernel.org, linux-ext4@vger.kernel.org,
	tytso@mit.edu, jens.axboe@oracle.com
Subject: Re: [PATCH 6/6] writeback: limit write_cache_pages integrity scanning to current EOF
Date: Tue, 1 Jun 2010 17:54:17 +0200	[thread overview]
Message-ID: <20100601155417.GA7425@quack.suse.cz> (raw)
In-Reply-To: <20100527143341.d4258798.akpm@linux-foundation.org>

On Thu 27-05-10 14:33:41, Andrew Morton wrote:
> On Tue, 25 May 2010 20:54:12 +1000
> Dave Chinner <david@fromorbit.com> wrote:
> 
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > sync can currently take a really long time if a concurrent writer is
> > extending a file. The problem is that the dirty pages on the address
> > space grow in the same direction as write_cache_pages scans, so if
> > the writer keeps ahead of writeback, the writeback will not
> > terminate until the writer stops adding dirty pages.
> 
> <looks at Jens>
> 
> The really was a pretty basic bug.  It's writeback 101 to test that case :(
  The code has this live-lock since Nick fixed data integrity issues in
write_cache_pages which was (digging) commit 05fe478d ("mm:
write_cache_pages integrity fix") in January 2009. Jens just kept the code
as it was...

...
> That being said, I think the patch is insufficient.  If I create an
> enormous (possibly sparse) file with a 16TB hole (or a run of clean
> pages) in the middle and then start busily writing into that hole (run
> of clean pages), the problem will still occur.
> 
> One obvious fix for that (a) would be to add another radix-tree tag and
> do two passes across the radix-tree.
> 
> Another fix (b) would be to track the number of dirty pages per
> adddress_space, and only write that number of pages.
> 
> Another fix would be to work out how the code handled this situation
> before we broke it, and restore that in some fashion.  I guess fix (b)
> above kinda does that.
  (b) does not work for data integrity sync (see changelog of the above
mentioned commit). I was sending a patch doing (a) in February but in
particular you raised concerns whether it's not too expensive... Since
it indeed has some cost (although I was not able to measure any performance
impact) and I didn't know a better solution, I just postponed the patches.
But I guess it's time to revive the series and maybe we'll get further with
it.

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

      parent reply	other threads:[~2010-06-01 15:54 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-05-25 10:54 [PATCH 0/6] writeback: tracing and fixes Dave Chinner
2010-05-25 10:54 ` [PATCH 1/6] writeback: initial tracing support Dave Chinner
2010-05-25 11:13   ` Christoph Hellwig
2010-05-27 21:32   ` Andrew Morton
2010-05-28  0:44     ` Dave Chinner
2010-05-28  1:20       ` Steven Rostedt
2010-05-28  1:18     ` Steven Rostedt
2010-05-28  7:45     ` Christoph Hellwig
2010-05-25 10:54 ` [PATCH 2/6] writeback: Add tracing to balance_dirty_pages Dave Chinner
2010-05-25 11:13   ` Christoph Hellwig
2010-05-25 10:54 ` [PATCH 3/6] ext4: Use our own write_cache_pages() Dave Chinner
2010-05-25 13:06   ` tytso
2010-05-25 22:42     ` Dave Chinner
2010-05-25 10:54 ` [PATCH 4/6] writeback: pay attention to wbc->nr_to_write in write_cache_pages Dave Chinner
2010-05-25 11:11   ` Christoph Hellwig
2010-05-27 21:32   ` Andrew Morton
2010-05-28  0:56     ` Dave Chinner
2010-05-25 10:54 ` [PATCH 5/6] xfs: remove nr_to_write writeback windup Dave Chinner
2010-05-25 11:14   ` Christoph Hellwig
2010-05-25 10:54 ` [PATCH 6/6] writeback: limit write_cache_pages integrity scanning to current EOF Dave Chinner
2010-05-27 21:33   ` Andrew Morton
2010-05-28  1:23     ` Dave Chinner
2010-05-28  5:06     ` Nick Piggin
2010-06-01 15:54     ` Jan Kara [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=20100601155417.GA7425@quack.suse.cz \
    --to=jack@suse.cz \
    --cc=akpm@linux-foundation.org \
    --cc=david@fromorbit.com \
    --cc=jens.axboe@oracle.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tytso@mit.edu \
    --cc=xfs@oss.sgi.com \
    /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).