| Message ID | ca35ce34f64fc203266a7336390d82745d82ed5f.1734368270.git.fdmanana@suse.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | btrfs: some header cleanups and move things around | expand |
在 2024/12/17 03:47, fdmanana@kernel.org 写道: > From: Filipe Manana <fdmanana@suse.com> > > The ctree module is about the implementation of the btree data structure > and not a place holder for generic filesystem things like the csum > algorithm details. Move the functions related to the csum algorithm > details away from ctree.c and into fs.c, which is a far better place for > them. Also fix missing punctuation in comments and change one multiline > comment to a single line comment since everything fits in under 80 > characters. > > For some reason this also sligthly reduces the module's size. > > Before this change: > > $ size fs/btrfs/btrfs.ko > text data bss dec hex filename > 1782126 161045 16920 1960091 1de89b fs/btrfs/btrfs.ko > > After this change: > > $ size fs/btrfs/btrfs.ko > text data bss dec hex filename > 1782094 161045 16920 1960059 1de87b fs/btrfs/btrfs.ko > > Signed-off-by: Filipe Manana <fdmanana@suse.com> > --- > fs/btrfs/ctree.c | 51 ------------------------------------------------ > fs/btrfs/ctree.h | 6 ------ > fs/btrfs/fs.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++ > fs/btrfs/fs.h | 6 ++++++ > 4 files changed, 55 insertions(+), 57 deletions(-) > > diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c > index 99a58ede387e..c93f52a30a16 100644 > --- a/fs/btrfs/ctree.c > +++ b/fs/btrfs/ctree.c > @@ -37,19 +37,6 @@ static int push_node_left(struct btrfs_trans_handle *trans, > static int balance_node_right(struct btrfs_trans_handle *trans, > struct extent_buffer *dst_buf, > struct extent_buffer *src_buf); > - > -static const struct btrfs_csums { > - u16 size; > - const char name[10]; > - const char driver[12]; > -} btrfs_csums[] = { > - [BTRFS_CSUM_TYPE_CRC32] = { .size = 4, .name = "crc32c" }, > - [BTRFS_CSUM_TYPE_XXHASH] = { .size = 8, .name = "xxhash64" }, > - [BTRFS_CSUM_TYPE_SHA256] = { .size = 32, .name = "sha256" }, > - [BTRFS_CSUM_TYPE_BLAKE2] = { .size = 32, .name = "blake2b", > - .driver = "blake2b-256" }, > -}; > - > /* > * The leaf data grows from end-to-front in the node. this returns the address > * of the start of the last item, which is the stop of the leaf data stack. > @@ -148,44 +135,6 @@ static inline void copy_leaf_items(const struct extent_buffer *dst, > nr_items * sizeof(struct btrfs_item)); > } > > -/* This exists for btrfs-progs usages. */ > -u16 btrfs_csum_type_size(u16 type) > -{ > - return btrfs_csums[type].size; > -} > - > -int btrfs_super_csum_size(const struct btrfs_super_block *s) > -{ > - u16 t = btrfs_super_csum_type(s); > - /* > - * csum type is validated at mount time > - */ > - return btrfs_csum_type_size(t); > -} > - > -const char *btrfs_super_csum_name(u16 csum_type) > -{ > - /* csum type is validated at mount time */ > - return btrfs_csums[csum_type].name; > -} > - > -/* > - * Return driver name if defined, otherwise the name that's also a valid driver > - * name > - */ > -const char *btrfs_super_csum_driver(u16 csum_type) > -{ > - /* csum type is validated at mount time */ > - return btrfs_csums[csum_type].driver[0] ? > - btrfs_csums[csum_type].driver : > - btrfs_csums[csum_type].name; > -} > - > -size_t __attribute_const__ btrfs_get_num_csums(void) > -{ > - return ARRAY_SIZE(btrfs_csums); > -} > - > struct btrfs_path *btrfs_alloc_path(void) > { > might_sleep(); > diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h > index 2c341956a01c..a1bab0b3f193 100644 > --- a/fs/btrfs/ctree.h > +++ b/fs/btrfs/ctree.h > @@ -756,12 +756,6 @@ static inline bool btrfs_is_data_reloc_root(const struct btrfs_root *root) > return root->root_key.objectid == BTRFS_DATA_RELOC_TREE_OBJECTID; > } > > -u16 btrfs_csum_type_size(u16 type); > -int btrfs_super_csum_size(const struct btrfs_super_block *s); > -const char *btrfs_super_csum_name(u16 csum_type); > -const char *btrfs_super_csum_driver(u16 csum_type); > -size_t __attribute_const__ btrfs_get_num_csums(void); > - > /* > * We use folio flag owner_2 to indicate there is an ordered extent with > * unfinished IO. > diff --git a/fs/btrfs/fs.c b/fs/btrfs/fs.c > index 31c1648bc0b4..3756a3b9c9da 100644 > --- a/fs/btrfs/fs.c > +++ b/fs/btrfs/fs.c > @@ -5,6 +5,55 @@ > #include "fs.h" > #include "accessors.h" > > +static const struct btrfs_csums { > + u16 size; > + const char name[10]; > + const char driver[12]; > +} btrfs_csums[] = { > + [BTRFS_CSUM_TYPE_CRC32] = { .size = 4, .name = "crc32c" }, > + [BTRFS_CSUM_TYPE_XXHASH] = { .size = 8, .name = "xxhash64" }, > + [BTRFS_CSUM_TYPE_SHA256] = { .size = 32, .name = "sha256" }, > + [BTRFS_CSUM_TYPE_BLAKE2] = { .size = 32, .name = "blake2b", > + .driver = "blake2b-256" }, > +}; > + > +/* This exists for btrfs-progs usages. */ > +u16 btrfs_csum_type_size(u16 type) > +{ > + return btrfs_csums[type].size; > +} > + > +int btrfs_super_csum_size(const struct btrfs_super_block *s) > +{ > + u16 t = btrfs_super_csum_type(s); > + > + /* csum type is validated at mount time. */ > + return btrfs_csum_type_size(t); > +} > + > +const char *btrfs_super_csum_name(u16 csum_type) > +{ > + /* csum type is validated at mount time. */ > + return btrfs_csums[csum_type].name; > +} > + > +/* > + * Return driver name if defined, otherwise the name that's also a valid driver > + * name. > + */ > +const char *btrfs_super_csum_driver(u16 csum_type) > +{ > + /* csum type is validated at mount time */ > + return btrfs_csums[csum_type].driver[0] ? > + btrfs_csums[csum_type].driver : > + btrfs_csums[csum_type].name; > +} > + > +size_t __attribute_const__ btrfs_get_num_csums(void) > +{ > + return ARRAY_SIZE(btrfs_csums); > +} > + I'm wondering if those simple functions can be converted to inline. Although btrfs_csums[] array has to be put inside fs.c, those functions seems be fine defined as inline ones inside the header. Otherwise looks good to me. Thanks, Qu > void __btrfs_set_fs_incompat(struct btrfs_fs_info *fs_info, u64 flag, > const char *name) > { > diff --git a/fs/btrfs/fs.h b/fs/btrfs/fs.h > index 79a1a3d6f04d..b05f2af97140 100644 > --- a/fs/btrfs/fs.h > +++ b/fs/btrfs/fs.h > @@ -982,6 +982,12 @@ void btrfs_exclop_balance(struct btrfs_fs_info *fs_info, > > int btrfs_check_ioctl_vol_args_path(const struct btrfs_ioctl_vol_args *vol_args); > > +u16 btrfs_csum_type_size(u16 type); > +int btrfs_super_csum_size(const struct btrfs_super_block *s); > +const char *btrfs_super_csum_name(u16 csum_type); > +const char *btrfs_super_csum_driver(u16 csum_type); > +size_t __attribute_const__ btrfs_get_num_csums(void); > + > /* Compatibility and incompatibility defines */ > void __btrfs_set_fs_incompat(struct btrfs_fs_info *fs_info, u64 flag, > const char *name);
On Tue, Dec 17, 2024 at 12:26 AM Qu Wenruo <quwenruo.btrfs@gmx.com> wrote: > > > > 在 2024/12/17 03:47, fdmanana@kernel.org 写道: > > From: Filipe Manana <fdmanana@suse.com> > > > > The ctree module is about the implementation of the btree data structure > > and not a place holder for generic filesystem things like the csum > > algorithm details. Move the functions related to the csum algorithm > > details away from ctree.c and into fs.c, which is a far better place for > > them. Also fix missing punctuation in comments and change one multiline > > comment to a single line comment since everything fits in under 80 > > characters. > > > > For some reason this also sligthly reduces the module's size. > > > > Before this change: > > > > $ size fs/btrfs/btrfs.ko > > text data bss dec hex filename > > 1782126 161045 16920 1960091 1de89b fs/btrfs/btrfs.ko > > > > After this change: > > > > $ size fs/btrfs/btrfs.ko > > text data bss dec hex filename > > 1782094 161045 16920 1960059 1de87b fs/btrfs/btrfs.ko > > > > Signed-off-by: Filipe Manana <fdmanana@suse.com> > > --- > > fs/btrfs/ctree.c | 51 ------------------------------------------------ > > fs/btrfs/ctree.h | 6 ------ > > fs/btrfs/fs.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++ > > fs/btrfs/fs.h | 6 ++++++ > > 4 files changed, 55 insertions(+), 57 deletions(-) > > > > diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c > > index 99a58ede387e..c93f52a30a16 100644 > > --- a/fs/btrfs/ctree.c > > +++ b/fs/btrfs/ctree.c > > @@ -37,19 +37,6 @@ static int push_node_left(struct btrfs_trans_handle *trans, > > static int balance_node_right(struct btrfs_trans_handle *trans, > > struct extent_buffer *dst_buf, > > struct extent_buffer *src_buf); > > - > > -static const struct btrfs_csums { > > - u16 size; > > - const char name[10]; > > - const char driver[12]; > > -} btrfs_csums[] = { > > - [BTRFS_CSUM_TYPE_CRC32] = { .size = 4, .name = "crc32c" }, > > - [BTRFS_CSUM_TYPE_XXHASH] = { .size = 8, .name = "xxhash64" }, > > - [BTRFS_CSUM_TYPE_SHA256] = { .size = 32, .name = "sha256" }, > > - [BTRFS_CSUM_TYPE_BLAKE2] = { .size = 32, .name = "blake2b", > > - .driver = "blake2b-256" }, > > -}; > > - > > /* > > * The leaf data grows from end-to-front in the node. this returns the address > > * of the start of the last item, which is the stop of the leaf data stack. > > @@ -148,44 +135,6 @@ static inline void copy_leaf_items(const struct extent_buffer *dst, > > nr_items * sizeof(struct btrfs_item)); > > } > > > > -/* This exists for btrfs-progs usages. */ > > -u16 btrfs_csum_type_size(u16 type) > > -{ > > - return btrfs_csums[type].size; > > -} > > - > > -int btrfs_super_csum_size(const struct btrfs_super_block *s) > > -{ > > - u16 t = btrfs_super_csum_type(s); > > - /* > > - * csum type is validated at mount time > > - */ > > - return btrfs_csum_type_size(t); > > -} > > - > > -const char *btrfs_super_csum_name(u16 csum_type) > > -{ > > - /* csum type is validated at mount time */ > > - return btrfs_csums[csum_type].name; > > -} > > - > > -/* > > - * Return driver name if defined, otherwise the name that's also a valid driver > > - * name > > - */ > > -const char *btrfs_super_csum_driver(u16 csum_type) > > -{ > > - /* csum type is validated at mount time */ > > - return btrfs_csums[csum_type].driver[0] ? > > - btrfs_csums[csum_type].driver : > > - btrfs_csums[csum_type].name; > > -} > > - > > -size_t __attribute_const__ btrfs_get_num_csums(void) > > -{ > > - return ARRAY_SIZE(btrfs_csums); > > -} > > - > > struct btrfs_path *btrfs_alloc_path(void) > > { > > might_sleep(); > > diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h > > index 2c341956a01c..a1bab0b3f193 100644 > > --- a/fs/btrfs/ctree.h > > +++ b/fs/btrfs/ctree.h > > @@ -756,12 +756,6 @@ static inline bool btrfs_is_data_reloc_root(const struct btrfs_root *root) > > return root->root_key.objectid == BTRFS_DATA_RELOC_TREE_OBJECTID; > > } > > > > -u16 btrfs_csum_type_size(u16 type); > > -int btrfs_super_csum_size(const struct btrfs_super_block *s); > > -const char *btrfs_super_csum_name(u16 csum_type); > > -const char *btrfs_super_csum_driver(u16 csum_type); > > -size_t __attribute_const__ btrfs_get_num_csums(void); > > - > > /* > > * We use folio flag owner_2 to indicate there is an ordered extent with > > * unfinished IO. > > diff --git a/fs/btrfs/fs.c b/fs/btrfs/fs.c > > index 31c1648bc0b4..3756a3b9c9da 100644 > > --- a/fs/btrfs/fs.c > > +++ b/fs/btrfs/fs.c > > @@ -5,6 +5,55 @@ > > #include "fs.h" > > #include "accessors.h" > > > > +static const struct btrfs_csums { > > + u16 size; > > + const char name[10]; > > + const char driver[12]; > > +} btrfs_csums[] = { > > + [BTRFS_CSUM_TYPE_CRC32] = { .size = 4, .name = "crc32c" }, > > + [BTRFS_CSUM_TYPE_XXHASH] = { .size = 8, .name = "xxhash64" }, > > + [BTRFS_CSUM_TYPE_SHA256] = { .size = 32, .name = "sha256" }, > > + [BTRFS_CSUM_TYPE_BLAKE2] = { .size = 32, .name = "blake2b", > > + .driver = "blake2b-256" }, > > +}; > > + > > +/* This exists for btrfs-progs usages. */ > > +u16 btrfs_csum_type_size(u16 type) > > +{ > > + return btrfs_csums[type].size; > > +} > > + > > +int btrfs_super_csum_size(const struct btrfs_super_block *s) > > +{ > > + u16 t = btrfs_super_csum_type(s); > > + > > + /* csum type is validated at mount time. */ > > + return btrfs_csum_type_size(t); > > +} > > + > > +const char *btrfs_super_csum_name(u16 csum_type) > > +{ > > + /* csum type is validated at mount time. */ > > + return btrfs_csums[csum_type].name; > > +} > > + > > +/* > > + * Return driver name if defined, otherwise the name that's also a valid driver > > + * name. > > + */ > > +const char *btrfs_super_csum_driver(u16 csum_type) > > +{ > > + /* csum type is validated at mount time */ > > + return btrfs_csums[csum_type].driver[0] ? > > + btrfs_csums[csum_type].driver : > > + btrfs_csums[csum_type].name; > > +} > > + > > +size_t __attribute_const__ btrfs_get_num_csums(void) > > +{ > > + return ARRAY_SIZE(btrfs_csums); > > +} > > + > > I'm wondering if those simple functions can be converted to inline. > > Although btrfs_csums[] array has to be put inside fs.c, those functions > seems be fine defined as inline ones inside the header. Yes, the array being inside fs.c makes it hard to inline in the header. Keep in mind that this is a change just to move things around, the goal is not to change the implementation or optimize anything. Plus it's not like these functions are called in hot code paths where saving a function call has any significant impact. Thanks. > > Otherwise looks good to me. > > Thanks, > Qu > > void __btrfs_set_fs_incompat(struct btrfs_fs_info *fs_info, u64 flag, > > const char *name) > > { > > diff --git a/fs/btrfs/fs.h b/fs/btrfs/fs.h > > index 79a1a3d6f04d..b05f2af97140 100644 > > --- a/fs/btrfs/fs.h > > +++ b/fs/btrfs/fs.h > > @@ -982,6 +982,12 @@ void btrfs_exclop_balance(struct btrfs_fs_info *fs_info, > > > > int btrfs_check_ioctl_vol_args_path(const struct btrfs_ioctl_vol_args *vol_args); > > > > +u16 btrfs_csum_type_size(u16 type); > > +int btrfs_super_csum_size(const struct btrfs_super_block *s); > > +const char *btrfs_super_csum_name(u16 csum_type); > > +const char *btrfs_super_csum_driver(u16 csum_type); > > +size_t __attribute_const__ btrfs_get_num_csums(void); > > + > > /* Compatibility and incompatibility defines */ > > void __btrfs_set_fs_incompat(struct btrfs_fs_info *fs_info, u64 flag, > > const char *name); >
diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c index 99a58ede387e..c93f52a30a16 100644 --- a/fs/btrfs/ctree.c +++ b/fs/btrfs/ctree.c @@ -37,19 +37,6 @@ static int push_node_left(struct btrfs_trans_handle *trans, static int balance_node_right(struct btrfs_trans_handle *trans, struct extent_buffer *dst_buf, struct extent_buffer *src_buf); - -static const struct btrfs_csums { - u16 size; - const char name[10]; - const char driver[12]; -} btrfs_csums[] = { - [BTRFS_CSUM_TYPE_CRC32] = { .size = 4, .name = "crc32c" }, - [BTRFS_CSUM_TYPE_XXHASH] = { .size = 8, .name = "xxhash64" }, - [BTRFS_CSUM_TYPE_SHA256] = { .size = 32, .name = "sha256" }, - [BTRFS_CSUM_TYPE_BLAKE2] = { .size = 32, .name = "blake2b", - .driver = "blake2b-256" }, -}; - /* * The leaf data grows from end-to-front in the node. this returns the address * of the start of the last item, which is the stop of the leaf data stack. @@ -148,44 +135,6 @@ static inline void copy_leaf_items(const struct extent_buffer *dst, nr_items * sizeof(struct btrfs_item)); } -/* This exists for btrfs-progs usages. */ -u16 btrfs_csum_type_size(u16 type) -{ - return btrfs_csums[type].size; -} - -int btrfs_super_csum_size(const struct btrfs_super_block *s) -{ - u16 t = btrfs_super_csum_type(s); - /* - * csum type is validated at mount time - */ - return btrfs_csum_type_size(t); -} - -const char *btrfs_super_csum_name(u16 csum_type) -{ - /* csum type is validated at mount time */ - return btrfs_csums[csum_type].name; -} - -/* - * Return driver name if defined, otherwise the name that's also a valid driver - * name - */ -const char *btrfs_super_csum_driver(u16 csum_type) -{ - /* csum type is validated at mount time */ - return btrfs_csums[csum_type].driver[0] ? - btrfs_csums[csum_type].driver : - btrfs_csums[csum_type].name; -} - -size_t __attribute_const__ btrfs_get_num_csums(void) -{ - return ARRAY_SIZE(btrfs_csums); -} - struct btrfs_path *btrfs_alloc_path(void) { might_sleep(); diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h index 2c341956a01c..a1bab0b3f193 100644 --- a/fs/btrfs/ctree.h +++ b/fs/btrfs/ctree.h @@ -756,12 +756,6 @@ static inline bool btrfs_is_data_reloc_root(const struct btrfs_root *root) return root->root_key.objectid == BTRFS_DATA_RELOC_TREE_OBJECTID; } -u16 btrfs_csum_type_size(u16 type); -int btrfs_super_csum_size(const struct btrfs_super_block *s); -const char *btrfs_super_csum_name(u16 csum_type); -const char *btrfs_super_csum_driver(u16 csum_type); -size_t __attribute_const__ btrfs_get_num_csums(void); - /* * We use folio flag owner_2 to indicate there is an ordered extent with * unfinished IO. diff --git a/fs/btrfs/fs.c b/fs/btrfs/fs.c index 31c1648bc0b4..3756a3b9c9da 100644 --- a/fs/btrfs/fs.c +++ b/fs/btrfs/fs.c @@ -5,6 +5,55 @@ #include "fs.h" #include "accessors.h" +static const struct btrfs_csums { + u16 size; + const char name[10]; + const char driver[12]; +} btrfs_csums[] = { + [BTRFS_CSUM_TYPE_CRC32] = { .size = 4, .name = "crc32c" }, + [BTRFS_CSUM_TYPE_XXHASH] = { .size = 8, .name = "xxhash64" }, + [BTRFS_CSUM_TYPE_SHA256] = { .size = 32, .name = "sha256" }, + [BTRFS_CSUM_TYPE_BLAKE2] = { .size = 32, .name = "blake2b", + .driver = "blake2b-256" }, +}; + +/* This exists for btrfs-progs usages. */ +u16 btrfs_csum_type_size(u16 type) +{ + return btrfs_csums[type].size; +} + +int btrfs_super_csum_size(const struct btrfs_super_block *s) +{ + u16 t = btrfs_super_csum_type(s); + + /* csum type is validated at mount time. */ + return btrfs_csum_type_size(t); +} + +const char *btrfs_super_csum_name(u16 csum_type) +{ + /* csum type is validated at mount time. */ + return btrfs_csums[csum_type].name; +} + +/* + * Return driver name if defined, otherwise the name that's also a valid driver + * name. + */ +const char *btrfs_super_csum_driver(u16 csum_type) +{ + /* csum type is validated at mount time */ + return btrfs_csums[csum_type].driver[0] ? + btrfs_csums[csum_type].driver : + btrfs_csums[csum_type].name; +} + +size_t __attribute_const__ btrfs_get_num_csums(void) +{ + return ARRAY_SIZE(btrfs_csums); +} + void __btrfs_set_fs_incompat(struct btrfs_fs_info *fs_info, u64 flag, const char *name) { diff --git a/fs/btrfs/fs.h b/fs/btrfs/fs.h index 79a1a3d6f04d..b05f2af97140 100644 --- a/fs/btrfs/fs.h +++ b/fs/btrfs/fs.h @@ -982,6 +982,12 @@ void btrfs_exclop_balance(struct btrfs_fs_info *fs_info, int btrfs_check_ioctl_vol_args_path(const struct btrfs_ioctl_vol_args *vol_args); +u16 btrfs_csum_type_size(u16 type); +int btrfs_super_csum_size(const struct btrfs_super_block *s); +const char *btrfs_super_csum_name(u16 csum_type); +const char *btrfs_super_csum_driver(u16 csum_type); +size_t __attribute_const__ btrfs_get_num_csums(void); + /* Compatibility and incompatibility defines */ void __btrfs_set_fs_incompat(struct btrfs_fs_info *fs_info, u64 flag, const char *name);