diff mbox series

kernfs: reorder 'struct kernfs_node' to save some memory

Message ID 465890c56c6f5ad702a091a1ecd3c70bd4a3a74c.1701010150.git.christophe.jaillet@wanadoo.fr
State New
Headers
Series kernfs: reorder 'struct kernfs_node' to save some memory |

Commit Message

Christophe JAILLET Nov. 26, 2023, 2:49 p.m. UTC 
  'struct kernfs_node' uses a dedicated cache, so shrinking its size is
always a good idea.

On my system, each entry is 128 bytes and their are 32 entries per pages.
After the re-ordering, the struct is 120 bytes and 34 entries are stored
in each page.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
The numbers below are with a allmodconfig configuration. The delta is
related to CONFIG_DEBUG_LOCK_ALLOC and struct lockdep_map	dep_map.

When I checked on my system, it would have saved 372kb of RAM:
  sudo less /proc/slabinfo | grep kernf
  kernfs_node_cache  49397  49504    128   32    1 : tunables    0    0    0 : slabdata   1547   1547      0

I have left flags close to the union, I *think* that they are related.
I don't if having 'mode' here is logical or not.

Before:
======
struct kernfs_node {
        atomic_t                   count;                /*     0     4 */
        atomic_t                   active;               /*     4     4 */
        struct lockdep_map         dep_map;              /*     8    48 */
        struct kernfs_node *       parent;               /*    56     8 */
        /* --- cacheline 1 boundary (64 bytes) --- */
        const char  *              name;                 /*    64     8 */
        struct rb_node             rb __attribute__((__aligned__(8))); /*    72    24 */
        const void  *              ns;                   /*    96     8 */
        unsigned int               hash;                 /*   104     4 */

        /* XXX 4 bytes hole, try to pack */

        union {
                struct kernfs_elem_dir dir;              /*   112    32 */
                struct kernfs_elem_symlink symlink;      /*   112     8 */
                struct kernfs_elem_attr attr;            /*   112    32 */
        };                                               /*   112    32 */
        /* --- cacheline 2 boundary (128 bytes) was 16 bytes ago --- */
        void *                     priv;                 /*   144     8 */
        u64                        id;                   /*   152     8 */
        short unsigned int         flags;                /*   160     2 */
        umode_t                    mode;                 /*   162     2 */

        /* XXX 4 bytes hole, try to pack */

        struct kernfs_iattrs *     iattr;                /*   168     8 */

        /* size: 176, cachelines: 3, members: 14 */
        /* sum members: 168, holes: 2, sum holes: 8 */
        /* forced alignments: 1 */
        /* last cacheline: 48 bytes */
} __attribute__((__aligned__(8)));

After:
=====
struct kernfs_node {
        atomic_t                   count;                /*     0     4 */
        atomic_t                   active;               /*     4     4 */
        struct lockdep_map         dep_map;              /*     8    48 */
        struct kernfs_node *       parent;               /*    56     8 */
        /* --- cacheline 1 boundary (64 bytes) --- */
        const char  *              name;                 /*    64     8 */
        struct rb_node             rb __attribute__((__aligned__(8))); /*    72    24 */
        const void  *              ns;                   /*    96     8 */
        unsigned int               hash;                 /*   104     4 */
        umode_t                    mode;                 /*   108     2 */
        short unsigned int         flags;                /*   110     2 */
        union {
                struct kernfs_elem_dir dir;              /*   112    32 */
                struct kernfs_elem_symlink symlink;      /*   112     8 */
                struct kernfs_elem_attr attr;            /*   112    32 */
        };                                               /*   112    32 */
        /* --- cacheline 2 boundary (128 bytes) was 16 bytes ago --- */
        void *                     priv;                 /*   144     8 */
        u64                        id;                   /*   152     8 */
        struct kernfs_iattrs *     iattr;                /*   160     8 */

        /* size: 168, cachelines: 3, members: 14 */
        /* forced alignments: 1 */
        /* last cacheline: 40 bytes */
} __attribute__((__aligned__(8)));
---
 include/linux/kernfs.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Greg KH Nov. 26, 2023, 3:17 p.m. UTC | #1 
On Sun, Nov 26, 2023 at 03:49:37PM +0100, Christophe JAILLET wrote:
> 'struct kernfs_node' uses a dedicated cache, so shrinking its size is
> always a good idea.
> 
> On my system, each entry is 128 bytes and their are 32 entries per pages.
> After the re-ordering, the struct is 120 bytes and 34 entries are stored
> in each page.
> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> The numbers below are with a allmodconfig configuration. The delta is
> related to CONFIG_DEBUG_LOCK_ALLOC and struct lockdep_map	dep_map.
> 
> When I checked on my system, it would have saved 372kb of RAM:
>   sudo less /proc/slabinfo | grep kernf
>   kernfs_node_cache  49397  49504    128   32    1 : tunables    0    0    0 : slabdata   1547   1547      0
> 
> I have left flags close to the union, I *think* that they are related.
> I don't if having 'mode' here is logical or not.

I'm all for fixing up holes, but have you checked this before and after
with lockdep disabled?  That's usually the biggest chunk, and can cause
alignment issues, perhaps moving that to the end would make more sense?
With that, I think everything can fit in 2 cachelines except for the
lockdep structure?

thanks,

greg k-h
diff mbox series

Patch

diff --git a/include/linux/kernfs.h b/include/linux/kernfs.h
index 99aaa050ccb7..b7cd6e793286 100644
--- a/include/linux/kernfs.h
+++ b/include/linux/kernfs.h
@@ -206,6 +206,8 @@  struct kernfs_node {
 
 	const void		*ns;	/* namespace tag */
 	unsigned int		hash;	/* ns + name hash */
+	umode_t			mode;
+	unsigned short		flags;
 	union {
 		struct kernfs_elem_dir		dir;
 		struct kernfs_elem_symlink	symlink;
@@ -220,8 +222,6 @@  struct kernfs_node {
 	 */
 	u64			id;
 
-	unsigned short		flags;
-	umode_t			mode;
 	struct kernfs_iattrs	*iattr;
 };