diff mbox series

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

Message ID 980fa7926d0aa651c42a4ff4e58c0d644d712b7e.1663175597.git.josef@toxicpanda.com (mailing list archive)
State New, archived
Headers show
Series btrfs: split out larger chunks of ctree.h | expand

Commit Message

Josef Bacik Sept. 14, 2022, 5:18 p.m. UTC 
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

Comments

Anand Jain Sept. 19, 2022, 12:53 p.m. UTC | #1 
On 9/15/22 01:18, 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>

Reviewed-by: Anand Jain <anand.jain@oracle.com>
David Sterba Oct. 11, 2022, 10:39 a.m. UTC | #2 
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.
David Sterba Oct. 11, 2022, 11:37 a.m. UTC | #3 
On Tue, Oct 11, 2022 at 12:39:07PM +0200, David Sterba wrote:
> On Wed, Sep 14, 2022 at 01:18:20PM -0400, Josef Bacik wrote:
> > +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.

It looks simple in C but in asm it's about 27 instructions due to
page_address that does some masking and and check. Overall, uninlining
seems ok.
diff mbox series

Patch

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;
+}
+
 /*
  * Macro templates that define helpers to read/write extent buffer data of a
  * given size, that are also used via ctree.h for access to item members by
diff --git a/fs/btrfs/item-accessors.h b/fs/btrfs/item-accessors.h
new file mode 100644
index 000000000000..1f40aa503047
--- /dev/null
+++ b/fs/btrfs/item-accessors.h
@@ -0,0 +1,14 @@ 
+// SPDX-License-Identifier: GPL-2.0
+
+#ifndef BTRFS_ITEM_ACCESSORS_H
+#define BTRFS_ITEM_ACCESSORS_H
+
+struct btrfs_map_token {
+	struct extent_buffer *eb;
+	char *kaddr;
+	unsigned long offset;
+};
+
+void btrfs_init_map_token(struct btrfs_map_token *token,
+			  struct extent_buffer *eb);
+#endif /* BTRFS_ITEM_ACCESSORS_H */
diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 90cc8a97c13b..8818a2b19192 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -22,6 +22,7 @@ 
 #include "zoned.h"
 #include "inode-item.h"
 #include "fs.h"
+#include "item-accessors.h"
 
 #define MAX_CONFLICT_INODES 10