linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] btrfs: Evaluate io_tree in find_lock_delalloc_range()
@ 2019-06-21 15:02 Goldwyn Rodrigues
  2019-07-01 15:52 ` David Sterba
  0 siblings, 1 reply; 6+ messages in thread
From: Goldwyn Rodrigues @ 2019-06-21 15:02 UTC (permalink / raw)
  To: linux-btrfs

Simplification.
No point passing the tree variable when it can be evaluated
from inode. The tests now use the io_tree from btrfs_inode
as opposed to creating one.

Changes since v1:
 - included btrfs sanity tests

Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
---
 fs/btrfs/extent_io.c             |  6 ++----
 fs/btrfs/extent_io.h             |  2 +-
 fs/btrfs/tests/extent-io-tests.c | 30 ++++++++++++++++--------------
 3 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index db337e53aab3..e9475d7e11bf 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -1719,10 +1719,10 @@ static noinline int lock_delalloc_pages(struct inode *inode,
  */
 EXPORT_FOR_TESTS
 noinline_for_stack bool find_lock_delalloc_range(struct inode *inode,
-				    struct extent_io_tree *tree,
 				    struct page *locked_page, u64 *start,
 				    u64 *end)
 {
+	struct extent_io_tree *tree = &BTRFS_I(inode)->io_tree;
 	u64 max_bytes = BTRFS_MAX_EXTENT_SIZE;
 	u64 delalloc_start;
 	u64 delalloc_end;
@@ -3290,7 +3290,6 @@ static noinline_for_stack int writepage_delalloc(struct inode *inode,
 		struct page *page, struct writeback_control *wbc,
 		u64 delalloc_start, unsigned long *nr_written)
 {
-	struct extent_io_tree *tree = &BTRFS_I(inode)->io_tree;
 	u64 page_end = delalloc_start + PAGE_SIZE - 1;
 	bool found;
 	u64 delalloc_to_write = 0;
@@ -3300,8 +3299,7 @@ static noinline_for_stack int writepage_delalloc(struct inode *inode,
 
 
 	while (delalloc_end < page_end) {
-		found = find_lock_delalloc_range(inode, tree,
-					       page,
+		found = find_lock_delalloc_range(inode, page,
 					       &delalloc_start,
 					       &delalloc_end);
 		if (!found) {
diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
index aa18a16a6ed7..2919c3be1933 100644
--- a/fs/btrfs/extent_io.h
+++ b/fs/btrfs/extent_io.h
@@ -549,7 +549,7 @@ int free_io_failure(struct extent_io_tree *failure_tree,
 		    struct extent_io_tree *io_tree,
 		    struct io_failure_record *rec);
 #ifdef CONFIG_BTRFS_FS_RUN_SANITY_TESTS
-bool find_lock_delalloc_range(struct inode *inode, struct extent_io_tree *tree,
+bool find_lock_delalloc_range(struct inode *inode,
 			     struct page *locked_page, u64 *start,
 			     u64 *end);
 #endif
diff --git a/fs/btrfs/tests/extent-io-tests.c b/fs/btrfs/tests/extent-io-tests.c
index 7bf4d5734dbe..0518aeb4ba95 100644
--- a/fs/btrfs/tests/extent-io-tests.c
+++ b/fs/btrfs/tests/extent-io-tests.c
@@ -10,6 +10,7 @@
 #include "btrfs-tests.h"
 #include "../ctree.h"
 #include "../extent_io.h"
+#include "../btrfs_inode.h"
 
 #define PROCESS_UNLOCK		(1 << 0)
 #define PROCESS_RELEASE		(1 << 1)
@@ -58,7 +59,7 @@ static noinline int process_page_range(struct inode *inode, u64 start, u64 end,
 static int test_find_delalloc(u32 sectorsize)
 {
 	struct inode *inode;
-	struct extent_io_tree tmp;
+	struct extent_io_tree *tmp;
 	struct page *page;
 	struct page *locked_page = NULL;
 	unsigned long index = 0;
@@ -76,12 +77,13 @@ static int test_find_delalloc(u32 sectorsize)
 		test_std_err(TEST_ALLOC_INODE);
 		return -ENOMEM;
 	}
+	tmp = &BTRFS_I(inode)->io_tree;
 
 	/*
 	 * Passing NULL as we don't have fs_info but tracepoints are not used
 	 * at this point
 	 */
-	extent_io_tree_init(NULL, &tmp, IO_TREE_SELFTEST, NULL);
+	extent_io_tree_init(NULL, tmp, IO_TREE_SELFTEST, NULL);
 
 	/*
 	 * First go through and create and mark all of our pages dirty, we pin
@@ -108,10 +110,10 @@ static int test_find_delalloc(u32 sectorsize)
 	 * |--- delalloc ---|
 	 * |---  search  ---|
 	 */
-	set_extent_delalloc(&tmp, 0, sectorsize - 1, 0, NULL);
+	set_extent_delalloc(tmp, 0, sectorsize - 1, 0, NULL);
 	start = 0;
 	end = 0;
-	found = find_lock_delalloc_range(inode, &tmp, locked_page, &start,
+	found = find_lock_delalloc_range(inode, locked_page, &start,
 					 &end);
 	if (!found) {
 		test_err("should have found at least one delalloc");
@@ -122,7 +124,7 @@ static int test_find_delalloc(u32 sectorsize)
 			sectorsize - 1, start, end);
 		goto out_bits;
 	}
-	unlock_extent(&tmp, start, end);
+	unlock_extent(tmp, start, end);
 	unlock_page(locked_page);
 	put_page(locked_page);
 
@@ -139,10 +141,10 @@ static int test_find_delalloc(u32 sectorsize)
 		test_err("couldn't find the locked page");
 		goto out_bits;
 	}
-	set_extent_delalloc(&tmp, sectorsize, max_bytes - 1, 0, NULL);
+	set_extent_delalloc(tmp, sectorsize, max_bytes - 1, 0, NULL);
 	start = test_start;
 	end = 0;
-	found = find_lock_delalloc_range(inode, &tmp, locked_page, &start,
+	found = find_lock_delalloc_range(inode, locked_page, &start,
 					 &end);
 	if (!found) {
 		test_err("couldn't find delalloc in our range");
@@ -158,7 +160,7 @@ static int test_find_delalloc(u32 sectorsize)
 		test_err("there were unlocked pages in the range");
 		goto out_bits;
 	}
-	unlock_extent(&tmp, start, end);
+	unlock_extent(tmp, start, end);
 	/* locked_page was unlocked above */
 	put_page(locked_page);
 
@@ -176,7 +178,7 @@ static int test_find_delalloc(u32 sectorsize)
 	}
 	start = test_start;
 	end = 0;
-	found = find_lock_delalloc_range(inode, &tmp, locked_page, &start,
+	found = find_lock_delalloc_range(inode, locked_page, &start,
 					 &end);
 	if (found) {
 		test_err("found range when we shouldn't have");
@@ -194,10 +196,10 @@ static int test_find_delalloc(u32 sectorsize)
 	 *
 	 * We are re-using our test_start from above since it works out well.
 	 */
-	set_extent_delalloc(&tmp, max_bytes, total_dirty - 1, 0, NULL);
+	set_extent_delalloc(tmp, max_bytes, total_dirty - 1, 0, NULL);
 	start = test_start;
 	end = 0;
-	found = find_lock_delalloc_range(inode, &tmp, locked_page, &start,
+	found = find_lock_delalloc_range(inode, locked_page, &start,
 					 &end);
 	if (!found) {
 		test_err("didn't find our range");
@@ -213,7 +215,7 @@ static int test_find_delalloc(u32 sectorsize)
 		test_err("pages in range were not all locked");
 		goto out_bits;
 	}
-	unlock_extent(&tmp, start, end);
+	unlock_extent(tmp, start, end);
 
 	/*
 	 * Now to test where we run into a page that is no longer dirty in the
@@ -238,7 +240,7 @@ static int test_find_delalloc(u32 sectorsize)
 	 * this changes at any point in the future we will need to fix this
 	 * tests expected behavior.
 	 */
-	found = find_lock_delalloc_range(inode, &tmp, locked_page, &start,
+	found = find_lock_delalloc_range(inode, locked_page, &start,
 					 &end);
 	if (!found) {
 		test_err("didn't find our range");
@@ -256,7 +258,7 @@ static int test_find_delalloc(u32 sectorsize)
 	}
 	ret = 0;
 out_bits:
-	clear_extent_bits(&tmp, 0, total_dirty - 1, (unsigned)-1);
+	clear_extent_bits(tmp, 0, total_dirty - 1, (unsigned)-1);
 out:
 	if (locked_page)
 		put_page(locked_page);
-- 
2.16.4


-- 
Goldwyn

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

* Re: [PATCH] btrfs: Evaluate io_tree in find_lock_delalloc_range()
  2019-06-21 15:02 [PATCH] btrfs: Evaluate io_tree in find_lock_delalloc_range() Goldwyn Rodrigues
@ 2019-07-01 15:52 ` David Sterba
  0 siblings, 0 replies; 6+ messages in thread
From: David Sterba @ 2019-07-01 15:52 UTC (permalink / raw)
  To: Goldwyn Rodrigues; +Cc: linux-btrfs

On Fri, Jun 21, 2019 at 10:02:54AM -0500, Goldwyn Rodrigues wrote:
> Simplification.
> No point passing the tree variable when it can be evaluated
> from inode. The tests now use the io_tree from btrfs_inode
> as opposed to creating one.
> 
> Changes since v1:
>  - included btrfs sanity tests
> 
> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>

Reviewed-by: David Sterba <dsterba@suse.com>

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

* Re: [PATCH] btrfs: Evaluate io_tree in find_lock_delalloc_range()
  2019-06-19 11:36   ` Goldwyn Rodrigues
@ 2019-06-19 13:09     ` David Sterba
  0 siblings, 0 replies; 6+ messages in thread
From: David Sterba @ 2019-06-19 13:09 UTC (permalink / raw)
  To: Goldwyn Rodrigues; +Cc: Nikolay Borisov, linux-btrfs

On Wed, Jun 19, 2019 at 06:36:08AM -0500, Goldwyn Rodrigues wrote:
> On  9:05 19/06, Nikolay Borisov wrote:
> > 
> > 
> > On 19.06.19 г. 3:35 ч., Goldwyn Rodrigues wrote:
> > > Simplification.
> > > No point passing the tree variable when it can be evaluated
> > > from inode.
> > > 
> > > Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
> > 
> > The patch is good, however, there are several calls to find_
> > lock_delalloc_range in btrfs tests so you'd need to fix those 
> > invocations, otherwise compilation of the in-kernel test suite will fails. 
> > 
> > fs/btrfs/tests/extent-io-tests.c:       found = find_lock_delalloc_range(inode, &tmp, locked_page, &start,
> > fs/btrfs/tests/extent-io-tests.c:       found = find_lock_delalloc_range(inode, &tmp, locked_page, &start,
> > fs/btrfs/tests/extent-io-tests.c:       found = find_lock_delalloc_range(inode, &tmp, locked_page, &start,
> > fs/btrfs/tests/extent-io-tests.c:       found = find_lock_delalloc_range(inode, &tmp, locked_page, &start,
> > fs/btrfs/tests/extent-io-tests.c:       found = find_lock_delalloc_range(inode, &tmp, locked_page, &start,
> > 
> > 
> 
> Oh, I missed the test, even if it was listed as EXPORT_FOR_TESTS.
> Looking at the tests, it seems it needs to work on a different io_tree than
> in the inode.

If it's possible/correct to set the test inode tree pointer, then I
guess the change still applies. The testing code works on dummy
or otherwise crafted structures, so this kind of tweak does not sound
off.

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

* Re: [PATCH] btrfs: Evaluate io_tree in find_lock_delalloc_range()
  2019-06-19  6:05 ` Nikolay Borisov
@ 2019-06-19 11:36   ` Goldwyn Rodrigues
  2019-06-19 13:09     ` David Sterba
  0 siblings, 1 reply; 6+ messages in thread
From: Goldwyn Rodrigues @ 2019-06-19 11:36 UTC (permalink / raw)
  To: Nikolay Borisov; +Cc: linux-btrfs

On  9:05 19/06, Nikolay Borisov wrote:
> 
> 
> On 19.06.19 г. 3:35 ч., Goldwyn Rodrigues wrote:
> > Simplification.
> > No point passing the tree variable when it can be evaluated
> > from inode.
> > 
> > Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
> 
> The patch is good, however, there are several calls to find_
> lock_delalloc_range in btrfs tests so you'd need to fix those 
> invocations, otherwise compilation of the in-kernel test suite will fails. 
> 
> fs/btrfs/tests/extent-io-tests.c:       found = find_lock_delalloc_range(inode, &tmp, locked_page, &start,
> fs/btrfs/tests/extent-io-tests.c:       found = find_lock_delalloc_range(inode, &tmp, locked_page, &start,
> fs/btrfs/tests/extent-io-tests.c:       found = find_lock_delalloc_range(inode, &tmp, locked_page, &start,
> fs/btrfs/tests/extent-io-tests.c:       found = find_lock_delalloc_range(inode, &tmp, locked_page, &start,
> fs/btrfs/tests/extent-io-tests.c:       found = find_lock_delalloc_range(inode, &tmp, locked_page, &start,
> 
> 

Oh, I missed the test, even if it was listed as EXPORT_FOR_TESTS.
Looking at the tests, it seems it needs to work on a different io_tree than
in the inode. So, I suppose this patch should NOT be included for the sake
of tests. Perhaps we should use wrapper functions for simplifications.

-- 
Goldwyn

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

* Re: [PATCH] btrfs: Evaluate io_tree in find_lock_delalloc_range()
  2019-06-19  0:35 Goldwyn Rodrigues
@ 2019-06-19  6:05 ` Nikolay Borisov
  2019-06-19 11:36   ` Goldwyn Rodrigues
  0 siblings, 1 reply; 6+ messages in thread
From: Nikolay Borisov @ 2019-06-19  6:05 UTC (permalink / raw)
  To: Goldwyn Rodrigues, linux-btrfs; +Cc: Goldwyn Rodrigues



On 19.06.19 г. 3:35 ч., Goldwyn Rodrigues wrote:
> Simplification.
> No point passing the tree variable when it can be evaluated
> from inode.
> 
> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>

The patch is good, however, there are several calls to find_
lock_delalloc_range in btrfs tests so you'd need to fix those 
invocations, otherwise compilation of the in-kernel test suite will fails. 

fs/btrfs/tests/extent-io-tests.c:       found = find_lock_delalloc_range(inode, &tmp, locked_page, &start,
fs/btrfs/tests/extent-io-tests.c:       found = find_lock_delalloc_range(inode, &tmp, locked_page, &start,
fs/btrfs/tests/extent-io-tests.c:       found = find_lock_delalloc_range(inode, &tmp, locked_page, &start,
fs/btrfs/tests/extent-io-tests.c:       found = find_lock_delalloc_range(inode, &tmp, locked_page, &start,
fs/btrfs/tests/extent-io-tests.c:       found = find_lock_delalloc_range(inode, &tmp, locked_page, &start,


> ---
>  fs/btrfs/extent_io.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index db337e53aab3..e9475d7e11bf 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -1719,10 +1719,10 @@ static noinline int lock_delalloc_pages(struct inode *inode,
>   */
>  EXPORT_FOR_TESTS
>  noinline_for_stack bool find_lock_delalloc_range(struct inode *inode,
> -				    struct extent_io_tree *tree,
>  				    struct page *locked_page, u64 *start,
>  				    u64 *end)
>  {
> +	struct extent_io_tree *tree = &BTRFS_I(inode)->io_tree;
>  	u64 max_bytes = BTRFS_MAX_EXTENT_SIZE;
>  	u64 delalloc_start;
>  	u64 delalloc_end;
> @@ -3290,7 +3290,6 @@ static noinline_for_stack int writepage_delalloc(struct inode *inode,
>  		struct page *page, struct writeback_control *wbc,
>  		u64 delalloc_start, unsigned long *nr_written)
>  {
> -	struct extent_io_tree *tree = &BTRFS_I(inode)->io_tree;
>  	u64 page_end = delalloc_start + PAGE_SIZE - 1;
>  	bool found;
>  	u64 delalloc_to_write = 0;
> @@ -3300,8 +3299,7 @@ static noinline_for_stack int writepage_delalloc(struct inode *inode,
>  
>  
>  	while (delalloc_end < page_end) {
> -		found = find_lock_delalloc_range(inode, tree,
> -					       page,
> +		found = find_lock_delalloc_range(inode, page,
>  					       &delalloc_start,
>  					       &delalloc_end);
>  		if (!found) {
> 

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

* [PATCH] btrfs: Evaluate io_tree in find_lock_delalloc_range()
@ 2019-06-19  0:35 Goldwyn Rodrigues
  2019-06-19  6:05 ` Nikolay Borisov
  0 siblings, 1 reply; 6+ messages in thread
From: Goldwyn Rodrigues @ 2019-06-19  0:35 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Goldwyn Rodrigues, Goldwyn Rodrigues

Simplification.
No point passing the tree variable when it can be evaluated
from inode.

Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
---
 fs/btrfs/extent_io.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index db337e53aab3..e9475d7e11bf 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -1719,10 +1719,10 @@ static noinline int lock_delalloc_pages(struct inode *inode,
  */
 EXPORT_FOR_TESTS
 noinline_for_stack bool find_lock_delalloc_range(struct inode *inode,
-				    struct extent_io_tree *tree,
 				    struct page *locked_page, u64 *start,
 				    u64 *end)
 {
+	struct extent_io_tree *tree = &BTRFS_I(inode)->io_tree;
 	u64 max_bytes = BTRFS_MAX_EXTENT_SIZE;
 	u64 delalloc_start;
 	u64 delalloc_end;
@@ -3290,7 +3290,6 @@ static noinline_for_stack int writepage_delalloc(struct inode *inode,
 		struct page *page, struct writeback_control *wbc,
 		u64 delalloc_start, unsigned long *nr_written)
 {
-	struct extent_io_tree *tree = &BTRFS_I(inode)->io_tree;
 	u64 page_end = delalloc_start + PAGE_SIZE - 1;
 	bool found;
 	u64 delalloc_to_write = 0;
@@ -3300,8 +3299,7 @@ static noinline_for_stack int writepage_delalloc(struct inode *inode,
 
 
 	while (delalloc_end < page_end) {
-		found = find_lock_delalloc_range(inode, tree,
-					       page,
+		found = find_lock_delalloc_range(inode, page,
 					       &delalloc_start,
 					       &delalloc_end);
 		if (!found) {
-- 
2.16.4


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

end of thread, other threads:[~2019-07-01 15:51 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-21 15:02 [PATCH] btrfs: Evaluate io_tree in find_lock_delalloc_range() Goldwyn Rodrigues
2019-07-01 15:52 ` David Sterba
  -- strict thread matches above, loose matches on Subject: below --
2019-06-19  0:35 Goldwyn Rodrigues
2019-06-19  6:05 ` Nikolay Borisov
2019-06-19 11:36   ` Goldwyn Rodrigues
2019-06-19 13:09     ` David Sterba

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).