RE: [PATCH] Btrfs: remove unnecessary code of chunk_root assignment in btrfs_read_chunk_tree.

RE: [PATCH] Btrfs: remove unnecessary code of chunk_root assignment in btrfs_read_chunk_tree.

[Zhao Lei]

Hi, Sean Fu

> From: Sean Fu [mailto:fxinrong@gmail.com]
> Sent: Sunday, September 04, 2016 7:54 PM
> To: dsterba@suse.com
> Cc: clm@fb.com; anand.jain@oracle.com; fdmanana@suse.com;
> zhaolei@cn.fujitsu.com; linux-btrfs@vger.kernel.org;
> linux-kernel@vger.kernel.org; Sean Fu <fxinrong@gmail.com>
> Subject: [PATCH] Btrfs: remove unnecessary code of chunk_root assignment in
> btrfs_read_chunk_tree.
> 
> The input argument root is already set with "fs_info->chunk_root".
> "chunk_root = fs_info->chunk_root = btrfs_alloc_root(fs_info)" in caller
> "open_ctree".
> “root->fs_info = fs_info” in "btrfs_alloc_root".
> 
The root argument of this function means "any root".
And the function is designed getting chunk root from
"any root" in head.

Since there is only one caller of this function,
and the caller always send chunk_root as root argument in
current code, we can remove above conversion,
and I suggest renaming root to chunk_root to make it clear,
something like:

- btrfs_read_chunk_tree(struct btrfs_root *root)
+ btrfs_read_chunk_tree(struct btrfs_root *chunk_root)

Thanks
Zhaolei

> Signed-off-by: Sean Fu <fxinrong@gmail.com>
> ---
>  fs/btrfs/volumes.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 366b335..384a6d2 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -6600,8 +6600,6 @@ int btrfs_read_chunk_tree(struct btrfs_root *root)
>  	int ret;
>  	int slot;
> 
> -	root = root->fs_info->chunk_root;
> -
>  	path = btrfs_alloc_path();
>  	if (!path)
>  		return -ENOMEM;
> --
> 2.6.2
> 

Re: [PATCH] Btrfs: remove unnecessary code of chunk_root assignment in btrfs_read_chunk_tree.

[Qu Wenruo]

At 09/05/2016 09:19 AM, Zhao Lei wrote:
> Hi, Sean Fu
>
>> From: Sean Fu [mailto:fxinrong@gmail.com]
>> Sent: Sunday, September 04, 2016 7:54 PM
>> To: dsterba@suse.com
>> Cc: clm@fb.com; anand.jain@oracle.com; fdmanana@suse.com;
>> zhaolei@cn.fujitsu.com; linux-btrfs@vger.kernel.org;
>> linux-kernel@vger.kernel.org; Sean Fu <fxinrong@gmail.com>
>> Subject: [PATCH] Btrfs: remove unnecessary code of chunk_root assignment in
>> btrfs_read_chunk_tree.
>>
>> The input argument root is already set with "fs_info->chunk_root".
>> "chunk_root = fs_info->chunk_root = btrfs_alloc_root(fs_info)" in caller
>> "open_ctree".
>> “root->fs_info = fs_info” in "btrfs_alloc_root".
>>
> The root argument of this function means "any root".
> And the function is designed getting chunk root from
> "any root" in head.
>
> Since there is only one caller of this function,
> and the caller always send chunk_root as root argument in
> current code, we can remove above conversion,
> and I suggest renaming root to chunk_root to make it clear,
> something like:
>
> - btrfs_read_chunk_tree(struct btrfs_root *root)
> + btrfs_read_chunk_tree(struct btrfs_root *chunk_root)

Since root is only used to get fs_info->chunk_root, why not use fs_info 
directly?

Thanks,
Qu

>
> Thanks
> Zhaolei
>
>> Signed-off-by: Sean Fu <fxinrong@gmail.com>
>> ---
>>  fs/btrfs/volumes.c | 2 --
>>  1 file changed, 2 deletions(-)
>>
>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>> index 366b335..384a6d2 100644
>> --- a/fs/btrfs/volumes.c
>> +++ b/fs/btrfs/volumes.c
>> @@ -6600,8 +6600,6 @@ int btrfs_read_chunk_tree(struct btrfs_root *root)
>>  	int ret;
>>  	int slot;
>>
>> -	root = root->fs_info->chunk_root;
>> -
>>  	path = btrfs_alloc_path();
>>  	if (!path)
>>  		return -ENOMEM;
>> --
>> 2.6.2
>>
>
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

RE: [PATCH] Btrfs: remove unnecessary code of chunk_root assignment in btrfs_read_chunk_tree.

[Zhao Lei]

Hi, Qu Wenruo

> From: Qu Wenruo [mailto:quwenruo@cn.fujitsu.com]
> Sent: Monday, September 05, 2016 3:57 PM
> To: Zhao Lei <zhaolei@cn.fujitsu.com>; 'Sean Fu' <fxinrong@gmail.com>;
> dsterba@suse.com
> Cc: clm@fb.com; anand.jain@oracle.com; fdmanana@suse.com;
> linux-btrfs@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] Btrfs: remove unnecessary code of chunk_root
> assignment in btrfs_read_chunk_tree.
> 
> 
> 
> At 09/05/2016 09:19 AM, Zhao Lei wrote:
> > Hi, Sean Fu
> >
> >> From: Sean Fu [mailto:fxinrong@gmail.com]
> >> Sent: Sunday, September 04, 2016 7:54 PM
> >> To: dsterba@suse.com
> >> Cc: clm@fb.com; anand.jain@oracle.com; fdmanana@suse.com;
> >> zhaolei@cn.fujitsu.com; linux-btrfs@vger.kernel.org;
> >> linux-kernel@vger.kernel.org; Sean Fu <fxinrong@gmail.com>
> >> Subject: [PATCH] Btrfs: remove unnecessary code of chunk_root assignment
> in
> >> btrfs_read_chunk_tree.
> >>
> >> The input argument root is already set with "fs_info->chunk_root".
> >> "chunk_root = fs_info->chunk_root = btrfs_alloc_root(fs_info)" in caller
> >> "open_ctree".
> >> “root->fs_info = fs_info” in "btrfs_alloc_root".
> >>
> > The root argument of this function means "any root".
> > And the function is designed getting chunk root from
> > "any root" in head.
> >
> > Since there is only one caller of this function,
> > and the caller always send chunk_root as root argument in
> > current code, we can remove above conversion,
> > and I suggest renaming root to chunk_root to make it clear,
> > something like:
> >
> > - btrfs_read_chunk_tree(struct btrfs_root *root)
> > + btrfs_read_chunk_tree(struct btrfs_root *chunk_root)
> 
> Since root is only used to get fs_info->chunk_root, why not use fs_info
> directly?
> 
Good idea.

Thanks
Zhaolei

> Thanks,
> Qu
> 
> >
> > Thanks
> > Zhaolei
> >
> >> Signed-off-by: Sean Fu <fxinrong@gmail.com>
> >> ---
> >>  fs/btrfs/volumes.c | 2 --
> >>  1 file changed, 2 deletions(-)
> >>
> >> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> >> index 366b335..384a6d2 100644
> >> --- a/fs/btrfs/volumes.c
> >> +++ b/fs/btrfs/volumes.c
> >> @@ -6600,8 +6600,6 @@ int btrfs_read_chunk_tree(struct btrfs_root
> *root)
> >>  	int ret;
> >>  	int slot;
> >>
> >> -	root = root->fs_info->chunk_root;
> >> -
> >>  	path = btrfs_alloc_path();
> >>  	if (!path)
> >>  		return -ENOMEM;
> >> --
> >> 2.6.2
> >>
> >
> >
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >

Re: [PATCH] Btrfs: remove unnecessary code of chunk_root assignment in btrfs_read_chunk_tree.

[Jeff Mahoney]

[-- Attachment #1.1: Type: text/plain, Size: 2605 bytes --]

On 9/5/16 3:56 AM, Qu Wenruo wrote:
> 
> 
> At 09/05/2016 09:19 AM, Zhao Lei wrote:
>> Hi, Sean Fu
>>
>>> From: Sean Fu [mailto:fxinrong@gmail.com]
>>> Sent: Sunday, September 04, 2016 7:54 PM
>>> To: dsterba@suse.com
>>> Cc: clm@fb.com; anand.jain@oracle.com; fdmanana@suse.com;
>>> zhaolei@cn.fujitsu.com; linux-btrfs@vger.kernel.org;
>>> linux-kernel@vger.kernel.org; Sean Fu <fxinrong@gmail.com>
>>> Subject: [PATCH] Btrfs: remove unnecessary code of chunk_root
>>> assignment in
>>> btrfs_read_chunk_tree.
>>>
>>> The input argument root is already set with "fs_info->chunk_root".
>>> "chunk_root = fs_info->chunk_root = btrfs_alloc_root(fs_info)" in caller
>>> "open_ctree".
>>> “root->fs_info = fs_info” in "btrfs_alloc_root".
>>>
>> The root argument of this function means "any root".
>> And the function is designed getting chunk root from
>> "any root" in head.
>>
>> Since there is only one caller of this function,
>> and the caller always send chunk_root as root argument in
>> current code, we can remove above conversion,
>> and I suggest renaming root to chunk_root to make it clear,
>> something like:
>>
>> - btrfs_read_chunk_tree(struct btrfs_root *root)
>> + btrfs_read_chunk_tree(struct btrfs_root *chunk_root)
> 
> Since root is only used to get fs_info->chunk_root, why not use fs_info
> directly?

Weird.  Exactly this was a part of my fs_info patchset.  I guess I need
to go back and check what else is missing.

-Jeff


> Thanks,
> Qu
> 
>>
>> Thanks
>> Zhaolei
>>
>>> Signed-off-by: Sean Fu <fxinrong@gmail.com>
>>> ---
>>>  fs/btrfs/volumes.c | 2 --
>>>  1 file changed, 2 deletions(-)
>>>
>>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>>> index 366b335..384a6d2 100644
>>> --- a/fs/btrfs/volumes.c
>>> +++ b/fs/btrfs/volumes.c
>>> @@ -6600,8 +6600,6 @@ int btrfs_read_chunk_tree(struct btrfs_root *root)
>>>      int ret;
>>>      int slot;
>>>
>>> -    root = root->fs_info->chunk_root;
>>> -
>>>      path = btrfs_alloc_path();
>>>      if (!path)
>>>          return -ENOMEM;
>>> -- 
>>> 2.6.2
>>>
>>
>>
>>
>>
>> -- 
>> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> 
> 
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


-- 
Jeff Mahoney
SUSE Labs


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 827 bytes --]

Re: [PATCH] Btrfs: remove unnecessary code of chunk_root assignment in btrfs_read_chunk_tree.

[Jeff Mahoney]

[-- Attachment #1.1: Type: text/plain, Size: 2859 bytes --]

On 9/5/16 11:05 PM, Jeff Mahoney wrote:
> On 9/5/16 3:56 AM, Qu Wenruo wrote:
>>
>>
>> At 09/05/2016 09:19 AM, Zhao Lei wrote:
>>> Hi, Sean Fu
>>>
>>>> From: Sean Fu [mailto:fxinrong@gmail.com]
>>>> Sent: Sunday, September 04, 2016 7:54 PM
>>>> To: dsterba@suse.com
>>>> Cc: clm@fb.com; anand.jain@oracle.com; fdmanana@suse.com;
>>>> zhaolei@cn.fujitsu.com; linux-btrfs@vger.kernel.org;
>>>> linux-kernel@vger.kernel.org; Sean Fu <fxinrong@gmail.com>
>>>> Subject: [PATCH] Btrfs: remove unnecessary code of chunk_root
>>>> assignment in
>>>> btrfs_read_chunk_tree.
>>>>
>>>> The input argument root is already set with "fs_info->chunk_root".
>>>> "chunk_root = fs_info->chunk_root = btrfs_alloc_root(fs_info)" in caller
>>>> "open_ctree".
>>>> “root->fs_info = fs_info” in "btrfs_alloc_root".
>>>>
>>> The root argument of this function means "any root".
>>> And the function is designed getting chunk root from
>>> "any root" in head.
>>>
>>> Since there is only one caller of this function,
>>> and the caller always send chunk_root as root argument in
>>> current code, we can remove above conversion,
>>> and I suggest renaming root to chunk_root to make it clear,
>>> something like:
>>>
>>> - btrfs_read_chunk_tree(struct btrfs_root *root)
>>> + btrfs_read_chunk_tree(struct btrfs_root *chunk_root)
>>
>> Since root is only used to get fs_info->chunk_root, why not use fs_info
>> directly?
> 
> Weird.  Exactly this was a part of my fs_info patchset.  I guess I need
> to go back and check what else is missing.

Actually, most of this didn't land.  Pretty much anything that's a root
->fs_info conversion is in there.

-Jeff

> -Jeff
> 
> 
>> Thanks,
>> Qu
>>
>>>
>>> Thanks
>>> Zhaolei
>>>
>>>> Signed-off-by: Sean Fu <fxinrong@gmail.com>
>>>> ---
>>>>  fs/btrfs/volumes.c | 2 --
>>>>  1 file changed, 2 deletions(-)
>>>>
>>>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>>>> index 366b335..384a6d2 100644
>>>> --- a/fs/btrfs/volumes.c
>>>> +++ b/fs/btrfs/volumes.c
>>>> @@ -6600,8 +6600,6 @@ int btrfs_read_chunk_tree(struct btrfs_root *root)
>>>>      int ret;
>>>>      int slot;
>>>>
>>>> -    root = root->fs_info->chunk_root;
>>>> -
>>>>      path = btrfs_alloc_path();
>>>>      if (!path)
>>>>          return -ENOMEM;
>>>> -- 
>>>> 2.6.2
>>>>
>>>
>>>
>>>
>>>
>>> -- 
>>> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>>
>>
>> -- 
>> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> 
> 


-- 
Jeff Mahoney
SUSE Labs


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 827 bytes --]

Re: [PATCH] Btrfs: remove unnecessary code of chunk_root assignment in btrfs_read_chunk_tree.

[David Sterba]

On Mon, Sep 05, 2016 at 11:13:40PM -0400, Jeff Mahoney wrote:
> >> Since root is only used to get fs_info->chunk_root, why not use fs_info
> >> directly?
> > 
> > Weird.  Exactly this was a part of my fs_info patchset.  I guess I need
> > to go back and check what else is missing.
> 
> Actually, most of this didn't land.  Pretty much anything that's a root
> ->fs_info conversion is in there.

Only half of the patchset has been merged so far because it did not pass
testing, so I bisected to some point. I was about to let you know once
most of 4.9 patches are prepared so there are less merge conflicts.

Re: [PATCH] Btrfs: remove unnecessary code of chunk_root assignment in btrfs_read_chunk_tree.

[Jeff Mahoney]

[-- Attachment #1.1: Type: text/plain, Size: 814 bytes --]

On 9/6/16 5:58 AM, David Sterba wrote:
> On Mon, Sep 05, 2016 at 11:13:40PM -0400, Jeff Mahoney wrote:
>>>> Since root is only used to get fs_info->chunk_root, why not use fs_info
>>>> directly?
>>>
>>> Weird.  Exactly this was a part of my fs_info patchset.  I guess I need
>>> to go back and check what else is missing.
>>
>> Actually, most of this didn't land.  Pretty much anything that's a root
>> ->fs_info conversion is in there.
> 
> Only half of the patchset has been merged so far because it did not pass
> testing, so I bisected to some point. I was about to let you know once
> most of 4.9 patches are prepared so there are less merge conflicts.

Ok, thanks.  I was going to start the rebase today but I'll hold off
until you're set for 4.9.

-Jeff

-- 
Jeff Mahoney
SUSE Labs


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 881 bytes --]

Re: [PATCH] Btrfs: remove unnecessary code of chunk_root assignment in btrfs_read_chunk_tree.

[Sean Fu]

On Mon, Sep 05, 2016 at 03:56:41PM +0800, Qu Wenruo wrote:
> 
> 
> At 09/05/2016 09:19 AM, Zhao Lei wrote:
> >Hi, Sean Fu
> >
> >>From: Sean Fu [mailto:fxinrong@gmail.com]
> >>Sent: Sunday, September 04, 2016 7:54 PM
> >>To: dsterba@suse.com
> >>Cc: clm@fb.com; anand.jain@oracle.com; fdmanana@suse.com;
> >>zhaolei@cn.fujitsu.com; linux-btrfs@vger.kernel.org;
> >>linux-kernel@vger.kernel.org; Sean Fu <fxinrong@gmail.com>
> >>Subject: [PATCH] Btrfs: remove unnecessary code of chunk_root assignment in
> >>btrfs_read_chunk_tree.
> >>
> >>The input argument root is already set with "fs_info->chunk_root".
> >>"chunk_root = fs_info->chunk_root = btrfs_alloc_root(fs_info)" in caller
> >>"open_ctree".
> >>“root->fs_info = fs_info” in "btrfs_alloc_root".
> >>
> >The root argument of this function means "any root".
> >And the function is designed getting chunk root from
> >"any root" in head.
> >
> >Since there is only one caller of this function,
> >and the caller always send chunk_root as root argument in
> >current code, we can remove above conversion,
> >and I suggest renaming root to chunk_root to make it clear,
> >something like:
> >
> >- btrfs_read_chunk_tree(struct btrfs_root *root)
> >+ btrfs_read_chunk_tree(struct btrfs_root *chunk_root)
> 
> Since root is only used to get fs_info->chunk_root, why not use fs_info
> directly?
Sorry for late reply.
chunk_root is processed in btrfs_read_chunk_tree.
Why should we pass fs_info directly to btrfs_read_chunk_tree?
Could you give me more detail?

Many thanks
> 
> Thanks,
> Qu
> 
> >
> >Thanks
> >Zhaolei
> >
> >>Signed-off-by: Sean Fu <fxinrong@gmail.com>
> >>---
> >> fs/btrfs/volumes.c | 2 --
> >> 1 file changed, 2 deletions(-)
> >>
> >>diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> >>index 366b335..384a6d2 100644
> >>--- a/fs/btrfs/volumes.c
> >>+++ b/fs/btrfs/volumes.c
> >>@@ -6600,8 +6600,6 @@ int btrfs_read_chunk_tree(struct btrfs_root *root)
> >> 	int ret;
> >> 	int slot;
> >>
> >>-	root = root->fs_info->chunk_root;
> >>-
> >> 	path = btrfs_alloc_path();
> >> 	if (!path)
> >> 		return -ENOMEM;
> >>--
> >>2.6.2
> >>
> >
> >
> >
> >
> >--
> >To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> >the body of a message to majordomo@vger.kernel.org
> >More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >
> 
> 

Re: [PATCH] Btrfs: remove unnecessary code of chunk_root assignment in btrfs_read_chunk_tree.

[Sean Fu]

On Tue, Sep 06, 2016 at 11:12:20AM -0400, Jeff Mahoney wrote:
> On 9/6/16 5:58 AM, David Sterba wrote:
> > On Mon, Sep 05, 2016 at 11:13:40PM -0400, Jeff Mahoney wrote:
> >>>> Since root is only used to get fs_info->chunk_root, why not use fs_info
> >>>> directly?
> >>>
> >>> Weird.  Exactly this was a part of my fs_info patchset.  I guess I need
> >>> to go back and check what else is missing.
> >>
> >> Actually, most of this didn't land.  Pretty much anything that's a root
> >> ->fs_info conversion is in there.
> > 
> > Only half of the patchset has been merged so far because it did not pass
> > testing, so I bisected to some point. I was about to let you know once
> > most of 4.9 patches are prepared so there are less merge conflicts.
> 
> Ok, thanks.  I was going to start the rebase today but I'll hold off
> until you're set for 4.9.
Sorry for late reply.
Many thanks to all of you.
> 
> -Jeff
> 
> -- 
> Jeff Mahoney
> SUSE Labs
> 

Re: [PATCH] Btrfs: remove unnecessary code of chunk_root assignment in btrfs_read_chunk_tree.

[Qu Wenruo]

At 09/07/2016 09:38 AM, Sean Fu wrote:
> On Mon, Sep 05, 2016 at 03:56:41PM +0800, Qu Wenruo wrote:
>>
>>
>> At 09/05/2016 09:19 AM, Zhao Lei wrote:
>>> Hi, Sean Fu
>>>
>>>> From: Sean Fu [mailto:fxinrong@gmail.com]
>>>> Sent: Sunday, September 04, 2016 7:54 PM
>>>> To: dsterba@suse.com
>>>> Cc: clm@fb.com; anand.jain@oracle.com; fdmanana@suse.com;
>>>> zhaolei@cn.fujitsu.com; linux-btrfs@vger.kernel.org;
>>>> linux-kernel@vger.kernel.org; Sean Fu <fxinrong@gmail.com>
>>>> Subject: [PATCH] Btrfs: remove unnecessary code of chunk_root assignment in
>>>> btrfs_read_chunk_tree.
>>>>
>>>> The input argument root is already set with "fs_info->chunk_root".
>>>> "chunk_root = fs_info->chunk_root = btrfs_alloc_root(fs_info)" in caller
>>>> "open_ctree".
>>>> “root->fs_info = fs_info” in "btrfs_alloc_root".
>>>>
>>> The root argument of this function means "any root".
>>> And the function is designed getting chunk root from
>>> "any root" in head.
>>>
>>> Since there is only one caller of this function,
>>> and the caller always send chunk_root as root argument in
>>> current code, we can remove above conversion,
>>> and I suggest renaming root to chunk_root to make it clear,
>>> something like:
>>>
>>> - btrfs_read_chunk_tree(struct btrfs_root *root)
>>> + btrfs_read_chunk_tree(struct btrfs_root *chunk_root)
>>
>> Since root is only used to get fs_info->chunk_root, why not use fs_info
>> directly?
> Sorry for late reply.
> chunk_root is processed in btrfs_read_chunk_tree.
> Why should we pass fs_info directly to btrfs_read_chunk_tree?
> Could you give me more detail?
>
> Many thanks

Normally we should only pass btrfs_root as parameter if it's a 
file/log/relocation tree which can't be grabbed directly from fs_info.

For system wide trees, which are already in fs_info, like 
fs_info->extent_root/chunk_root/..., we should pass fs_info.

Which is much much safer than passing a btrfs_root.
Careless caller can pass wrong tree and cause undefined behavior.

And such behavior makes caller more aware of what they really want to do.
Cases like just to grab sectorsize/nodesize shouldn't need a full 
btrfs_root.
(Jeff's patchset has already done such things quite well)

Thanks,
Qu

>>
>> Thanks,
>> Qu
>>
>>>
>>> Thanks
>>> Zhaolei
>>>
>>>> Signed-off-by: Sean Fu <fxinrong@gmail.com>
>>>> ---
>>>> fs/btrfs/volumes.c | 2 --
>>>> 1 file changed, 2 deletions(-)
>>>>
>>>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>>>> index 366b335..384a6d2 100644
>>>> --- a/fs/btrfs/volumes.c
>>>> +++ b/fs/btrfs/volumes.c
>>>> @@ -6600,8 +6600,6 @@ int btrfs_read_chunk_tree(struct btrfs_root *root)
>>>> 	int ret;
>>>> 	int slot;
>>>>
>>>> -	root = root->fs_info->chunk_root;
>>>> -
>>>> 	path = btrfs_alloc_path();
>>>> 	if (!path)
>>>> 		return -ENOMEM;
>>>> --
>>>> 2.6.2
>>>>
>>>
>>>
>>>
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>

Re: [PATCH] Btrfs: remove unnecessary code of chunk_root assignment in btrfs_read_chunk_tree.

[Sean Fu]

On Tue, Sep 06, 2016 at 11:12:20AM -0400, Jeff Mahoney wrote:
> On 9/6/16 5:58 AM, David Sterba wrote:
> > On Mon, Sep 05, 2016 at 11:13:40PM -0400, Jeff Mahoney wrote:
> >>>> Since root is only used to get fs_info->chunk_root, why not use fs_info
> >>>> directly?
> >>>
> >>> Weird.  Exactly this was a part of my fs_info patchset.  I guess I need
> >>> to go back and check what else is missing.
> >>
> >> Actually, most of this didn't land.  Pretty much anything that's a root
> >> ->fs_info conversion is in there.
> > 
> > Only half of the patchset has been merged so far because it did not pass
> > testing, so I bisected to some point. I was about to let you know once
> > most of 4.9 patches are prepared so there are less merge conflicts.
> 
> Ok, thanks.  I was going to start the rebase today but I'll hold off
> until you're set for 4.9.
> 
Hi Jeff, Could you please share your patch? Where can i get it?
I wanna have a look at it.

Thanks
> -Jeff
> 
> -- 
> Jeff Mahoney
> SUSE Labs
> 

Re: [PATCH] Btrfs: remove unnecessary code of chunk_root assignment in btrfs_read_chunk_tree.

[Jeff Mahoney]

[-- Attachment #1.1: Type: text/plain, Size: 1770 bytes --]

On 9/8/16 11:08 PM, Sean Fu wrote:
> On Tue, Sep 06, 2016 at 11:12:20AM -0400, Jeff Mahoney wrote:
>> On 9/6/16 5:58 AM, David Sterba wrote:
>>> On Mon, Sep 05, 2016 at 11:13:40PM -0400, Jeff Mahoney wrote:
>>>>>> Since root is only used to get fs_info->chunk_root, why not use fs_info
>>>>>> directly?
>>>>>
>>>>> Weird.  Exactly this was a part of my fs_info patchset.  I guess I need
>>>>> to go back and check what else is missing.
>>>>
>>>> Actually, most of this didn't land.  Pretty much anything that's a root
>>>> ->fs_info conversion is in there.
>>>
>>> Only half of the patchset has been merged so far because it did not pass
>>> testing, so I bisected to some point. I was about to let you know once
>>> most of 4.9 patches are prepared so there are less merge conflicts.
>>
>> Ok, thanks.  I was going to start the rebase today but I'll hold off
>> until you're set for 4.9.
>>
> Hi Jeff, Could you please share your patch? Where can i get it?
> I wanna have a look at it.

Sure, it's the whole series that starts with this commit:
commit 160ceedfd40085cfb1e08305917fcc24cefdad93
Author: Jeff Mahoney <jeffm@suse.com>
Date:   Wed Aug 31 23:55:33 2016 -0400

    btrfs: add dynamic debug support

... I still need to do clean up some commits that need merging.

https://git.kernel.org/cgit/linux/kernel/git/jeffm/linux-btrfs.git/log/?h=btrfs-testing/kdave/misc-4.9/root-fsinfo-cleanup

-Jeff


> Thanks
>> -Jeff
>>
>> -- 
>> Jeff Mahoney
>> SUSE Labs
>>
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


-- 
Jeff Mahoney
SUSE Labs


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 881 bytes --]

Re: [PATCH] Btrfs: remove unnecessary code of chunk_root assignment in btrfs_read_chunk_tree.

[Sean Fu]

On Thu, Sep 08, 2016 at 11:25:48PM -0400, Jeff Mahoney wrote:
> On 9/8/16 11:08 PM, Sean Fu wrote:
> > On Tue, Sep 06, 2016 at 11:12:20AM -0400, Jeff Mahoney wrote:
> >> On 9/6/16 5:58 AM, David Sterba wrote:
> >>> On Mon, Sep 05, 2016 at 11:13:40PM -0400, Jeff Mahoney wrote:
> >>>>>> Since root is only used to get fs_info->chunk_root, why not use fs_info
> >>>>>> directly?
> >>>>>
> >>>>> Weird.  Exactly this was a part of my fs_info patchset.  I guess I need
> >>>>> to go back and check what else is missing.
> >>>>
> >>>> Actually, most of this didn't land.  Pretty much anything that's a root
> >>>> ->fs_info conversion is in there.
> >>>
> >>> Only half of the patchset has been merged so far because it did not pass
> >>> testing, so I bisected to some point. I was about to let you know once
> >>> most of 4.9 patches are prepared so there are less merge conflicts.
> >>
> >> Ok, thanks.  I was going to start the rebase today but I'll hold off
> >> until you're set for 4.9.
> >>
> > Hi Jeff, Could you please share your patch? Where can i get it?
> > I wanna have a look at it.
> 
> Sure, it's the whole series that starts with this commit:
> commit 160ceedfd40085cfb1e08305917fcc24cefdad93
> Author: Jeff Mahoney <jeffm@suse.com>
> Date:   Wed Aug 31 23:55:33 2016 -0400
> 
>     btrfs: add dynamic debug support
> 
> ... I still need to do clean up some commits that need merging.
> 
> https://git.kernel.org/cgit/linux/kernel/git/jeffm/linux-btrfs.git/log/?h=btrfs-testing/kdave/misc-4.9/root-fsinfo-cleanup
>
Nice work.
Thanks

> -Jeff
> 
> 
> > Thanks
> >> -Jeff
> >>
> >> -- 
> >> Jeff Mahoney
> >> SUSE Labs
> >>
> > 
> > 
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> 
> 
> -- 
> Jeff Mahoney
> SUSE Labs
> 

Re: [PATCH] Btrfs: remove unnecessary code of chunk_root assignment in btrfs_read_chunk_tree.

[Sean Fu]

On Thu, Sep 08, 2016 at 11:25:48PM -0400, Jeff Mahoney wrote:
> On 9/8/16 11:08 PM, Sean Fu wrote:
> > On Tue, Sep 06, 2016 at 11:12:20AM -0400, Jeff Mahoney wrote:
> >> On 9/6/16 5:58 AM, David Sterba wrote:
> >>> On Mon, Sep 05, 2016 at 11:13:40PM -0400, Jeff Mahoney wrote:
> >>>>>> Since root is only used to get fs_info->chunk_root, why not use fs_info
> >>>>>> directly?
> >>>>>
> >>>>> Weird.  Exactly this was a part of my fs_info patchset.  I guess I need
> >>>>> to go back and check what else is missing.
> >>>>
> >>>> Actually, most of this didn't land.  Pretty much anything that's a root
> >>>> ->fs_info conversion is in there.
> >>>
> >>> Only half of the patchset has been merged so far because it did not pass
> >>> testing, so I bisected to some point. I was about to let you know once
> >>> most of 4.9 patches are prepared so there are less merge conflicts.
> >>
> >> Ok, thanks.  I was going to start the rebase today but I'll hold off
> >> until you're set for 4.9.
> >>
> > Hi Jeff, Could you please share your patch? Where can i get it?
> > I wanna have a look at it.
> 
> Sure, it's the whole series that starts with this commit:
> commit 160ceedfd40085cfb1e08305917fcc24cefdad93
> Author: Jeff Mahoney <jeffm@suse.com>
> Date:   Wed Aug 31 23:55:33 2016 -0400
> 
>     btrfs: add dynamic debug support
> 
> ... I still need to do clean up some commits that need merging.
> 
> https://git.kernel.org/cgit/linux/kernel/git/jeffm/linux-btrfs.git/log/?h=btrfs-testing/kdave/misc-4.9/root-fsinfo-cleanup
> 
Nice work. Thanks for your explaination.
I have one more question about it.
Although the total text size of this function is not changed, using
fs_info should need one more instruction to get chunk_root field of
fs_info than using chunk_root directly.
Does it cause any performance impact?

Sean
> -Jeff
> 
> 
> > Thanks
> >> -Jeff
> >>
> >> -- 
> >> Jeff Mahoney
> >> SUSE Labs
> >>
> > 
> > 
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> 
> 
> -- 
> Jeff Mahoney
> SUSE Labs
> 

Re: [PATCH] Btrfs: remove unnecessary code of chunk_root assignment in btrfs_read_chunk_tree.

[Jeff Mahoney]

[-- Attachment #1.1: Type: text/plain, Size: 2436 bytes --]

On 9/10/16 11:58 AM, Sean Fu wrote:
> On Thu, Sep 08, 2016 at 11:25:48PM -0400, Jeff Mahoney wrote:
>> On 9/8/16 11:08 PM, Sean Fu wrote:
>>> On Tue, Sep 06, 2016 at 11:12:20AM -0400, Jeff Mahoney wrote:
>>>> On 9/6/16 5:58 AM, David Sterba wrote:
>>>>> On Mon, Sep 05, 2016 at 11:13:40PM -0400, Jeff Mahoney wrote:
>>>>>>>> Since root is only used to get fs_info->chunk_root, why not use fs_info
>>>>>>>> directly?
>>>>>>>
>>>>>>> Weird.  Exactly this was a part of my fs_info patchset.  I guess I need
>>>>>>> to go back and check what else is missing.
>>>>>>
>>>>>> Actually, most of this didn't land.  Pretty much anything that's a root
>>>>>> ->fs_info conversion is in there.
>>>>>
>>>>> Only half of the patchset has been merged so far because it did not pass
>>>>> testing, so I bisected to some point. I was about to let you know once
>>>>> most of 4.9 patches are prepared so there are less merge conflicts.
>>>>
>>>> Ok, thanks.  I was going to start the rebase today but I'll hold off
>>>> until you're set for 4.9.
>>>>
>>> Hi Jeff, Could you please share your patch? Where can i get it?
>>> I wanna have a look at it.
>>
>> Sure, it's the whole series that starts with this commit:
>> commit 160ceedfd40085cfb1e08305917fcc24cefdad93
>> Author: Jeff Mahoney <jeffm@suse.com>
>> Date:   Wed Aug 31 23:55:33 2016 -0400
>>
>>     btrfs: add dynamic debug support
>>
>> ... I still need to do clean up some commits that need merging.
>>
>> https://git.kernel.org/cgit/linux/kernel/git/jeffm/linux-btrfs.git/log/?h=btrfs-testing/kdave/misc-4.9/root-fsinfo-cleanup
>>
> Nice work. Thanks for your explaination.
> I have one more question about it.
> Although the total text size of this function is not changed, using
> fs_info should need one more instruction to get chunk_root field of
> fs_info than using chunk_root directly.
> Does it cause any performance impact?

I doubt it.  The caller had to do the dereference before.

-Jeff

> Sean
>> -Jeff
>>
>>
>>> Thanks
>>>> -Jeff
>>>>
>>>> -- 
>>>> Jeff Mahoney
>>>> SUSE Labs
>>>>
>>>
>>>
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>>
>>
>> -- 
>> Jeff Mahoney
>> SUSE Labs
>>
> 
> 
> 
> 


-- 
Jeff Mahoney
SUSE Labs


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 881 bytes --]