On 02/28/14 13:22, Brian Foster wrote:
xfs_aops_discard_page() was introduced in the following commit: xfs: truncate delalloc extents when IO fails in writeback ... to clean up left over delalloc ranges after I/O failure in ->writepage(). generic/224 tests for this scenario and occasionally reproduces panics on sub-4k blocksize filesystems. The cause of this is failure to clean up the delalloc range on a page where the first buffer does not match one of the expected states of xfs_check_page_type(). If a buffer is not unwritten, delayed or dirty&mapped, xfs_check_page_type() stops and immediately returns 0. The stress test of generic/224 creates a scenario where the first several buffers of a page with delayed buffers are mapped&uptodate and some subsequent buffer is delayed. If the ->writepage() happens to fail for this page, xfs_aops_discard_page() incorrectly skips the entire page. Modify xfs_aops_discard_page() to iterate all of the page buffers to ensure a delayed buffer does not go undetected. Signed-off-by: Brian Foster<bfoster@xxxxxxxxxx> --- The only other caller to xfs_check_page_type() is xfs_convert_page(). I think this is safe with respect to that codepath, given the additional imap checks therein and whatnot, but thoughts appreciated. Brian fs/xfs/xfs_aops.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c index db2cfb0..5962a9f 100644 --- a/fs/xfs/xfs_aops.c +++ b/fs/xfs/xfs_aops.c @@ -655,8 +655,6 @@ xfs_check_page_type( acceptable += (type == XFS_IO_DELALLOC); else if (buffer_dirty(bh) && buffer_mapped(bh)) acceptable += (type == XFS_IO_OVERWRITE); - else - break; } while ((bh = bh->b_this_page) != head); if (acceptable)
Is there any reason to scan all the buffers when we all we want is an indication that at least one is acceptable? Maybe there are generally not may buffers to a page to make it worthwhile.
--Mark. _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
