diff mbox series

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

Message ID 20191115161700.12305-8-rgoldwyn@suse.de (mailing list archive)
State New, archived
Headers show
Series [1/7] fs: Export generic_file_buffered_read() | expand

Commit Message

Goldwyn Rodrigues Nov. 15, 2019, 4:17 p.m. UTC 
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.

Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>

---
 fs/btrfs/extent_io.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Christoph Hellwig Nov. 15, 2019, 4:56 p.m. UTC | #1 
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;
 }
 
 /*
diff mbox series

Patch

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index cceaf05aada2..57b37463da48 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -4370,8 +4370,14 @@  static int try_release_extent_state(struct extent_io_tree *tree,
 	int ret = 1;
 
 	if (test_range_bit(tree, start, end, EXTENT_LOCKED, 0, NULL)) {
-		ret = 0;
+		if (gfpflags_allow_blocking(mask)) {
+			wait_extent_bit(tree, start, end, EXTENT_LOCKED);
+			goto clear_bits;
+		} else {
+			ret = 0;
+		}
 	} else {
+clear_bits:
 		/*
 		 * at this point we can safely clear everything except the
 		 * locked bit and the nodatasum bit