Btrfs devel - Aug 2011 - [patch] Move nodesize/leafsize/sectorsize to fs_info

moved nodesize, leafsize and sectorsize from btrfs_root to
btrfs_fs_info as mentioned in [1] and updated everything accordingly.
patch against the "for-linus" tree
builds and loads, no further testing done.


[1] https://btrfs.wiki.kernel.org/index.php/Cleanup_ideas

ps: "Pass fs_info instead of root" is next.

Hi,

sorry for late reply. This patch tries to unify node-/leaf-/... sizes
nad put it just into fs_info, but this assumes all trees share the same
sizes. Unfortunatelly this is not true (once we allow big blocks; soon?).

The root tree has a hardcoded size of 4k, see the __setup_root called
with 4k in open_ctree:

1760         __setup_root(4096, 4096, 4096, 4096, tree_root,
1761                      fs_info, BTRFS_ROOT_TREE_OBJECTID);

the other trees have size defined during mkfs. And only these trees
share the size. You''d have to add exception to use 4k or ->leafsize
depending on the tree being used. It''s less error prone to always just
the structure item as it will hold always the correct value, although
there is some memory wasted.

From the peformace POV, the ->leafsize etc items can cause cacheline
bouncing when any of the code wants to access the item (and it''s not
that rare) for a different tree / cpu. But anyway with current
performance status, this effect will be hardly measurable.

On Sun, Aug 07, 2011 at 11:54:31PM +0200, Peeters Simon
wrote:
> [1] https://btrfs.wiki.kernel.org/index.php/Cleanup_ideas
I''d like to see how this idea was expected to be implemented, but
there''s no wiki to check.


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

On 10/26/2011 03:34 PM, David Sterba wrote:
> Hi,
>
> sorry for late reply. This patch tries to unify node-/leaf-/... sizes
> nad put it just into fs_info, but this assumes all trees share the same
> sizes. Unfortunatelly this is not true (once we allow big blocks; soon?).
>
> The root tree has a hardcoded size of 4k, see the __setup_root called
> with 4k in open_ctree:
>
> 1760         __setup_root(4096, 4096, 4096, 4096, tree_root,
> 1761                      fs_info, BTRFS_ROOT_TREE_OBJECTID);
>
> the other trees have size defined during mkfs. And only these trees
> share the size. You''d have to add exception to use 4k or
->leafsize
> depending on the tree being used. It''s less error prone to always
just
> the structure item as it will hold always the correct value, although
> there is some memory wasted.
The sizes get initialized to 4096, but after the super block is read,
these are replaced by those from the SB.
>
>  From the peformace POV, the ->leafsize etc items can cause cacheline
> bouncing when any of the code wants to access the item (and it''s
not
> that rare) for a different tree / cpu. But anyway with current
> performance status, this effect will be hardly measurable.
>
> On Sun, Aug 07, 2011 at 11:54:31PM +0200, Peeters Simon wrote:
>> [1] https://btrfs.wiki.kernel.org/index.php/Cleanup_ideas
>
> I''d like to see how this idea was expected to be implemented, but
> there''s no wiki to check.
It was indeed meant to get rid of the possibility to have different
sizes for different trees, as it just adds complexity. Chris mentioned
once that he does not intend to allow different sizes between trees,
thus the idea for this cleanup.

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

On Wed, Oct 26, 2011 at 04:47:15PM +0200, Arne Jansen
wrote:
> The sizes get initialized to 4096, but after the super block is read,
> these are replaced by those from the SB.
[reads sources again] right, and the initial values are not used up to
that point, so 4096 could be any number.
> It was indeed meant to get rid of the possibility to have different
> sizes for different trees, as it just adds complexity. Chris mentioned
> once that he does not intend to allow different sizes between trees,
> thus the idea for this cleanup.
Ok then. I''ll gather it to cleanup patch queue.


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

On Wed, Oct 26, 2011 at 06:23:38PM +0200, David Sterba
wrote:
> On Wed, Oct 26, 2011 at 04:47:15PM +0200, Arne Jansen wrote:
> > The sizes get initialized to 4096, but after the super block is read,
> > these are replaced by those from the SB.
> 
> [reads sources again] right, and the initial values are not used up to
> that point, so 4096 could be any number.
> 
> > It was indeed meant to get rid of the possibility to have different
> > sizes for different trees, as it just adds complexity. Chris mentioned
> > once that he does not intend to allow different sizes between trees,
> > thus the idea for this cleanup.
> 
> Ok then. I''ll gather it to cleanup patch queue.
Thanks, I actually don''t intend to allow different leaf and node sizes
anymore either, but we might as well keep both numbers.

-chris

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

On 10/26/2011 06:27 PM, Chris Mason wrote:
> On Wed, Oct 26, 2011 at 06:23:38PM +0200, David Sterba wrote:
>> On Wed, Oct 26, 2011 at 04:47:15PM +0200, Arne Jansen wrote:
>>> The sizes get initialized to 4096, but after the super block is
read,
>>> these are replaced by those from the SB.
>>
>> [reads sources again] right, and the initial values are not used up to
>> that point, so 4096 could be any number.
>>
>>> It was indeed meant to get rid of the possibility to have different
>>> sizes for different trees, as it just adds complexity. Chris
mentioned
>>> once that he does not intend to allow different sizes between
trees,
>>> thus the idea for this cleanup.
>>
>> Ok then. I''ll gather it to cleanup patch queue.
>
> Thanks, I actually don''t intend to allow different leaf and node
sizes
> anymore either, but we might as well keep both numbers.
So that would be another good cleanup, as it is not always easy to tell
if a block is a node or a leaf. It would actually simplify things.

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

Hi Simon,

On Sun, Aug 07, 2011 at 11:54:31PM +0200, Peeters Simon
wrote:
> moved nodesize, leafsize and sectorsize from btrfs_root to
> btrfs_fs_info as mentioned in [1] and updated everything accordingly.
> patch against the "for-linus" tree
> builds and loads, no further testing done.
Please add a "btrfs:" prefix to the subject line of the patch and add
your "Signed-off-by:" line, and also revise the changelog, so it
describes what the patch does and why, no need to reference the wiki
there (you can add such information after a ''---'' marker
following the
changelog).

Please rebase your patch on top of Josef''s for-chris branch, the
sources
have evolved since your first posting (though the fixup is trivial, but
the process still needs your S-O-B line).


thanks,
david
--
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