Btrfs: make sure we retry if we couldn't get the page
PATCH 1/3 Btrfs: make sure we retry if we couldn't get the page
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.
--- fs/btrfs/inode.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
fs/btrfs/inode.c +3 -1
@@ -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?
Signature · 1 line
1.9.1
PATCH 2/3 Btrfs: make sure we retry if page is a retriable exception
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.
--- fs/btrfs/inode.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
fs/btrfs/inode.c +3 -1
@@ -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
Signature · 1 line
1.9.1
PATCH 3/3 Btrfs: don't release invalid page in btrfs_page_exists_in_range()
In reply to Filipe David Borba MananaIn 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.
--- fs/btrfs/inode.c | 1 + 1 file changed, 1 insertion(+)
fs/btrfs/inode.c +1 -0
@@ -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? */ }
Signature · 1 line
1.9.1