[PATCH 0/2] btrfs: optimize btrfs_ino()

[PATCH 0/2] btrfs: optimize btrfs_ino()

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

The following patchset optimizes btrfs_ino(), an inline function that is
used pretty much everywhere, to avoid doing less work and be trivial on
64 bits systems. Details on the changelogs of the patches.

Filipe Manana (2):
  btrfs: set the objectid of the btree inode's location key
  btrfs: add optimized btrfs_ino() version for 64 bits systems

 fs/btrfs/btrfs_inode.h       | 22 +++++++++++++++++-----
 fs/btrfs/disk-io.c           |  4 +++-
 fs/btrfs/tests/btrfs-tests.c |  1 +
 3 files changed, 21 insertions(+), 6 deletions(-)

-- 
2.35.1

[PATCH 1/2] btrfs: set the objectid of the btree inode's location key

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

We currently don't use the location key of the btree inode, its content
is set to zeroes, as it's a special inode that is not persisted (it has
no inode item stored in any btree).

At btrfs_ino(), an inline function used extensively in btrfs, we have
this special check if the given inode's location objectid is 0, and if it
is, we return the value stored in the VFS' inode i_ino field instead
(which is BTRFS_BTREE_INODE_OBJECTID for the btree inode).

To reduce the code at btrfs_ino(), we can simply set the objectid of the
btree inode to the value BTRFS_BTREE_INODE_OBJECTID. This eliminates the
need to check for the special case of the objectid being zero, with the
side effect of reducing the overall code size and having less code to
execute, as btrfs_ino() is an inline function.

Before:

$ size fs/btrfs/btrfs.ko
   text	   data	    bss	    dec	    hex	filename
1620502	 189240	  29032	1838774	 1c0eb6	fs/btrfs/btrfs.ko

After:

$ size fs/btrfs/btrfs.ko
   text	   data	    bss	    dec	    hex	filename
1617487	 189240	  29032	1835759	 1c02ef	fs/btrfs/btrfs.ko

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/btrfs_inode.h | 7 ++-----
 fs/btrfs/disk-io.c     | 4 +++-
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
index b467264bd1bb..a18f90ff16f1 100644
--- a/fs/btrfs/btrfs_inode.h
+++ b/fs/btrfs/btrfs_inode.h
@@ -283,11 +283,8 @@ static inline u64 btrfs_ino(const struct btrfs_inode *inode)
 {
 	u64 ino = inode->location.objectid;
 
-	/*
-	 * !ino: btree_inode
-	 * type == BTRFS_ROOT_ITEM_KEY: subvol dir
-	 */
-	if (!ino || inode->location.type == BTRFS_ROOT_ITEM_KEY)
+	/* type == BTRFS_ROOT_ITEM_KEY: subvol dir */
+	if (inode->location.type == BTRFS_ROOT_ITEM_KEY)
 		ino = inode->vfs_inode.i_ino;
 	return ino;
 }
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 0da86fba370e..b42908ad4af7 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -2306,7 +2306,9 @@ static void btrfs_init_btree_inode(struct btrfs_fs_info *fs_info)
 	extent_map_tree_init(&BTRFS_I(inode)->extent_tree);
 
 	BTRFS_I(inode)->root = btrfs_grab_root(fs_info->tree_root);
-	memset(&BTRFS_I(inode)->location, 0, sizeof(struct btrfs_key));
+	BTRFS_I(inode)->location.objectid = BTRFS_BTREE_INODE_OBJECTID;
+	BTRFS_I(inode)->location.type = 0;
+	BTRFS_I(inode)->location.offset = 0;
 	set_bit(BTRFS_INODE_DUMMY, &BTRFS_I(inode)->runtime_flags);
 	btrfs_insert_inode_hash(inode);
 }
-- 
2.35.1

Re: [PATCH 1/2] btrfs: set the objectid of the btree inode's location key

[Nikolay Borisov]

On 11.07.22 г. 17:22 ч., fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> We currently don't use the location key of the btree inode, its content
> is set to zeroes, as it's a special inode that is not persisted (it has
> no inode item stored in any btree).
> 
> At btrfs_ino(), an inline function used extensively in btrfs, we have
> this special check if the given inode's location objectid is 0, and if it
> is, we return the value stored in the VFS' inode i_ino field instead
> (which is BTRFS_BTREE_INODE_OBJECTID for the btree inode).
> 
> To reduce the code at btrfs_ino(), we can simply set the objectid of the
> btree inode to the value BTRFS_BTREE_INODE_OBJECTID. This eliminates the
> need to check for the special case of the objectid being zero, with the
> side effect of reducing the overall code size and having less code to
> execute, as btrfs_ino() is an inline function.
> 
> Before:
> 
> $ size fs/btrfs/btrfs.ko
>     text	   data	    bss	    dec	    hex	filename
> 1620502	 189240	  29032	1838774	 1c0eb6	fs/btrfs/btrfs.ko
> 
> After:
> 
> $ size fs/btrfs/btrfs.ko
>     text	   data	    bss	    dec	    hex	filename
> 1617487	 189240	  29032	1835759	 1c02ef	fs/btrfs/btrfs.ko
> 
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

Cool, that's around 3k of code savings.

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

[PATCH 2/2] btrfs: add optimized btrfs_ino() version for 64 bits systems

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

Currently btrfs_ino() tries to use first the objectid of the inode's
location key. This is to avoid truncation of the inode number on 32 bits
platforms because the i_ino field of struct inode has the unsigned long
type, while the objectid is a 64 bits unsigned type (u64) on every system.
This logic was added in commit 33345d01522f81 ("Btrfs: Always use 64bit
inode number").

However if we are running on a 64 bits system, we can always directly
return the i_ino value from struct inode, which eliminates the need for
he special if statement that tests for a location key type of
BTRFS_ROOT_ITEM_KEY - in which case i_ino may not have the same value as
the objectid in the inode's location objectid, it may have a value of
BTRFS_EMPTY_SUBVOL_DIR_OBJECTID, for the case of snapshots of trees with
subvolumes/snapshots inside them.

So add a special version for 64 bits system that directly returns i_ino
of struct inode. This eliminates one branch and reduces the overall code
size, since btrfs_ino() is an inline function that is extensively used.

Before:

$ size fs/btrfs/btrfs.ko
   text	   data	    bss	    dec	    hex	filename
1617487	 189240	  29032	1835759	 1c02ef	fs/btrfs/btrfs.ko

After:

$ size fs/btrfs/btrfs.ko
   text	   data	    bss	    dec	    hex	filename
1612028	 189180	  29032	1830240	 1bed60	fs/btrfs/btrfs.ko

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/btrfs_inode.h       | 15 +++++++++++++++
 fs/btrfs/tests/btrfs-tests.c |  1 +
 2 files changed, 16 insertions(+)

diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
index a18f90ff16f1..e38d0f09b89f 100644
--- a/fs/btrfs/btrfs_inode.h
+++ b/fs/btrfs/btrfs_inode.h
@@ -279,6 +279,12 @@ static inline void btrfs_insert_inode_hash(struct inode *inode)
 	__insert_inode_hash(inode, h);
 }
 
+#if BITS_PER_LONG == 32
+
+/*
+ * On 32 bits systems the i_ino of struct inode is 32 bits (unsigned long), so
+ * we use the inode's location objectid which is a u64 to avoid truncation.
+ */
 static inline u64 btrfs_ino(const struct btrfs_inode *inode)
 {
 	u64 ino = inode->location.objectid;
@@ -289,6 +295,15 @@ static inline u64 btrfs_ino(const struct btrfs_inode *inode)
 	return ino;
 }
 
+#else
+
+static inline u64 btrfs_ino(const struct btrfs_inode *inode)
+{
+	return inode->vfs_inode.i_ino;
+}
+
+#endif
+
 static inline void btrfs_i_size_write(struct btrfs_inode *inode, u64 size)
 {
 	i_size_write(&inode->vfs_inode, size);
diff --git a/fs/btrfs/tests/btrfs-tests.c b/fs/btrfs/tests/btrfs-tests.c
index 1591bfa55bcc..293e01228375 100644
--- a/fs/btrfs/tests/btrfs-tests.c
+++ b/fs/btrfs/tests/btrfs-tests.c
@@ -59,6 +59,7 @@ struct inode *btrfs_new_test_inode(void)
 		return NULL;
 
 	inode->i_mode = S_IFREG;
+	inode->i_ino = BTRFS_FIRST_FREE_OBJECTID;
 	BTRFS_I(inode)->location.type = BTRFS_INODE_ITEM_KEY;
 	BTRFS_I(inode)->location.objectid = BTRFS_FIRST_FREE_OBJECTID;
 	BTRFS_I(inode)->location.offset = 0;
-- 
2.35.1

Re: [PATCH 0/2] btrfs: optimize btrfs_ino()

[David Sterba]

On Mon, Jul 11, 2022 at 03:22:48PM +0100, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> The following patchset optimizes btrfs_ino(), an inline function that is
> used pretty much everywhere, to avoid doing less work and be trivial on
> 64 bits systems. Details on the changelogs of the patches.
> 
> Filipe Manana (2):
>   btrfs: set the objectid of the btree inode's location key
>   btrfs: add optimized btrfs_ino() version for 64 bits systems

Great, thanks. The module size on a release build is -7k.