* [PATCH] btrfs: use enum instead of constant value @ 2017-12-05 10:35 Gu Jinxiang 2017-12-05 17:35 ` David Sterba 0 siblings, 1 reply; 9+ messages in thread From: Gu Jinxiang @ 2017-12-05 10:35 UTC (permalink / raw) To: linux-btrfs Use enum READA_BACK instead of value 1 to keep source robust. Signed-off-by: Gu Jinxiang <gujx@cn.fujitsu.com> --- fs/btrfs/free-space-tree.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c index a5e34de..a105629 100644 --- a/fs/btrfs/free-space-tree.c +++ b/fs/btrfs/free-space-tree.c @@ -1071,7 +1071,7 @@ static int populate_free_space_tree(struct btrfs_trans_handle *trans, path = btrfs_alloc_path(); if (!path) return -ENOMEM; - path->reada = 1; + path->reada = READA_BACK; path2 = btrfs_alloc_path(); if (!path2) { @@ -1577,7 +1577,7 @@ int load_free_space_tree(struct btrfs_caching_control *caching_ctl) */ path->skip_locking = 1; path->search_commit_root = 1; - path->reada = 1; + path->reada = READA_BACK; info = search_free_space_info(NULL, fs_info, block_group, path, 0); if (IS_ERR(info)) { -- 1.9.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] btrfs: use enum instead of constant value 2017-12-05 10:35 [PATCH] btrfs: use enum instead of constant value Gu Jinxiang @ 2017-12-05 17:35 ` David Sterba 2018-01-11 6:59 ` [PATCH v2] " Gu Jinxiang 0 siblings, 1 reply; 9+ messages in thread From: David Sterba @ 2017-12-05 17:35 UTC (permalink / raw) To: Gu Jinxiang; +Cc: linux-btrfs On Tue, Dec 05, 2017 at 06:35:30PM +0800, Gu Jinxiang wrote: > Use enum READA_BACK instead of value 1 to keep source robust. Are you sure that READA_BACK is the right one? It corresponds to the value 1, but the actual readahead direction semantics should be considered. It is not obvious and needs some explanation in the changelog. ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2] btrfs: use enum instead of constant value 2017-12-05 17:35 ` David Sterba @ 2018-01-11 6:59 ` Gu Jinxiang 2018-01-11 7:24 ` Nikolay Borisov 0 siblings, 1 reply; 9+ messages in thread From: Gu Jinxiang @ 2018-01-11 6:59 UTC (permalink / raw) To: dsterba; +Cc: linux-btrfs Use enum READA_FORWARD instead of integers value to keep source robust. Changelog: v2->v1: change enum from READA_BACK to READA_FORWARD, since according to the logic of the source, it should reada_for_search forward, not backward. And, Reference: commit e4058b54d1e4 ("btrfs: cleanup, use enum values for btrfs_path reada") Signed-off-by: Gu Jinxiang <gujx@cn.fujitsu.com> --- fs/btrfs/free-space-tree.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c index fe5e0324dca9..af36a6a971fe 100644 --- a/fs/btrfs/free-space-tree.c +++ b/fs/btrfs/free-space-tree.c @@ -1071,7 +1071,7 @@ static int populate_free_space_tree(struct btrfs_trans_handle *trans, path = btrfs_alloc_path(); if (!path) return -ENOMEM; - path->reada = 1; + path->reada = READA_FORWARD; path2 = btrfs_alloc_path(); if (!path2) { @@ -1573,7 +1573,7 @@ int load_free_space_tree(struct btrfs_caching_control *caching_ctl) */ path->skip_locking = 1; path->search_commit_root = 1; - path->reada = 1; + path->reada = READA_FORWARD; info = search_free_space_info(NULL, fs_info, block_group, path, 0); if (IS_ERR(info)) { -- 2.14.3 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2] btrfs: use enum instead of constant value 2018-01-11 6:59 ` [PATCH v2] " Gu Jinxiang @ 2018-01-11 7:24 ` Nikolay Borisov 2018-01-11 8:12 ` [PATCH v3 1/2] " Gu Jinxiang 0 siblings, 1 reply; 9+ messages in thread From: Nikolay Borisov @ 2018-01-11 7:24 UTC (permalink / raw) To: Gu Jinxiang, dsterba; +Cc: linux-btrfs On 11.01.2018 08:59, Gu Jinxiang wrote: > Use enum READA_FORWARD instead of integers value to keep source robust. > > Changelog: > v2->v1: change enum from READA_BACK to READA_FORWARD, since according > to the logic of the source, it should reada_for_search forward, not > backward. > And, Reference: > commit e4058b54d1e4 ("btrfs: cleanup, use enum values for btrfs_path reada") > > Signed-off-by: Gu Jinxiang <gujx@cn.fujitsu.com> > --- > fs/btrfs/free-space-tree.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c > index fe5e0324dca9..af36a6a971fe 100644 > --- a/fs/btrfs/free-space-tree.c > +++ b/fs/btrfs/free-space-tree.c > @@ -1071,7 +1071,7 @@ static int populate_free_space_tree(struct btrfs_trans_handle *trans, > path = btrfs_alloc_path(); > if (!path) > return -ENOMEM; > - path->reada = 1; > + path->reada = READA_FORWARD; So in btrfs_search_slot_for_read we can go either backwards to forward based on the penultimate parameter (find_higher). We always pass that to 1 meaning we will be going iterating the leaves forward. In that regard we do want READA_FORWARD. Perhaps you might want to put this explanation in your commit message. > > path2 = btrfs_alloc_path(); > if (!path2) { > @@ -1573,7 +1573,7 @@ int load_free_space_tree(struct btrfs_caching_control *caching_ctl) > */ > path->skip_locking = 1; > path->search_commit_root = 1; > - path->reada = 1; > + path->reada = READA_FORWARD; Similar thing here. When we call load_free_space_bitmaps or load_free_space_extents we would be calling btrfs_next_item so reading forward makes sense. > > info = search_free_space_info(NULL, fs_info, block_group, path, 0); > if (IS_ERR(info)) { > ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 1/2] btrfs: use enum instead of constant value 2018-01-11 7:24 ` Nikolay Borisov @ 2018-01-11 8:12 ` Gu Jinxiang 2018-01-11 8:12 ` [PATCH v3 2/2] " Gu Jinxiang ` (2 more replies) 0 siblings, 3 replies; 9+ messages in thread From: Gu Jinxiang @ 2018-01-11 8:12 UTC (permalink / raw) To: nborisov; +Cc: dsterba, linux-btrfs, Gu JinXiang populate_free_space_tree calls function btrfs_search_slot_for_read with parameter int find_higher = 1, it means that, if no exact match is found, then use the next higher item. So in function populate_free_space_tree, use READA_FORWARD to read forward ahead. Changelog: v2->v1: change enum from READA_BACK to READA_FORWARD, since according to the logic of the source, it should reada_for_search forward, not backward. And, Reference: commit e4058b54d1e4 ("btrfs: cleanup, use enum values for btrfs_path reada") v3->v2: add commit message to make it easy to understand. And split it to two patch. Signed-off-by: Gu JinXiang <gujx@cn.fujitsu.com> --- fs/btrfs/free-space-tree.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c index fe5e0324dca9..d7b6c0017143 100644 --- a/fs/btrfs/free-space-tree.c +++ b/fs/btrfs/free-space-tree.c @@ -1071,7 +1071,7 @@ static int populate_free_space_tree(struct btrfs_trans_handle *trans, path = btrfs_alloc_path(); if (!path) return -ENOMEM; - path->reada = 1; + path->reada = READA_FORWARD; path2 = btrfs_alloc_path(); if (!path2) { -- 2.14.3 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v3 2/2] btrfs: use enum instead of constant value 2018-01-11 8:12 ` [PATCH v3 1/2] " Gu Jinxiang @ 2018-01-11 8:12 ` Gu Jinxiang 2018-01-11 8:14 ` Nikolay Borisov 2018-01-11 8:14 ` [PATCH v3 1/2] " Nikolay Borisov 2018-02-06 14:54 ` David Sterba 2 siblings, 1 reply; 9+ messages in thread From: Gu Jinxiang @ 2018-01-11 8:12 UTC (permalink / raw) To: nborisov; +Cc: dsterba, linux-btrfs, Gu JinXiang From: Gu JinXiang <gujx@cn.fujitsu.com> load_free_space_tree calls either function load_free_space_bitmaps or load_free_space_extents. And either of those two will lead to call btrfs_next_item. So in function load_free_space_tree, use READA_FORWARD to read forward ahead. Changelog: v2->v1: change enum from READA_BACK to READA_FORWARD, since according to the logic of the source, it should reada_for_search forward, not backward. And, Reference: commit e4058b54d1e4 ("btrfs: cleanup, use enum values for btrfs_path reada") v3->v2: add commit message to make it easy to understand. And split it to two patch. Signed-off-by: Gu JinXiang <gujx@cn.fujitsu.com> --- fs/btrfs/free-space-tree.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c index d7b6c0017143..af36a6a971fe 100644 --- a/fs/btrfs/free-space-tree.c +++ b/fs/btrfs/free-space-tree.c @@ -1573,7 +1573,7 @@ int load_free_space_tree(struct btrfs_caching_control *caching_ctl) */ path->skip_locking = 1; path->search_commit_root = 1; - path->reada = 1; + path->reada = READA_FORWARD; info = search_free_space_info(NULL, fs_info, block_group, path, 0); if (IS_ERR(info)) { -- 2.14.3 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v3 2/2] btrfs: use enum instead of constant value 2018-01-11 8:12 ` [PATCH v3 2/2] " Gu Jinxiang @ 2018-01-11 8:14 ` Nikolay Borisov 0 siblings, 0 replies; 9+ messages in thread From: Nikolay Borisov @ 2018-01-11 8:14 UTC (permalink / raw) To: Gu Jinxiang; +Cc: dsterba, linux-btrfs On 11.01.2018 10:12, Gu Jinxiang wrote: > From: Gu JinXiang <gujx@cn.fujitsu.com> > > load_free_space_tree calls either function load_free_space_bitmaps or > load_free_space_extents. And either of those two will lead to call > btrfs_next_item. > So in function load_free_space_tree, use READA_FORWARD to read forward > ahead. > > Changelog: > v2->v1: change enum from READA_BACK to READA_FORWARD, since according > to the logic of the source, it should reada_for_search forward, not > backward. > And, Reference: > commit e4058b54d1e4 ("btrfs: cleanup, use enum values for btrfs_path reada") > > v3->v2: add commit message to make it easy to understand. And split it > to two patch. > > Signed-off-by: Gu JinXiang <gujx@cn.fujitsu.com> Reviewed-by: Nikolay Borisov <nborisov@suse.com> > --- > fs/btrfs/free-space-tree.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c > index d7b6c0017143..af36a6a971fe 100644 > --- a/fs/btrfs/free-space-tree.c > +++ b/fs/btrfs/free-space-tree.c > @@ -1573,7 +1573,7 @@ int load_free_space_tree(struct btrfs_caching_control *caching_ctl) > */ > path->skip_locking = 1; > path->search_commit_root = 1; > - path->reada = 1; > + path->reada = READA_FORWARD; > > info = search_free_space_info(NULL, fs_info, block_group, path, 0); > if (IS_ERR(info)) { > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/2] btrfs: use enum instead of constant value 2018-01-11 8:12 ` [PATCH v3 1/2] " Gu Jinxiang 2018-01-11 8:12 ` [PATCH v3 2/2] " Gu Jinxiang @ 2018-01-11 8:14 ` Nikolay Borisov 2018-02-06 14:54 ` David Sterba 2 siblings, 0 replies; 9+ messages in thread From: Nikolay Borisov @ 2018-01-11 8:14 UTC (permalink / raw) To: Gu Jinxiang; +Cc: dsterba, linux-btrfs On 11.01.2018 10:12, Gu Jinxiang wrote: > populate_free_space_tree calls function btrfs_search_slot_for_read with > parameter int find_higher = 1, it means that, if no exact match is found, > then use the next higher item. > So in function populate_free_space_tree, use READA_FORWARD to read > forward ahead. > > Changelog: > v2->v1: change enum from READA_BACK to READA_FORWARD, since according > to the logic of the source, it should reada_for_search forward, not > backward. > And, Reference: > commit e4058b54d1e4 ("btrfs: cleanup, use enum values for btrfs_path reada") > > v3->v2: add commit message to make it easy to understand. And split it > to two patch. > > Signed-off-by: Gu JinXiang <gujx@cn.fujitsu.com> Reviewed-by: Nikolay Borisov <nborisov@suse.com> > --- > fs/btrfs/free-space-tree.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c > index fe5e0324dca9..d7b6c0017143 100644 > --- a/fs/btrfs/free-space-tree.c > +++ b/fs/btrfs/free-space-tree.c > @@ -1071,7 +1071,7 @@ static int populate_free_space_tree(struct btrfs_trans_handle *trans, > path = btrfs_alloc_path(); > if (!path) > return -ENOMEM; > - path->reada = 1; > + path->reada = READA_FORWARD; > > path2 = btrfs_alloc_path(); > if (!path2) { > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/2] btrfs: use enum instead of constant value 2018-01-11 8:12 ` [PATCH v3 1/2] " Gu Jinxiang 2018-01-11 8:12 ` [PATCH v3 2/2] " Gu Jinxiang 2018-01-11 8:14 ` [PATCH v3 1/2] " Nikolay Borisov @ 2018-02-06 14:54 ` David Sterba 2 siblings, 0 replies; 9+ messages in thread From: David Sterba @ 2018-02-06 14:54 UTC (permalink / raw) To: Gu Jinxiang; +Cc: nborisov, dsterba, linux-btrfs On Thu, Jan 11, 2018 at 04:12:17PM +0800, Gu Jinxiang wrote: > populate_free_space_tree calls function btrfs_search_slot_for_read with > parameter int find_higher = 1, it means that, if no exact match is found, > then use the next higher item. > So in function populate_free_space_tree, use READA_FORWARD to read > forward ahead. > > Changelog: > v2->v1: change enum from READA_BACK to READA_FORWARD, since according > to the logic of the source, it should reada_for_search forward, not > backward. > And, Reference: > commit e4058b54d1e4 ("btrfs: cleanup, use enum values for btrfs_path reada") > > v3->v2: add commit message to make it easy to understand. And split it > to two patch. > > Signed-off-by: Gu JinXiang <gujx@cn.fujitsu.com> For some reason I have the two mails tagged as applied but don't see them in any branch. Anyway, now added, thanks. ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-02-06 14:56 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-12-05 10:35 [PATCH] btrfs: use enum instead of constant value Gu Jinxiang 2017-12-05 17:35 ` David Sterba 2018-01-11 6:59 ` [PATCH v2] " Gu Jinxiang 2018-01-11 7:24 ` Nikolay Borisov 2018-01-11 8:12 ` [PATCH v3 1/2] " Gu Jinxiang 2018-01-11 8:12 ` [PATCH v3 2/2] " Gu Jinxiang 2018-01-11 8:14 ` Nikolay Borisov 2018-01-11 8:14 ` [PATCH v3 1/2] " Nikolay Borisov 2018-02-06 14:54 ` 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).