* [PATCH 1/3] btrfs: remove always true if branch in find_delalloc_range @ 2018-11-28 3:21 Lu Fengqi 2018-11-28 3:22 ` [PATCH 2/3] btrfs: cleanup the useless DEFINE_WAIT in cleanup_transaction Lu Fengqi ` (3 more replies) 0 siblings, 4 replies; 14+ messages in thread From: Lu Fengqi @ 2018-11-28 3:21 UTC (permalink / raw) To: linux-btrfs The @found is always false when it comes to the if branch. Besides, the bool type is more suitable for @found. Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com> --- fs/btrfs/extent_io.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c index 582b4b1c41e0..b4ee3399be96 100644 --- a/fs/btrfs/extent_io.c +++ b/fs/btrfs/extent_io.c @@ -1461,7 +1461,7 @@ static noinline u64 find_delalloc_range(struct extent_io_tree *tree, struct rb_node *node; struct extent_state *state; u64 cur_start = *start; - u64 found = 0; + bool found = false; u64 total_bytes = 0; spin_lock(&tree->lock); @@ -1472,8 +1472,7 @@ static noinline u64 find_delalloc_range(struct extent_io_tree *tree, */ node = tree_search(tree, cur_start); if (!node) { - if (!found) - *end = (u64)-1; + *end = (u64)-1; goto out; } @@ -1493,7 +1492,7 @@ static noinline u64 find_delalloc_range(struct extent_io_tree *tree, *cached_state = state; refcount_inc(&state->refs); } - found++; + found = true; *end = state->end; cur_start = state->end + 1; node = rb_next(node); -- 2.19.2 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/3] btrfs: cleanup the useless DEFINE_WAIT in cleanup_transaction 2018-11-28 3:21 [PATCH 1/3] btrfs: remove always true if branch in find_delalloc_range Lu Fengqi @ 2018-11-28 3:22 ` Lu Fengqi 2018-11-28 7:02 ` Nikolay Borisov ` (2 more replies) 2018-11-28 3:23 ` [PATCH 3/3] btrfs: remove redundant nowait check for buffered_write Lu Fengqi ` (2 subsequent siblings) 3 siblings, 3 replies; 14+ messages in thread From: Lu Fengqi @ 2018-11-28 3:22 UTC (permalink / raw) To: linux-btrfs When it is introduced at commit f094ac32aba3 ("Btrfs: fix NULL pointer after aborting a transaction"), it's useless. Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com> --- fs/btrfs/transaction.c | 1 - 1 file changed, 1 deletion(-) diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c index f92c0a88c4ad..67e84939b758 100644 --- a/fs/btrfs/transaction.c +++ b/fs/btrfs/transaction.c @@ -1840,7 +1840,6 @@ static void cleanup_transaction(struct btrfs_trans_handle *trans, int err) { struct btrfs_fs_info *fs_info = trans->fs_info; struct btrfs_transaction *cur_trans = trans->transaction; - DEFINE_WAIT(wait); WARN_ON(refcount_read(&trans->use_count) > 1); -- 2.19.2 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] btrfs: cleanup the useless DEFINE_WAIT in cleanup_transaction 2018-11-28 3:22 ` [PATCH 2/3] btrfs: cleanup the useless DEFINE_WAIT in cleanup_transaction Lu Fengqi @ 2018-11-28 7:02 ` Nikolay Borisov 2018-11-28 13:34 ` Johannes Thumshirn 2018-11-28 16:24 ` David Sterba 2 siblings, 0 replies; 14+ messages in thread From: Nikolay Borisov @ 2018-11-28 7:02 UTC (permalink / raw) To: Lu Fengqi, linux-btrfs On 28.11.18 г. 5:22 ч., Lu Fengqi wrote: > When it is introduced at commit f094ac32aba3 ("Btrfs: fix NULL pointer > after aborting a transaction"), it's useless. > > Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com> Reviewed-by: Nikolay Borisov <nborisov@suse.com> > --- > fs/btrfs/transaction.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c > index f92c0a88c4ad..67e84939b758 100644 > --- a/fs/btrfs/transaction.c > +++ b/fs/btrfs/transaction.c > @@ -1840,7 +1840,6 @@ static void cleanup_transaction(struct btrfs_trans_handle *trans, int err) > { > struct btrfs_fs_info *fs_info = trans->fs_info; > struct btrfs_transaction *cur_trans = trans->transaction; > - DEFINE_WAIT(wait); > > WARN_ON(refcount_read(&trans->use_count) > 1); > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] btrfs: cleanup the useless DEFINE_WAIT in cleanup_transaction 2018-11-28 3:22 ` [PATCH 2/3] btrfs: cleanup the useless DEFINE_WAIT in cleanup_transaction Lu Fengqi 2018-11-28 7:02 ` Nikolay Borisov @ 2018-11-28 13:34 ` Johannes Thumshirn 2018-11-28 16:24 ` David Sterba 2 siblings, 0 replies; 14+ messages in thread From: Johannes Thumshirn @ 2018-11-28 13:34 UTC (permalink / raw) To: Lu Fengqi, linux-btrfs Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de> -- Johannes Thumshirn SUSE Labs Filesystems jthumshirn@suse.de +49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] btrfs: cleanup the useless DEFINE_WAIT in cleanup_transaction 2018-11-28 3:22 ` [PATCH 2/3] btrfs: cleanup the useless DEFINE_WAIT in cleanup_transaction Lu Fengqi 2018-11-28 7:02 ` Nikolay Borisov 2018-11-28 13:34 ` Johannes Thumshirn @ 2018-11-28 16:24 ` David Sterba 2 siblings, 0 replies; 14+ messages in thread From: David Sterba @ 2018-11-28 16:24 UTC (permalink / raw) To: Lu Fengqi; +Cc: linux-btrfs On Wed, Nov 28, 2018 at 11:22:54AM +0800, Lu Fengqi wrote: > When it is introduced at commit f094ac32aba3 ("Btrfs: fix NULL pointer > after aborting a transaction"), it's useless. > > Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com> Added to misc-next, thanks. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/3] btrfs: remove redundant nowait check for buffered_write 2018-11-28 3:21 [PATCH 1/3] btrfs: remove always true if branch in find_delalloc_range Lu Fengqi 2018-11-28 3:22 ` [PATCH 2/3] btrfs: cleanup the useless DEFINE_WAIT in cleanup_transaction Lu Fengqi @ 2018-11-28 3:23 ` Lu Fengqi 2018-11-28 7:04 ` Nikolay Borisov 2018-11-28 13:36 ` Johannes Thumshirn 2018-11-28 7:01 ` [PATCH 1/3] btrfs: remove always true if branch in find_delalloc_range Nikolay Borisov 2018-11-29 3:33 ` [PATCH v2 " Lu Fengqi 3 siblings, 2 replies; 14+ messages in thread From: Lu Fengqi @ 2018-11-28 3:23 UTC (permalink / raw) To: linux-btrfs The generic_write_checks will check the combination of IOCB_NOWAIT and !IOCB_DIRECT. Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com> --- fs/btrfs/file.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c index 3835bb8c146d..190db9a685a2 100644 --- a/fs/btrfs/file.c +++ b/fs/btrfs/file.c @@ -1889,10 +1889,6 @@ static ssize_t btrfs_file_write_iter(struct kiocb *iocb, loff_t oldsize; int clean_page = 0; - if (!(iocb->ki_flags & IOCB_DIRECT) && - (iocb->ki_flags & IOCB_NOWAIT)) - return -EOPNOTSUPP; - if (!inode_trylock(inode)) { if (iocb->ki_flags & IOCB_NOWAIT) return -EAGAIN; -- 2.19.2 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] btrfs: remove redundant nowait check for buffered_write 2018-11-28 3:23 ` [PATCH 3/3] btrfs: remove redundant nowait check for buffered_write Lu Fengqi @ 2018-11-28 7:04 ` Nikolay Borisov 2018-11-28 16:20 ` David Sterba 2018-11-28 13:36 ` Johannes Thumshirn 1 sibling, 1 reply; 14+ messages in thread From: Nikolay Borisov @ 2018-11-28 7:04 UTC (permalink / raw) To: Lu Fengqi, linux-btrfs On 28.11.18 г. 5:23 ч., Lu Fengqi wrote: > The generic_write_checks will check the combination of IOCB_NOWAIT and > !IOCB_DIRECT. True, however btrfs will return ENOSUPP whereas the generic code returns EINVAL. I guess this is not a big deal and it's likely generic code is correct, so: Reviewed-by: Nikolay Borisov <nborisov@suse.com> > > Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com> > --- > fs/btrfs/file.c | 4 ---- > 1 file changed, 4 deletions(-) > > diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c > index 3835bb8c146d..190db9a685a2 100644 > --- a/fs/btrfs/file.c > +++ b/fs/btrfs/file.c > @@ -1889,10 +1889,6 @@ static ssize_t btrfs_file_write_iter(struct kiocb *iocb, > loff_t oldsize; > int clean_page = 0; > > - if (!(iocb->ki_flags & IOCB_DIRECT) && > - (iocb->ki_flags & IOCB_NOWAIT)) > - return -EOPNOTSUPP; > - > if (!inode_trylock(inode)) { > if (iocb->ki_flags & IOCB_NOWAIT) > return -EAGAIN; > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] btrfs: remove redundant nowait check for buffered_write 2018-11-28 7:04 ` Nikolay Borisov @ 2018-11-28 16:20 ` David Sterba 0 siblings, 0 replies; 14+ messages in thread From: David Sterba @ 2018-11-28 16:20 UTC (permalink / raw) To: Nikolay Borisov; +Cc: Lu Fengqi, linux-btrfs On Wed, Nov 28, 2018 at 09:04:31AM +0200, Nikolay Borisov wrote: > On 28.11.18 г. 5:23 ч., Lu Fengqi wrote: > > The generic_write_checks will check the combination of IOCB_NOWAIT and > > !IOCB_DIRECT. > > True, however btrfs will return ENOSUPP whereas the generic code returns > EINVAL. I guess this is not a big deal and it's likely generic code is > correct, so: I tried to check the git history if there are some clues for that. The duplicate checks could be a intermediate step before the aio/nowait feature was complete but somehow not removed at the end. The btrfs part was Added in commit 91f9943e1c7b6638f27312d03fe71fcc67b23571 "fs: support RWF_NOWAIT for buffered reads" (4.13) The generic checks were added in 6be96d3ad34a124450028dabba43f07fe1d0c86d (4.12), so from that it seems that it was added tot btrfs as redundant already. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] btrfs: remove redundant nowait check for buffered_write 2018-11-28 3:23 ` [PATCH 3/3] btrfs: remove redundant nowait check for buffered_write Lu Fengqi 2018-11-28 7:04 ` Nikolay Borisov @ 2018-11-28 13:36 ` Johannes Thumshirn 1 sibling, 0 replies; 14+ messages in thread From: Johannes Thumshirn @ 2018-11-28 13:36 UTC (permalink / raw) To: Lu Fengqi, linux-btrfs Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de> -- Johannes Thumshirn SUSE Labs Filesystems jthumshirn@suse.de +49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] btrfs: remove always true if branch in find_delalloc_range 2018-11-28 3:21 [PATCH 1/3] btrfs: remove always true if branch in find_delalloc_range Lu Fengqi 2018-11-28 3:22 ` [PATCH 2/3] btrfs: cleanup the useless DEFINE_WAIT in cleanup_transaction Lu Fengqi 2018-11-28 3:23 ` [PATCH 3/3] btrfs: remove redundant nowait check for buffered_write Lu Fengqi @ 2018-11-28 7:01 ` Nikolay Borisov 2018-11-28 9:33 ` Lu Fengqi 2018-11-29 3:33 ` [PATCH v2 " Lu Fengqi 3 siblings, 1 reply; 14+ messages in thread From: Nikolay Borisov @ 2018-11-28 7:01 UTC (permalink / raw) To: Lu Fengqi, linux-btrfs On 28.11.18 г. 5:21 ч., Lu Fengqi wrote: > The @found is always false when it comes to the if branch. Besides, the > bool type is more suitable for @found. Well if you are ranging the type of found variable it also makes sense to change the return value of the function to bool as well. > > Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com> > --- > fs/btrfs/extent_io.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c > index 582b4b1c41e0..b4ee3399be96 100644 > --- a/fs/btrfs/extent_io.c > +++ b/fs/btrfs/extent_io.c > @@ -1461,7 +1461,7 @@ static noinline u64 find_delalloc_range(struct extent_io_tree *tree, > struct rb_node *node; > struct extent_state *state; > u64 cur_start = *start; > - u64 found = 0; > + bool found = false; > u64 total_bytes = 0; > > spin_lock(&tree->lock); > @@ -1472,8 +1472,7 @@ static noinline u64 find_delalloc_range(struct extent_io_tree *tree, > */ > node = tree_search(tree, cur_start); > if (!node) { > - if (!found) > - *end = (u64)-1; > + *end = (u64)-1; > goto out; > } > > @@ -1493,7 +1492,7 @@ static noinline u64 find_delalloc_range(struct extent_io_tree *tree, > *cached_state = state; > refcount_inc(&state->refs); > } > - found++; > + found = true; > *end = state->end; > cur_start = state->end + 1; > node = rb_next(node); > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] btrfs: remove always true if branch in find_delalloc_range 2018-11-28 7:01 ` [PATCH 1/3] btrfs: remove always true if branch in find_delalloc_range Nikolay Borisov @ 2018-11-28 9:33 ` Lu Fengqi 0 siblings, 0 replies; 14+ messages in thread From: Lu Fengqi @ 2018-11-28 9:33 UTC (permalink / raw) To: Nikolay Borisov; +Cc: linux-btrfs On Wed, Nov 28, 2018 at 09:01:42AM +0200, Nikolay Borisov wrote: > > >On 28.11.18 г. 5:21 ч., Lu Fengqi wrote: >> The @found is always false when it comes to the if branch. Besides, the >> bool type is more suitable for @found. > >Well if you are ranging the type of found variable it also makes sense >to change the return value of the function to bool as well. Good catch. -- Thanks, Lu > >> >> Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com> >> --- >> fs/btrfs/extent_io.c | 7 +++---- >> 1 file changed, 3 insertions(+), 4 deletions(-) >> >> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c >> index 582b4b1c41e0..b4ee3399be96 100644 >> --- a/fs/btrfs/extent_io.c >> +++ b/fs/btrfs/extent_io.c >> @@ -1461,7 +1461,7 @@ static noinline u64 find_delalloc_range(struct extent_io_tree *tree, >> struct rb_node *node; >> struct extent_state *state; >> u64 cur_start = *start; >> - u64 found = 0; >> + bool found = false; >> u64 total_bytes = 0; >> >> spin_lock(&tree->lock); >> @@ -1472,8 +1472,7 @@ static noinline u64 find_delalloc_range(struct extent_io_tree *tree, >> */ >> node = tree_search(tree, cur_start); >> if (!node) { >> - if (!found) >> - *end = (u64)-1; >> + *end = (u64)-1; >> goto out; >> } >> >> @@ -1493,7 +1492,7 @@ static noinline u64 find_delalloc_range(struct extent_io_tree *tree, >> *cached_state = state; >> refcount_inc(&state->refs); >> } >> - found++; >> + found = true; >> *end = state->end; >> cur_start = state->end + 1; >> node = rb_next(node); >> > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 1/3] btrfs: remove always true if branch in find_delalloc_range 2018-11-28 3:21 [PATCH 1/3] btrfs: remove always true if branch in find_delalloc_range Lu Fengqi ` (2 preceding siblings ...) 2018-11-28 7:01 ` [PATCH 1/3] btrfs: remove always true if branch in find_delalloc_range Nikolay Borisov @ 2018-11-29 3:33 ` Lu Fengqi 2018-11-29 7:05 ` Nikolay Borisov 2018-12-04 14:35 ` David Sterba 3 siblings, 2 replies; 14+ messages in thread From: Lu Fengqi @ 2018-11-29 3:33 UTC (permalink / raw) To: linux-btrfs; +Cc: nborisov The @found is always false when it comes to the if branch. Besides, the bool type is more suitable for @found. Change the return value of the function and its caller to bool as well. Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com> --- fs/btrfs/extent_io.c | 31 +++++++++++++++---------------- fs/btrfs/extent_io.h | 2 +- fs/btrfs/tests/extent-io-tests.c | 2 +- 3 files changed, 17 insertions(+), 18 deletions(-) diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c index b2769e92b556..4b6b87e63b4a 100644 --- a/fs/btrfs/extent_io.c +++ b/fs/btrfs/extent_io.c @@ -1452,16 +1452,16 @@ int find_first_extent_bit(struct extent_io_tree *tree, u64 start, * find a contiguous range of bytes in the file marked as delalloc, not * more than 'max_bytes'. start and end are used to return the range, * - * 1 is returned if we find something, 0 if nothing was in the tree + * true is returned if we find something, false if nothing was in the tree */ -static noinline u64 find_delalloc_range(struct extent_io_tree *tree, +static noinline bool find_delalloc_range(struct extent_io_tree *tree, u64 *start, u64 *end, u64 max_bytes, struct extent_state **cached_state) { struct rb_node *node; struct extent_state *state; u64 cur_start = *start; - u64 found = 0; + bool found = false; u64 total_bytes = 0; spin_lock(&tree->lock); @@ -1472,8 +1472,7 @@ static noinline u64 find_delalloc_range(struct extent_io_tree *tree, */ node = tree_search(tree, cur_start); if (!node) { - if (!found) - *end = (u64)-1; + *end = (u64)-1; goto out; } @@ -1493,7 +1492,7 @@ static noinline u64 find_delalloc_range(struct extent_io_tree *tree, *cached_state = state; refcount_inc(&state->refs); } - found++; + found = true; *end = state->end; cur_start = state->end + 1; node = rb_next(node); @@ -1551,13 +1550,13 @@ static noinline int lock_delalloc_pages(struct inode *inode, } /* - * find a contiguous range of bytes in the file marked as delalloc, not - * more than 'max_bytes'. start and end are used to return the range, + * find and lock a contiguous range of bytes in the file marked as delalloc, + * not more than 'max_bytes'. start and end are used to return the range, * - * 1 is returned if we find something, 0 if nothing was in the tree + * true is returned if we find something, false if nothing was in the tree */ EXPORT_FOR_TESTS -noinline_for_stack u64 find_lock_delalloc_range(struct inode *inode, +noinline_for_stack bool find_lock_delalloc_range(struct inode *inode, struct extent_io_tree *tree, struct page *locked_page, u64 *start, u64 *end) @@ -1565,7 +1564,7 @@ noinline_for_stack u64 find_lock_delalloc_range(struct inode *inode, u64 max_bytes = BTRFS_MAX_EXTENT_SIZE; u64 delalloc_start; u64 delalloc_end; - u64 found; + bool found; struct extent_state *cached_state = NULL; int ret; int loops = 0; @@ -1580,7 +1579,7 @@ noinline_for_stack u64 find_lock_delalloc_range(struct inode *inode, *start = delalloc_start; *end = delalloc_end; free_extent_state(cached_state); - return 0; + return false; } /* @@ -1612,7 +1611,7 @@ noinline_for_stack u64 find_lock_delalloc_range(struct inode *inode, loops = 1; goto again; } else { - found = 0; + found = false; goto out_failed; } } @@ -3195,7 +3194,7 @@ static noinline_for_stack int writepage_delalloc(struct inode *inode, { struct extent_io_tree *tree = &BTRFS_I(inode)->io_tree; u64 page_end = delalloc_start + PAGE_SIZE - 1; - u64 nr_delalloc; + bool found; u64 delalloc_to_write = 0; u64 delalloc_end = 0; int ret; @@ -3203,11 +3202,11 @@ static noinline_for_stack int writepage_delalloc(struct inode *inode, while (delalloc_end < page_end) { - nr_delalloc = find_lock_delalloc_range(inode, tree, + found = find_lock_delalloc_range(inode, tree, page, &delalloc_start, &delalloc_end); - if (nr_delalloc == 0) { + if (!found) { delalloc_start = delalloc_end + 1; continue; } diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h index a1d3ea5a0d32..67881bca15fe 100644 --- a/fs/btrfs/extent_io.h +++ b/fs/btrfs/extent_io.h @@ -521,7 +521,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 -u64 find_lock_delalloc_range(struct inode *inode, struct extent_io_tree *tree, +bool find_lock_delalloc_range(struct inode *inode, struct extent_io_tree *tree, 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 d33f9a95bce1..3c46d7f23456 100644 --- a/fs/btrfs/tests/extent-io-tests.c +++ b/fs/btrfs/tests/extent-io-tests.c @@ -66,7 +66,7 @@ static int test_find_delalloc(u32 sectorsize) u64 max_bytes = BTRFS_MAX_EXTENT_SIZE; u64 total_dirty = 2 * max_bytes; u64 start, end, test_start; - u64 found; + bool found; int ret = -EINVAL; test_msg("running find delalloc tests"); -- 2.19.2 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/3] btrfs: remove always true if branch in find_delalloc_range 2018-11-29 3:33 ` [PATCH v2 " Lu Fengqi @ 2018-11-29 7:05 ` Nikolay Borisov 2018-12-04 14:35 ` David Sterba 1 sibling, 0 replies; 14+ messages in thread From: Nikolay Borisov @ 2018-11-29 7:05 UTC (permalink / raw) To: Lu Fengqi, linux-btrfs On 29.11.18 г. 5:33 ч., Lu Fengqi wrote: > The @found is always false when it comes to the if branch. Besides, the > bool type is more suitable for @found. Change the return value of the > function and its caller to bool as well. > > Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com> Reviewed-by: Nikolay Borisov <nborisov@suse.com> > --- > fs/btrfs/extent_io.c | 31 +++++++++++++++---------------- > fs/btrfs/extent_io.h | 2 +- > fs/btrfs/tests/extent-io-tests.c | 2 +- > 3 files changed, 17 insertions(+), 18 deletions(-) > > diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c > index b2769e92b556..4b6b87e63b4a 100644 > --- a/fs/btrfs/extent_io.c > +++ b/fs/btrfs/extent_io.c > @@ -1452,16 +1452,16 @@ int find_first_extent_bit(struct extent_io_tree *tree, u64 start, > * find a contiguous range of bytes in the file marked as delalloc, not > * more than 'max_bytes'. start and end are used to return the range, > * > - * 1 is returned if we find something, 0 if nothing was in the tree > + * true is returned if we find something, false if nothing was in the tree > */ > -static noinline u64 find_delalloc_range(struct extent_io_tree *tree, > +static noinline bool find_delalloc_range(struct extent_io_tree *tree, > u64 *start, u64 *end, u64 max_bytes, > struct extent_state **cached_state) > { > struct rb_node *node; > struct extent_state *state; > u64 cur_start = *start; > - u64 found = 0; > + bool found = false; > u64 total_bytes = 0; > > spin_lock(&tree->lock); > @@ -1472,8 +1472,7 @@ static noinline u64 find_delalloc_range(struct extent_io_tree *tree, > */ > node = tree_search(tree, cur_start); > if (!node) { > - if (!found) > - *end = (u64)-1; > + *end = (u64)-1; > goto out; > } > > @@ -1493,7 +1492,7 @@ static noinline u64 find_delalloc_range(struct extent_io_tree *tree, > *cached_state = state; > refcount_inc(&state->refs); > } > - found++; > + found = true; > *end = state->end; > cur_start = state->end + 1; > node = rb_next(node); > @@ -1551,13 +1550,13 @@ static noinline int lock_delalloc_pages(struct inode *inode, > } > > /* > - * find a contiguous range of bytes in the file marked as delalloc, not > - * more than 'max_bytes'. start and end are used to return the range, > + * find and lock a contiguous range of bytes in the file marked as delalloc, > + * not more than 'max_bytes'. start and end are used to return the range, > * > - * 1 is returned if we find something, 0 if nothing was in the tree > + * true is returned if we find something, false if nothing was in the tree > */ > EXPORT_FOR_TESTS > -noinline_for_stack u64 find_lock_delalloc_range(struct inode *inode, > +noinline_for_stack bool find_lock_delalloc_range(struct inode *inode, > struct extent_io_tree *tree, > struct page *locked_page, u64 *start, > u64 *end) > @@ -1565,7 +1564,7 @@ noinline_for_stack u64 find_lock_delalloc_range(struct inode *inode, > u64 max_bytes = BTRFS_MAX_EXTENT_SIZE; > u64 delalloc_start; > u64 delalloc_end; > - u64 found; > + bool found; > struct extent_state *cached_state = NULL; > int ret; > int loops = 0; > @@ -1580,7 +1579,7 @@ noinline_for_stack u64 find_lock_delalloc_range(struct inode *inode, > *start = delalloc_start; > *end = delalloc_end; > free_extent_state(cached_state); > - return 0; > + return false; > } > > /* > @@ -1612,7 +1611,7 @@ noinline_for_stack u64 find_lock_delalloc_range(struct inode *inode, > loops = 1; > goto again; > } else { > - found = 0; > + found = false; > goto out_failed; > } > } > @@ -3195,7 +3194,7 @@ static noinline_for_stack int writepage_delalloc(struct inode *inode, > { > struct extent_io_tree *tree = &BTRFS_I(inode)->io_tree; > u64 page_end = delalloc_start + PAGE_SIZE - 1; > - u64 nr_delalloc; > + bool found; > u64 delalloc_to_write = 0; > u64 delalloc_end = 0; > int ret; > @@ -3203,11 +3202,11 @@ static noinline_for_stack int writepage_delalloc(struct inode *inode, > > > while (delalloc_end < page_end) { > - nr_delalloc = find_lock_delalloc_range(inode, tree, > + found = find_lock_delalloc_range(inode, tree, > page, > &delalloc_start, > &delalloc_end); > - if (nr_delalloc == 0) { > + if (!found) { > delalloc_start = delalloc_end + 1; > continue; > } > diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h > index a1d3ea5a0d32..67881bca15fe 100644 > --- a/fs/btrfs/extent_io.h > +++ b/fs/btrfs/extent_io.h > @@ -521,7 +521,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 > -u64 find_lock_delalloc_range(struct inode *inode, struct extent_io_tree *tree, > +bool find_lock_delalloc_range(struct inode *inode, struct extent_io_tree *tree, > 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 d33f9a95bce1..3c46d7f23456 100644 > --- a/fs/btrfs/tests/extent-io-tests.c > +++ b/fs/btrfs/tests/extent-io-tests.c > @@ -66,7 +66,7 @@ static int test_find_delalloc(u32 sectorsize) > u64 max_bytes = BTRFS_MAX_EXTENT_SIZE; > u64 total_dirty = 2 * max_bytes; > u64 start, end, test_start; > - u64 found; > + bool found; > int ret = -EINVAL; > > test_msg("running find delalloc tests"); > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/3] btrfs: remove always true if branch in find_delalloc_range 2018-11-29 3:33 ` [PATCH v2 " Lu Fengqi 2018-11-29 7:05 ` Nikolay Borisov @ 2018-12-04 14:35 ` David Sterba 1 sibling, 0 replies; 14+ messages in thread From: David Sterba @ 2018-12-04 14:35 UTC (permalink / raw) To: Lu Fengqi; +Cc: linux-btrfs, nborisov On Thu, Nov 29, 2018 at 11:33:38AM +0800, Lu Fengqi wrote: > The @found is always false when it comes to the if branch. Besides, the > bool type is more suitable for @found. Change the return value of the > function and its caller to bool as well. > > Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com> Added to misc-next, thanks. ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2018-12-04 14:35 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-11-28 3:21 [PATCH 1/3] btrfs: remove always true if branch in find_delalloc_range Lu Fengqi 2018-11-28 3:22 ` [PATCH 2/3] btrfs: cleanup the useless DEFINE_WAIT in cleanup_transaction Lu Fengqi 2018-11-28 7:02 ` Nikolay Borisov 2018-11-28 13:34 ` Johannes Thumshirn 2018-11-28 16:24 ` David Sterba 2018-11-28 3:23 ` [PATCH 3/3] btrfs: remove redundant nowait check for buffered_write Lu Fengqi 2018-11-28 7:04 ` Nikolay Borisov 2018-11-28 16:20 ` David Sterba 2018-11-28 13:36 ` Johannes Thumshirn 2018-11-28 7:01 ` [PATCH 1/3] btrfs: remove always true if branch in find_delalloc_range Nikolay Borisov 2018-11-28 9:33 ` Lu Fengqi 2018-11-29 3:33 ` [PATCH v2 " Lu Fengqi 2018-11-29 7:05 ` Nikolay Borisov 2018-12-04 14:35 ` 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).