Re: [PATCH 15/16] btrfs: move btrfs_map_token to item-accessors

[David Sterba <dsterba@suse.cz>]

On Wed, Sep 14, 2022 at 01:18:20PM -0400, Josef Bacik wrote:
> This is specific to the item-accessor code, move it out of ctree.h into
> item-accessor.h/.c and then update the users to include the new header
> file.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
>  fs/btrfs/ctree.c          |  1 +
>  fs/btrfs/ctree.h          | 16 ++--------------
>  fs/btrfs/inode.c          |  1 +
>  fs/btrfs/item-accessors.c |  9 +++++++++
>  fs/btrfs/item-accessors.h | 14 ++++++++++++++
>  fs/btrfs/tree-log.c       |  1 +
>  6 files changed, 28 insertions(+), 14 deletions(-)
>  create mode 100644 fs/btrfs/item-accessors.h
> 
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index a5fd4e2369f1..de01c892a885 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -19,6 +19,7 @@
>  #include "tree-mod-log.h"
>  #include "tree-checker.h"
>  #include "fs.h"
> +#include "item-accessors.h"
>  
>  static struct kmem_cache *btrfs_path_cachep;
>  
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index b6e86c1bc4b2..324400c5597f 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -46,6 +46,8 @@ struct btrfs_ref;
>  struct btrfs_bio;
>  struct btrfs_ioctl_encoded_io_args;
>  
> +struct btrfs_map_token;
> +
>  #define BTRFS_OLDEST_GENERATION	0ULL
>  
>  #define BTRFS_EMPTY_DIR_SIZE 0
> @@ -1174,23 +1176,9 @@ static inline u32 BTRFS_MAX_XATTR_SIZE(const struct btrfs_fs_info *info)
>  	return BTRFS_MAX_ITEM_SIZE(info) - sizeof(struct btrfs_dir_item);
>  }
>  
> -struct btrfs_map_token {
> -	struct extent_buffer *eb;
> -	char *kaddr;
> -	unsigned long offset;
> -};
> -
>  #define BTRFS_BYTES_TO_BLKS(fs_info, bytes) \
>  				((bytes) >> (fs_info)->sectorsize_bits)
>  
> -static inline void btrfs_init_map_token(struct btrfs_map_token *token,
> -					struct extent_buffer *eb)
> -{
> -	token->eb = eb;
> -	token->kaddr = page_address(eb->pages[0]);
> -	token->offset = 0;
> -}
> -
>  /* some macros to generate set/get functions for the struct fields.  This
>   * assumes there is a lefoo_to_cpu for every type, so lets make a simple
>   * one for u8:
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index a6615106002a..ca6fa9b01785 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -56,6 +56,7 @@
>  #include "subpage.h"
>  #include "inode-item.h"
>  #include "fs.h"
> +#include "item-accessors.h"
>  
>  struct btrfs_iget_args {
>  	u64 ino;
> diff --git a/fs/btrfs/item-accessors.c b/fs/btrfs/item-accessors.c
> index ec6b0436d09a..7edb59ba99fc 100644
> --- a/fs/btrfs/item-accessors.c
> +++ b/fs/btrfs/item-accessors.c
> @@ -7,6 +7,7 @@
>  
>  #include "btrfs-printk.h"
>  #include "ctree.h"
> +#include "item-accessors.h"
>  
>  static bool check_setget_bounds(const struct extent_buffer *eb,
>  				const void *ptr, unsigned off, int size)
> @@ -24,6 +25,14 @@ static bool check_setget_bounds(const struct extent_buffer *eb,
>  	return true;
>  }
>  
> +void btrfs_init_map_token(struct btrfs_map_token *token,
> +			  struct extent_buffer *eb)
> +{
> +	token->eb = eb;
> +	token->kaddr = page_address(eb->pages[0]);
> +	token->offset = 0;

This is prooobably ok to be uninlined, it's called once when used and
all call sites typically do lot of work using the token but still a
function call for three simple assignments does not look optimal.