Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1476065 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  RE: [PATCH] Btrfs: remove unnecessary code of chunk_root assignment in btrfs_read_chunk_tree. Zhao Lei <zhaolei@cn.fujitsu.com> - 2016-09-05 03:20 +0200
    Re: [PATCH] Btrfs: remove unnecessary code of chunk_root assignment  in btrfs_read_chunk_tree. Qu Wenruo <quwenruo@cn.fujitsu.com> - 2016-09-05 10:00 +0200
      RE: [PATCH] Btrfs: remove unnecessary code of chunk_root assignment in btrfs_read_chunk_tree. Zhao Lei <zhaolei@cn.fujitsu.com> - 2016-09-06 04:50 +0200
      Re: [PATCH] Btrfs: remove unnecessary code of chunk_root assignment  in btrfs_read_chunk_tree. Jeff Mahoney <jeffm@suse.com> - 2016-09-06 05:10 +0200
        Re: [PATCH] Btrfs: remove unnecessary code of chunk_root assignment  in btrfs_read_chunk_tree. Jeff Mahoney <jeffm@suse.com> - 2016-09-06 05:40 +0200
          Re: [PATCH] Btrfs: remove unnecessary code of chunk_root assignment  in btrfs_read_chunk_tree. David Sterba <dsterba@suse.cz> - 2016-09-06 12:10 +0200
            Re: [PATCH] Btrfs: remove unnecessary code of chunk_root assignment  in btrfs_read_chunk_tree. Jeff Mahoney <jeffm@suse.com> - 2016-09-06 17:20 +0200
              Re: [PATCH] Btrfs: remove unnecessary code of chunk_root assignment  in btrfs_read_chunk_tree. Sean Fu <fxinrong@gmail.com> - 2016-09-07 03:50 +0200
              Re: [PATCH] Btrfs: remove unnecessary code of chunk_root assignment  in btrfs_read_chunk_tree. Sean Fu <fxinrong@gmail.com> - 2016-09-09 05:10 +0200
                Re: [PATCH] Btrfs: remove unnecessary code of chunk_root assignment  in btrfs_read_chunk_tree. Jeff Mahoney <jeffm@suse.com> - 2016-09-09 05:30 +0200
                  Re: [PATCH] Btrfs: remove unnecessary code of chunk_root assignment  in btrfs_read_chunk_tree. Sean Fu <fxinrong@gmail.com> - 2016-09-09 05:50 +0200
      Re: [PATCH] Btrfs: remove unnecessary code of chunk_root assignment  in btrfs_read_chunk_tree. Sean Fu <fxinrong@gmail.com> - 2016-09-07 03:40 +0200
        Re: [PATCH] Btrfs: remove unnecessary code of chunk_root assignment  in btrfs_read_chunk_tree. Qu Wenruo <quwenruo@cn.fujitsu.com> - 2016-09-07 04:00 +0200

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

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
> 

[toc] | [next] | [standalone]

#1476172 — 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?

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
>

[toc] | [prev] | [next] | [standalone]

#1477033

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

[toc] | [prev] | [next] | [standalone]

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

[Multipart message — attachments visible in raw view] — view raw

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

[toc] | [prev] | [next] | [standalone]

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

[Multipart message — attachments visible in raw view] — view raw

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

[toc] | [prev] | [next] | [standalone]

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

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.

[toc] | [prev] | [next] | [standalone]

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

[Multipart message — attachments visible in raw view] — view raw

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

[toc] | [prev] | [next] | [standalone]

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

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
> 

[toc] | [prev] | [next] | [standalone]

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

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
> 

[toc] | [prev] | [next] | [standalone]

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

[Multipart message — attachments visible in raw view] — view raw

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

[toc] | [prev] | [next] | [standalone]

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

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
> 

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web