All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] Btrfs: make sure we retry if we couldn't get the page
@ 2014-06-05 12:22 Filipe David Borba Manana
  2014-06-05 12:22 ` [PATCH 2/3] Btrfs: make sure we retry if page is a retriable exception Filipe David Borba Manana
  2014-06-05 12:22 ` [PATCH 3/3] Btrfs: don't release invalid page in btrfs_page_exists_in_range() Filipe David Borba Manana
  0 siblings, 2 replies; 3+ messages in thread
From: Filipe David Borba Manana @ 2014-06-05 12:22 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Filipe David Borba Manana

In inode.c:btrfs_page_exists_in_range(), if we can't get the page
we need to retry. However we weren't retrying because we weren't
setting page to NULL, which makes the while loop exit immediately
and will make us call page_cache_release after exiting the loop
which is incorrect because our page get didn't succeed. This could
also make us return true when we shouldn't.

Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
---
 fs/btrfs/inode.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 38d1e7b..cdbd20e 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -6777,8 +6777,10 @@ bool btrfs_page_exists_in_range(struct inode *inode, loff_t start, loff_t end)
 			break; /* TODO: Is this relevant for this use case? */
 		}
 
-		if (!page_cache_get_speculative(page))
+		if (!page_cache_get_speculative(page)) {
+			page = NULL;
 			continue;
+		}
 
 		/*
 		 * Has the page moved?
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH 2/3] Btrfs: make sure we retry if page is a retriable exception
  2014-06-05 12:22 [PATCH 1/3] Btrfs: make sure we retry if we couldn't get the page Filipe David Borba Manana
@ 2014-06-05 12:22 ` Filipe David Borba Manana
  2014-06-05 12:22 ` [PATCH 3/3] Btrfs: don't release invalid page in btrfs_page_exists_in_range() Filipe David Borba Manana
  1 sibling, 0 replies; 3+ messages in thread
From: Filipe David Borba Manana @ 2014-06-05 12:22 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Filipe David Borba Manana

In inode.c:btrfs_page_exists_in_range(), if the page we get from the
radix tree is an exception which should make us retry, set page to
NULL in order to really retry, because otherwise we don't get another
loop iteration executed (page != NULL makes the while loop exit).
This also was making us call page_cache_release after exiting the loop,
which isn't correct because page doesn't point to a valid page, and
possibly return true from the function when we shouldn't.

Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
---
 fs/btrfs/inode.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index cdbd20e..f265f41 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -6767,8 +6767,10 @@ bool btrfs_page_exists_in_range(struct inode *inode, loff_t start, loff_t end)
 			break;
 
 		if (radix_tree_exception(page)) {
-			if (radix_tree_deref_retry(page))
+			if (radix_tree_deref_retry(page)) {
+				page = NULL;
 				continue;
+			}
 			/*
 			 * Otherwise, shmem/tmpfs must be storing a swap entry
 			 * here as an exceptional entry: so return it without
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH 3/3] Btrfs: don't release invalid page in btrfs_page_exists_in_range()
  2014-06-05 12:22 [PATCH 1/3] Btrfs: make sure we retry if we couldn't get the page Filipe David Borba Manana
  2014-06-05 12:22 ` [PATCH 2/3] Btrfs: make sure we retry if page is a retriable exception Filipe David Borba Manana
@ 2014-06-05 12:22 ` Filipe David Borba Manana
  1 sibling, 0 replies; 3+ messages in thread
From: Filipe David Borba Manana @ 2014-06-05 12:22 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Filipe David Borba Manana

In inode.c:btrfs_page_exists_in_range(), if the page we got from
the radix tree is an exception entry, which can't be retried, we
exit the loop with a non-NULL page and then call page_cache_release
against it, which is not ok since it's not a valid page. This could
also make us return true when we shouldn't.

Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
---
 fs/btrfs/inode.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index f265f41..477e64a 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -6776,6 +6776,7 @@ bool btrfs_page_exists_in_range(struct inode *inode, loff_t start, loff_t end)
 			 * here as an exceptional entry: so return it without
 			 * attempting to raise page count.
 			 */
+			page = NULL;
 			break; /* TODO: Is this relevant for this use case? */
 		}
 
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2014-06-05 11:24 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-05 12:22 [PATCH 1/3] Btrfs: make sure we retry if we couldn't get the page Filipe David Borba Manana
2014-06-05 12:22 ` [PATCH 2/3] Btrfs: make sure we retry if page is a retriable exception Filipe David Borba Manana
2014-06-05 12:22 ` [PATCH 3/3] Btrfs: don't release invalid page in btrfs_page_exists_in_range() Filipe David Borba Manana

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.