Skip to contentSkip to conversation

btrfs: avoid duplications by moving the static int array from header to c file

PATCH In review Cheng Renquan 1 participant 2 messages Aug 13, 2010 – Aug 27, 2010 Btrfs file system

lore
Share & tools
lore

Cheng Renquan

author
#0 lore raw

PATCH 1/1 btrfs: avoid duplications by moving the static int array from header to c file

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 <[email protected]>
---
 fs/btrfs/ctree.c |    9 +++++++++
 fs/btrfs/ctree.h |    9 +--------
 2 files changed, 10 insertions(+), 8 deletions(-)
fs/btrfs/ctree.c +9 -0
index c3df14c..3a89207 100644
@@ -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
fs/btrfs/ctree.h +1 -8
index e9bf864..99220ee 100644
@@ -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)
 {
Signature · 1 line
1.7.0.4
author
#1 lore raw

On Fri, Aug 13, 2010 at 9:55 AM, Cheng Renquan <[email protected]> wrote= :

Quoted 2 lines

The commit 607d432d referred a static int array defined in ctree.h, and a static inline function (btrfs_super_csum_size) using this array=

,

Quoted 4 lines

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:

Quoted 8 lines
$ 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 c=

opies

Quoted 6 lines

of btrfs_csum_sizes; but future code may call it in more c files, resulting in more copies;

=C2=A0fs/btrfs/ctree.h =C2=A0 =C2=A0 | =C2=A0 19 ++++++++++++++++- =C2=A0fs/btrfs/disk-io.c =C2=A0 | =C2=A0 25 +++++++++++++++++---- =C2=A0fs/btrfs/file-item.c | =C2=A0 56 ++++++++++++++++++++++++++++--=

-------------------

Quoted 5 lines

=C2=A0fs/btrfs/ioctl.c =C2=A0 =C2=A0 | =C2=A0 =C2=A09 ++++--- =C2=A0fs/btrfs/tree-log.c =C2=A0| =C2=A0 10 +++++--- =C2=A05 files changed, 81 insertions(+), 38 deletions(-)

multiple copies are just wasting memory; move it to a c file can avoi=

d

Quoted 1 line

duplications; and since the inline function referred ARRAY_SIZE of th=

at

Quoted 4 lines

array, must know the array size at compile time, so cannot be inlined anyway.

The cost is originally inlined function calling changed to external f=

unction

Quoted 3 lines

calling.

Signed-off-by Cheng Renquan <[email protected]>

Ping,

It was two weeks passed, and someone has some comments? Thanks,

Quoted 13 lines

---
=C2=A0fs/btrfs/ctree.c | =C2=A0 =C2=A09 +++++++++ =C2=A0fs/btrfs/ctree.h | =C2=A0 =C2=A09 +-------- =C2=A02 files changed, 10 insertions(+), 8 deletions(-)

fs/btrfs/ctree.c +0 -0
index c3df14c..3a89207 100644
@@ -24,6 +24,15 @@

=C2=A0#include "print-tree.h"
=C2=A0#include "locking.h"

fs/btrfs/ctree.c

Quoted 3 lines
fs/btrfs/ctree.c +3 -0
+int btrfs_super_csum_size(struct btrfs_super_block *s)
+{
+ =C2=A0 =C2=A0 =C2=A0 static const int btrfs_csum_sizes[] =3D { 4, 0=

};

Quoted 7 lines
fs/btrfs/ctree.c +6 -0
+
+ =C2=A0 =C2=A0 =C2=A0 int t =3D btrfs_super_csum_type(s);
+ =C2=A0 =C2=A0 =C2=A0 BUG_ON(t >=3D ARRAY_SIZE(btrfs_csum_sizes));
+ =C2=A0 =C2=A0 =C2=A0 return btrfs_csum_sizes[t];
+}
+

=C2=A0static int split_node(struct btrfs_trans_handle *trans, struct =

btrfs_root

Quoted 1 line

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=

=C2=A0*root, struct btrfs_path *path, int level);

Quoted 1 line

=C2=A0static int split_leaf(struct btrfs_trans_handle *trans, struct =

btrfs_root

fs/btrfs/ctree.h BTRFS_SETGET_STACK_FUNCS(super_incompat_flags,=

Quoted 14 lines
fs/btrfs/ctree.h +0 -0
index e9bf864..99220ee 100644
@@ -132,8 +132,6 @@ struct btrfs_ordered_sum;

=C2=A0/* csum types */
=C2=A0#define BTRFS_CSUM_TYPE_CRC32 =C2=A00

fs/btrfs/ctree.h +0 -2
-static int btrfs_csum_sizes[] =3D { 4, 0 };
-

=C2=A0/* four bytes for CRC32 */
=C2=A0#define BTRFS_EMPTY_DIR_SIZE 0

fs/btrfs/ctree.h +0 -0
@@ -1877,12 +1875,7 @@ BTRFS_SETGET_STACK_FUNCS(super_incompat_flags,=

struct btrfs_super_block,

Quoted 1 line

=C2=A0BTRFS_SETGET_STACK_FUNCS(super_csum_type, struct btrfs_super_bl=

ock,

Quoted 1 line

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=

=C2=A0 =C2=A0 csum_type, 16);

Quoted 10 lines
fs/btrfs/ctree.h +1 -6
-static inline int btrfs_super_csum_size(struct btrfs_super_block *s)
-{
- =C2=A0 =C2=A0 =C2=A0 int t =3D btrfs_super_csum_type(s);
- =C2=A0 =C2=A0 =C2=A0 BUG_ON(t >=3D ARRAY_SIZE(btrfs_csum_sizes));
- =C2=A0 =C2=A0 =C2=A0 return btrfs_csum_sizes[t];
-}
+int btrfs_super_csum_size(struct btrfs_super_block *s);

=C2=A0static inline unsigned long btrfs_leaf_data(struct extent_buffe=

r *l)

Quoted 2 lines

=C2=A0{
--

--
Cheng Renquan (=E7=A8=8B=E4=BB=BB=E5=85=A8), from Singapore --
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" = in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html