[PATCH] xfs: remove incorrect assert in xfs_vm_writepage

[PATCH] xfs: remove incorrect assert in xfs_vm_writepage

[Christoph Hellwig]

In commit 20cb52ebd1b5ca6fa8a5d9b6b1392292f5ca8a45, titled
"xfs: simplify xfs_vm_writepage" I added an assert that any !mapped and
uptodate buffers are not dirty.  That asserts turns out to trigger a lot
when running fsx on filesystems with small block sizes.  The reason for
that is that the assert is simply incorrect.  !mapped and uptodate
just mean this buffer covers a hole, and whenever we do a set_page_dirty
we mark all blocks in the page dirty, no matter if they have data or
not.  So remove the assert, and update the comment above the condition
to match reality.

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

Index: xfs/fs/xfs/linux-2.6/xfs_aops.c
===================================================================
--- xfs.orig/fs/xfs/linux-2.6/xfs_aops.c	2010-11-10 16:56:00.464863012 +0100
+++ xfs/fs/xfs/linux-2.6/xfs_aops.c	2010-11-10 17:00:23.528196346 +0100
@@ -1111,11 +1111,12 @@ xfs_vm_writepage(
 			uptodate = 0;
 
 		/*
-		 * A hole may still be marked uptodate because discard_buffer
-		 * leaves the flag set.
+		 * set_page_dirty dirties all buffers in a page, independent
+		 * of their state.  The dirty state however is entirely
+		 * meaningless for holes (!mapped && uptodate), so skip
+		 * buffers covering holes here.
 		 */
 		if (!buffer_mapped(bh) && buffer_uptodate(bh)) {
-			ASSERT(!buffer_dirty(bh));
 			imap_valid = 0;
 			continue;
 		}

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

Re: [PATCH] xfs: remove incorrect assert in xfs_vm_writepage

[Alex Elder]

On Wed, 2010-11-10 at 16:39 -0500, Christoph Hellwig wrote:
> In commit 20cb52ebd1b5ca6fa8a5d9b6b1392292f5ca8a45, titled
> "xfs: simplify xfs_vm_writepage" I added an assert that any !mapped
> and
> uptodate buffers are not dirty.  That asserts turns out to trigger a
> lot
> when running fsx on filesystems with small block sizes.  The reason
> for
> that is that the assert is simply incorrect.  !mapped and uptodate
> just mean this buffer covers a hole, and whenever we do a
> set_page_dirty
> we mark all blocks in the page dirty, no matter if they have data or
> not.  So remove the assert, and update the comment above the condition
> to match reality.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good.

Reviewed-by: Alex Elder <aelder@sgi.com>


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