[PATCH] xfs: handle negative wbc->nr_to_write during sync writeback

[PATCH] xfs: handle negative wbc->nr_to_write during sync writeback

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

During data integrity (WB_SYNC_ALL) writeback, wbc->nr_to_write will
go negative on inodes with more than 1024 dirty pages due to
implementation details of write_cache_pages(). Currently XFS will
abort page clustering in writeback once nr_to_write drops below
zero, and so for data integrity writeback we will do very
inefficient page at a time allocation and IO submission for inodes
with large numbers of dirty pages.

Fix this by only aborting the page clustering code when
wbc->nr_to_write is negative and the sync mode is WB_SYNC_NONE.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/linux-2.6/xfs_aops.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/linux-2.6/xfs_aops.c b/fs/xfs/linux-2.6/xfs_aops.c
index 15412fe..528be1b 100644
--- a/fs/xfs/linux-2.6/xfs_aops.c
+++ b/fs/xfs/linux-2.6/xfs_aops.c
@@ -852,8 +852,8 @@ xfs_convert_page(
 		SetPageUptodate(page);
 
 	if (count) {
-		wbc->nr_to_write--;
-		if (wbc->nr_to_write <= 0)
+		if (--wbc->nr_to_write <= 0 &&
+		    wbc->sync_mode == WB_SYNC_NONE)
 			done = 1;
 	}
 	xfs_start_page_writeback(page, !page_dirty, count);
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs: handle negative wbc->nr_to_write during sync writeback

[Christoph Hellwig]

On Mon, Aug 23, 2010 at 10:10:37PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> During data integrity (WB_SYNC_ALL) writeback, wbc->nr_to_write will
> go negative on inodes with more than 1024 dirty pages due to
> implementation details of write_cache_pages(). Currently XFS will
> abort page clustering in writeback once nr_to_write drops below
> zero, and so for data integrity writeback we will do very
> inefficient page at a time allocation and IO submission for inodes
> with large numbers of dirty pages.
> 
> Fix this by only aborting the page clustering code when
> wbc->nr_to_write is negative and the sync mode is WB_SYNC_NONE.

Looks okay, but I wonder why if we should remove the check entirely.
We're clustering inside the same extent, and we already have a cap
for the extent size.  That means the additional I/O basically is for
free and we really shouldn't let the writeback code restrict us here.

Reviewed-by: Christoph Hellwig <hch@lst.de>

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs