On Fri, Nov 15, 2019 at 10:17:00AM -0600, Goldwyn Rodrigues wrote:
> From: Goldwyn Rodrigues <rgoldwyn@xxxxxxxx>
>
> While trying to release a page, the extent containing the page may be locked
> which would stop the page from being released. Wait for the
> extent lock to be cleared, if blocking is allowed and then clear
> the bits.
>
> This is avoid warnings coming iomap->dio_rw() ->
> invalidate_inode_pages2_range() -> invalidate_complete_page2() ->
> try_to_release_page() results in stale pagecache warning.
I can't really comment on the technical aspects of this patch as I don't
know btrfs well enough for that, but try_release_extent_state already
is written in a rather convoluted way, and the additions in this patch
don't make that any better.
I think we want something like the version below. I can also send you
a patch with just the cleanup bits, so that you can add the actual
changes on top.
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index cceaf05aada2..4dc4b4c57d7c 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -4367,28 +4367,24 @@ static int try_release_extent_state(struct extent_io_tree *tree,
{
u64 start = page_offset(page);
u64 end = start + PAGE_SIZE - 1;
- int ret = 1;
if (test_range_bit(tree, start, end, EXTENT_LOCKED, 0, NULL)) {
- ret = 0;
- } else {
- /*
- * at this point we can safely clear everything except the
- * locked bit and the nodatasum bit
- */
- ret = __clear_extent_bit(tree, start, end,
- ~(EXTENT_LOCKED | EXTENT_NODATASUM),
- 0, 0, NULL, mask, NULL);
-
- /* if clear_extent_bit failed for enomem reasons,
- * we can't allow the release to continue.
- */
- if (ret < 0)
- ret = 0;
- else
- ret = 1;
+ if (!gfpflags_allow_blocking(mask))
+ return 0;
+ wait_extent_bit(tree, start, end, EXTENT_LOCKED);
}
- return ret;
+
+ /*
+ * At this point we can safely clear everything except the locked and
+ * nodatasum bits. But if clear_extent_bit failed because we are out
+ * of memory, we can't allow the release to continue.
+ */
+ if (__clear_extent_bit(tree, start, end,
+ ~(EXTENT_LOCKED | EXTENT_NODATASUM), 0, 0, NULL,
+ mask, NULL) < 0)
+ return 0;
+
+ return 1;
}
/*
