[PATCH] btrfs: use enum instead of constant value

[PATCH] btrfs: use enum instead of constant value

[Gu Jinxiang]

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

Re: [PATCH] btrfs: use enum instead of constant value

[David Sterba]

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.

[PATCH v2] btrfs: use enum instead of constant value

[Gu Jinxiang]

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

Re: [PATCH v2] btrfs: use enum instead of constant value

[Nikolay Borisov]

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)) {
> 

[PATCH v3 1/2] btrfs: use enum instead of constant value

[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

[PATCH v3 2/2] btrfs: use enum instead of constant value

[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

Re: [PATCH v3 1/2] btrfs: use enum instead of constant value

[Nikolay Borisov]

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) {
> 

Re: [PATCH v3 2/2] btrfs: use enum instead of constant value

[Nikolay Borisov]

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)) {
> 

Re: [PATCH v3 1/2] btrfs: use enum instead of constant value

[David Sterba]

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.