Commit 6ffc4db5 authored by Dave Chinner's avatar Dave Chinner Committed by Ben Myers
Browse files

xfs: page type check in writeback only checks last buffer



xfs_is_delayed_page() checks to see if a page has buffers matching
the given IO type passed in. It does so by walking the buffer heads
on the page and checking if the state flags match the IO type.

However, the "acceptable" variable that is calculated is overwritten
every time a new buffer is checked. Hence if the first buffer on the
page is of the right type, this state is lost if the second buffer
is not of the correct type. This means that xfs_aops_discard_page()
may not discard delalloc regions when it is supposed to, and
xfs_convert_page() may not cluster IO as efficiently as possible.

This problem only occurs on filesystems with a block size smaller
than page size.

Also, rename xfs_is_delayed_page() to xfs_check_page_type() to
better describe what it is doing - it is not delalloc specific
anymore.

The problem was first noticed by Peter Watkins.
Signed-off-by: default avatarDave Chinner <dchinner@redhat.com>
Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
Reviewed-by: default avatarMark Tinguely <tinguely@sgi.com>
Signed-off-by: default avatarBen Myers <bpm@sgi.com>
parent 4c2d542f
...@@ -623,7 +623,7 @@ xfs_map_at_offset( ...@@ -623,7 +623,7 @@ xfs_map_at_offset(
* or delayed allocate extent. * or delayed allocate extent.
*/ */
STATIC int STATIC int
xfs_is_delayed_page( xfs_check_page_type(
struct page *page, struct page *page,
unsigned int type) unsigned int type)
{ {
...@@ -637,11 +637,11 @@ xfs_is_delayed_page( ...@@ -637,11 +637,11 @@ xfs_is_delayed_page(
bh = head = page_buffers(page); bh = head = page_buffers(page);
do { do {
if (buffer_unwritten(bh)) if (buffer_unwritten(bh))
acceptable = (type == IO_UNWRITTEN); acceptable += (type == IO_UNWRITTEN);
else if (buffer_delay(bh)) else if (buffer_delay(bh))
acceptable = (type == IO_DELALLOC); acceptable += (type == IO_DELALLOC);
else if (buffer_dirty(bh) && buffer_mapped(bh)) else if (buffer_dirty(bh) && buffer_mapped(bh))
acceptable = (type == IO_OVERWRITE); acceptable += (type == IO_OVERWRITE);
else else
break; break;
} while ((bh = bh->b_this_page) != head); } while ((bh = bh->b_this_page) != head);
...@@ -684,7 +684,7 @@ xfs_convert_page( ...@@ -684,7 +684,7 @@ xfs_convert_page(
goto fail_unlock_page; goto fail_unlock_page;
if (page->mapping != inode->i_mapping) if (page->mapping != inode->i_mapping)
goto fail_unlock_page; goto fail_unlock_page;
if (!xfs_is_delayed_page(page, (*ioendp)->io_type)) if (!xfs_check_page_type(page, (*ioendp)->io_type))
goto fail_unlock_page; goto fail_unlock_page;
/* /*
...@@ -834,7 +834,7 @@ xfs_aops_discard_page( ...@@ -834,7 +834,7 @@ xfs_aops_discard_page(
struct buffer_head *bh, *head; struct buffer_head *bh, *head;
loff_t offset = page_offset(page); loff_t offset = page_offset(page);
if (!xfs_is_delayed_page(page, IO_DELALLOC)) if (!xfs_check_page_type(page, IO_DELALLOC))
goto out_invalidate; goto out_invalidate;
if (XFS_FORCED_SHUTDOWN(ip->i_mount)) if (XFS_FORCED_SHUTDOWN(ip->i_mount))
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment