Re: [PATCH 7/7] btrfs: Wait for extent bits to release page

[Christoph Hellwig <hch@infradead.org>]

On Fri, Nov 15, 2019 at 10:17:00AM -0600, Goldwyn Rodrigues wrote:
> From: Goldwyn Rodrigues <rgoldwyn@suse.com>
> 
> 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;
 }
 
 /*