On Fri, Aug 13, 2010 at 9:55 AM, Cheng Renquan <crquan@xxxxxxxxx> wrote:
> The commit 607d432d referred a static int array defined in ctree.h,
> and a static inline function (btrfs_super_csum_size) using this array,
> the obvious problem is every c file using that function would have a
> local copy of that int array, multiple c files calling would result
> multiple copies of that array:
>
> $ nm fs/btrfs/btrfs.ko | grep btrfs_csum_sizes
> 0000010c r btrfs_csum_sizes
> 00000114 r btrfs_csum_sizes
> 000001c0 r btrfs_csum_sizes
> 000005a0 r btrfs_csum_sizes
>
> the original commit has 4 c files called this static inline function,
> till now there are still those 4 c files calling it, so there are 4 copies
> of btrfs_csum_sizes; but future code may call it in more c files,
> resulting in more copies;
>
> fs/btrfs/ctree.h | 19 ++++++++++++++++-
> fs/btrfs/disk-io.c | 25 +++++++++++++++++----
> fs/btrfs/file-item.c | 56 ++++++++++++++++++++++++++++---------------------
> fs/btrfs/ioctl.c | 9 ++++---
> fs/btrfs/tree-log.c | 10 +++++---
> 5 files changed, 81 insertions(+), 38 deletions(-)
>
> multiple copies are just wasting memory; move it to a c file can avoid
> duplications; and since the inline function referred ARRAY_SIZE of that
> array, must know the array size at compile time, so cannot be inlined
> anyway.
>
> The cost is originally inlined function calling changed to external function
> calling.
>
> Signed-off-by: Cheng Renquan <crquan@xxxxxxxxx>
Ping,
It was two weeks passed, and someone has some comments? Thanks,
> ---
> fs/btrfs/ctree.c | 9 +++++++++
> fs/btrfs/ctree.h | 9 +--------
> 2 files changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index c3df14c..3a89207 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -24,6 +24,15 @@
> #include "print-tree.h"
> #include "locking.h"
>
> +int btrfs_super_csum_size(struct btrfs_super_block *s)
> +{
> + static const int btrfs_csum_sizes[] = { 4, 0 };
> +
> + int t = btrfs_super_csum_type(s);
> + BUG_ON(t >= ARRAY_SIZE(btrfs_csum_sizes));
> + return btrfs_csum_sizes[t];
> +}
> +
> static int split_node(struct btrfs_trans_handle *trans, struct btrfs_root
> *root, struct btrfs_path *path, int level);
> static int split_leaf(struct btrfs_trans_handle *trans, struct btrfs_root
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index e9bf864..99220ee 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -132,8 +132,6 @@ struct btrfs_ordered_sum;
> /* csum types */
> #define BTRFS_CSUM_TYPE_CRC32 0
>
> -static int btrfs_csum_sizes[] = { 4, 0 };
> -
> /* four bytes for CRC32 */
> #define BTRFS_EMPTY_DIR_SIZE 0
>
> @@ -1877,12 +1875,7 @@ BTRFS_SETGET_STACK_FUNCS(super_incompat_flags, struct btrfs_super_block,
> BTRFS_SETGET_STACK_FUNCS(super_csum_type, struct btrfs_super_block,
> csum_type, 16);
>
> -static inline int btrfs_super_csum_size(struct btrfs_super_block *s)
> -{
> - int t = btrfs_super_csum_type(s);
> - BUG_ON(t >= ARRAY_SIZE(btrfs_csum_sizes));
> - return btrfs_csum_sizes[t];
> -}
> +int btrfs_super_csum_size(struct btrfs_super_block *s);
>
> static inline unsigned long btrfs_leaf_data(struct extent_buffer *l)
> {
> --
--
Cheng Renquan (程任全), from Singapore
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
