Commit 793d7dbe authored by Dave Chinner's avatar Dave Chinner Committed by Darrick J. Wong

xfs: cancel dirty pages on invalidation

Recently we've had warnings arise from the vm handing us pages
without bufferheads attached to them. This should not ever occur
in XFS, but we don't defend against it properly if it does. The only
place where we remove bufferheads from a page is in
xfs_vm_releasepage(), but we can't tell the difference here between
"page is dirty so don't release" and "page is dirty but is being
invalidated so release it".

In some places that are invalidating pages ask for pages to be
released and follow up afterward calling ->releasepage by checking
whether the page was dirty and then aborting the invalidation. This
is a possible vector for releasing buffers from a page but then
leaving it in the mapping, so we really do need to avoid dirty pages
in xfs_vm_releasepage().

To differentiate between invalidated pages and normal pages, we need
to clear the page dirty flag when invalidating the pages. This can
be done through xfs_vm_invalidatepage(), and will result
xfs_vm_releasepage() seeing the page as clean which matches the
bufferhead state on the page after calling block_invalidatepage().

Hence we can re-add the page dirty check in xfs_vm_releasepage to
catch the case where we might be releasing a page that is actually
dirty and so should not have the bufferheads on it removed. This
will remove one possible vector of "dirty page with no bufferheads"
and so help narrow down the search for the root cause of that
problem.
Signed-Off-By: default avatarDave Chinner <dchinner@redhat.com>
Reviewed-by: default avatarDarrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: default avatarDarrick J. Wong <darrick.wong@oracle.com>
parent 33d930e5
...@@ -735,6 +735,14 @@ xfs_vm_invalidatepage( ...@@ -735,6 +735,14 @@ xfs_vm_invalidatepage(
{ {
trace_xfs_invalidatepage(page->mapping->host, page, offset, trace_xfs_invalidatepage(page->mapping->host, page, offset,
length); length);
/*
* If we are invalidating the entire page, clear the dirty state from it
* so that we can check for attempts to release dirty cached pages in
* xfs_vm_releasepage().
*/
if (offset == 0 && length >= PAGE_SIZE)
cancel_dirty_page(page);
block_invalidatepage(page, offset, length); block_invalidatepage(page, offset, length);
} }
...@@ -1190,25 +1198,27 @@ xfs_vm_releasepage( ...@@ -1190,25 +1198,27 @@ xfs_vm_releasepage(
* mm accommodates an old ext3 case where clean pages might not have had * mm accommodates an old ext3 case where clean pages might not have had
* the dirty bit cleared. Thus, it can send actual dirty pages to * the dirty bit cleared. Thus, it can send actual dirty pages to
* ->releasepage() via shrink_active_list(). Conversely, * ->releasepage() via shrink_active_list(). Conversely,
* block_invalidatepage() can send pages that are still marked dirty * block_invalidatepage() can send pages that are still marked dirty but
* but otherwise have invalidated buffers. * otherwise have invalidated buffers.
* *
* We want to release the latter to avoid unnecessary buildup of the * We want to release the latter to avoid unnecessary buildup of the
* LRU, skip the former and warn if we've left any lingering * LRU, so xfs_vm_invalidatepage() clears the page dirty flag on pages
* delalloc/unwritten buffers on clean pages. Skip pages with delalloc * that are entirely invalidated and need to be released. Hence the
* or unwritten buffers and warn if the page is not dirty. Otherwise * only time we should get dirty pages here is through
* try to release the buffers. * shrink_active_list() and so we can simply skip those now.
*
* warn if we've left any lingering delalloc/unwritten buffers on clean
* or invalidated pages we are about to release.
*/ */
if (PageDirty(page))
return 0;
xfs_count_page_state(page, &delalloc, &unwritten); xfs_count_page_state(page, &delalloc, &unwritten);
if (delalloc) { if (WARN_ON_ONCE(delalloc))
WARN_ON_ONCE(!PageDirty(page));
return 0; return 0;
} if (WARN_ON_ONCE(unwritten))
if (unwritten) {
WARN_ON_ONCE(!PageDirty(page));
return 0; return 0;
}
return try_to_free_buffers(page); return try_to_free_buffers(page);
} }
......
Markdown is supported
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