[PATCH 1/2] xfs: reorder xfs_inode structure elements to remove unneeded padding.

[PATCH 1/2] xfs: reorder xfs_inode structure elements to remove unneeded padding.

[Junchao Sun]

By reordering the elements in the xfs_inode structure, we can
reduce the padding needed on an x86_64 system by 8 bytes.

Signed-off-by: Junchao Sun <sunjunchao2870@gmail.com>
---
 fs/xfs/xfs_inode.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
index 292b90b5f2ac..3239ae4e33d2 100644
--- a/fs/xfs/xfs_inode.h
+++ b/fs/xfs/xfs_inode.h
@@ -40,8 +40,8 @@ typedef struct xfs_inode {
 	/* Transaction and locking information. */
 	struct xfs_inode_log_item *i_itemp;	/* logging information */
 	struct rw_semaphore	i_lock;		/* inode lock */
-	atomic_t		i_pincount;	/* inode pin count */
 	struct llist_node	i_gclist;	/* deferred inactivation list */
+	atomic_t		i_pincount;	/* inode pin count */
 
 	/*
 	 * Bitsets of inode metadata that have been checked and/or are sick.
-- 
2.39.2

[PATCH 2/2] vfs: reorder struct file structure elements to remove unneeded padding.

[Junchao Sun]

By reordering the elements in the struct file structure, we can
reduce the padding needed on an x86_64 system by 8 bytes.

Signed-off-by: Junchao Sun <sunjunchao2870@gmail.com>
---
 include/linux/fs.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 0283cf366c2a..9235b7a960d3 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -999,10 +999,10 @@ struct file {
 	 */
 	spinlock_t		f_lock;
 	fmode_t			f_mode;
+	unsigned int		f_flags;
 	atomic_long_t		f_count;
 	struct mutex		f_pos_lock;
 	loff_t			f_pos;
-	unsigned int		f_flags;
 	struct fown_struct	f_owner;
 	const struct cred	*f_cred;
 	struct file_ra_state	f_ra;
-- 
2.39.2

Re: [PATCH 2/2] vfs: reorder struct file structure elements to remove unneeded padding.

[Christian Brauner]

On Tue, Jun 18, 2024 at 07:35:05PM GMT, Junchao Sun wrote:
> By reordering the elements in the struct file structure, we can
> reduce the padding needed on an x86_64 system by 8 bytes.
> 
> Signed-off-by: Junchao Sun <sunjunchao2870@gmail.com>
> ---
>  include/linux/fs.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 0283cf366c2a..9235b7a960d3 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -999,10 +999,10 @@ struct file {
>  	 */
>  	spinlock_t		f_lock;
>  	fmode_t			f_mode;
> +	unsigned int		f_flags;

Iiuc, then this will push f_pos_lock into a new cache line whereas it's
explicitly optimized so that f_lock, f_mode, and f_pos_lock are all in
the same cache line.

You could play around with moving the union to the end of struct file.

Re: [PATCH 1/2] xfs: reorder xfs_inode structure elements to remove unneeded padding.

[Darrick J. Wong]

On Tue, Jun 18, 2024 at 07:35:04PM +0800, Junchao Sun wrote:
> By reordering the elements in the xfs_inode structure, we can
> reduce the padding needed on an x86_64 system by 8 bytes.

Does this result in denser packing of xfs_inode objects in the slab
page?

--D

> Signed-off-by: Junchao Sun <sunjunchao2870@gmail.com>
> ---
>  fs/xfs/xfs_inode.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
> index 292b90b5f2ac..3239ae4e33d2 100644
> --- a/fs/xfs/xfs_inode.h
> +++ b/fs/xfs/xfs_inode.h
> @@ -40,8 +40,8 @@ typedef struct xfs_inode {
>  	/* Transaction and locking information. */
>  	struct xfs_inode_log_item *i_itemp;	/* logging information */
>  	struct rw_semaphore	i_lock;		/* inode lock */
> -	atomic_t		i_pincount;	/* inode pin count */
>  	struct llist_node	i_gclist;	/* deferred inactivation list */
> +	atomic_t		i_pincount;	/* inode pin count */
>  
>  	/*
>  	 * Bitsets of inode metadata that have been checked and/or are sick.
> -- 
> 2.39.2
> 
> 

Re: [PATCH 1/2] xfs: reorder xfs_inode structure elements to remove unneeded padding.

[JunChao Sun]

Darrick J. Wong <djwong@kernel.org> 于2024年6月18日周二 12:23写道：
>
> On Tue, Jun 18, 2024 at 07:35:04PM +0800, Junchao Sun wrote:
> > By reordering the elements in the xfs_inode structure, we can
> > reduce the padding needed on an x86_64 system by 8 bytes.
>
>
> > Does this result in denser packing of xfs_inode objects in the slab
> > page?

No. Before applying the patch, the size of xfs_inode is 1800 bytes
with my config, and after applying the patch, the size is 1792 bytes.
This slight reduction does not result in a denser packing of xfs_inode
objects within a single page.

>
> --D
>
> > Signed-off-by: Junchao Sun <sunjunchao2870@gmail.com>
> > ---
> >  fs/xfs/xfs_inode.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
> > index 292b90b5f2ac..3239ae4e33d2 100644
> > --- a/fs/xfs/xfs_inode.h
> > +++ b/fs/xfs/xfs_inode.h
> > @@ -40,8 +40,8 @@ typedef struct xfs_inode {
> >       /* Transaction and locking information. */
> >       struct xfs_inode_log_item *i_itemp;     /* logging information */
> >       struct rw_semaphore     i_lock;         /* inode lock */
> > -     atomic_t                i_pincount;     /* inode pin count */
> >       struct llist_node       i_gclist;       /* deferred inactivation list */
> > +     atomic_t                i_pincount;     /* inode pin count */
> >
> >       /*
> >        * Bitsets of inode metadata that have been checked and/or are sick.
> > --
> > 2.39.2
> >
> >



-- 
Junchao Sun <sunjunchao2870@gmail.com>

Re: [PATCH 1/2] xfs: reorder xfs_inode structure elements to remove unneeded padding.

[Matthew Wilcox]

On Tue, Jun 18, 2024 at 12:40:23PM -0400, JunChao Sun wrote:
> Darrick J. Wong <djwong@kernel.org> 于2024年6月18日周二 12:23写道：
> >
> > On Tue, Jun 18, 2024 at 07:35:04PM +0800, Junchao Sun wrote:
> > > By reordering the elements in the xfs_inode structure, we can
> > > reduce the padding needed on an x86_64 system by 8 bytes.
> >
> >
> > > Does this result in denser packing of xfs_inode objects in the slab
> > > page?
> 
> No. Before applying the patch, the size of xfs_inode is 1800 bytes
> with my config, and after applying the patch, the size is 1792 bytes.
> This slight reduction does not result in a denser packing of xfs_inode
> objects within a single page.

The "config dependent" part of this is important though.  On my
laptop running Debian 6.6.15-amd64, xfs_inode is exactly 1024 bytes,
and slab chooses to allocate 32 of them from an order-3 slab.

Your config gets you 18 from an order-3 slab, and you'd need to get
it down to 1724 (probably 1720 bytes due to alignment) to get 19
from an order-3 slab.  I bet you have lockdep or something on.

Re: [PATCH 1/2] xfs: reorder xfs_inode structure elements to remove unneeded padding.

[JunChao Sun]

Matthew Wilcox <willy@infradead.org> 于2024年6月19日周三 00:51写道：
>
> On Tue, Jun 18, 2024 at 12:40:23PM -0400, JunChao Sun wrote:
> > Darrick J. Wong <djwong@kernel.org> 于2024年6月18日周二 12:23写道：
> > >
> > > On Tue, Jun 18, 2024 at 07:35:04PM +0800, Junchao Sun wrote:
> > > > By reordering the elements in the xfs_inode structure, we can
> > > > reduce the padding needed on an x86_64 system by 8 bytes.
> > >
> > >
> > > > Does this result in denser packing of xfs_inode objects in the slab
> > > > page?
> >
> > No. Before applying the patch, the size of xfs_inode is 1800 bytes
> > with my config, and after applying the patch, the size is 1792 bytes.
> > This slight reduction does not result in a denser packing of xfs_inode
> > objects within a single page.
>
>
> > The "config dependent" part of this is important though.  On my
> > laptop running Debian 6.6.15-amd64, xfs_inode is exactly 1024 bytes,
> > and slab chooses to allocate 32 of them from an order-3 slab.
> >
> > Your config gets you 18 from an order-3 slab, and you'd need to get
> > it down to 1724 (probably 1720 bytes due to alignment) to get 19
> > from an order-3 slab.  I bet you have lockdep or something on.

Hi,

I couldn't find the exact 6.6.15-amd64 kernel, but I installed the
Debian 6.8.12-amd64 and 6.1.0-21-amd64 kernels, along with their
corresponding debug packages. In both cases, the size of xfs_inode is
1000 bytes. By eliminating the padding bytes, the number of xfs_inode
objects allocated from an order-3 slab increased from 32 to 33.

I'm not sure what specific differences there are between our Debian
kernels, but I have submitted the v2 version of the patch. If there
are any issues, please feel free to let me know. Thank you!


Best regards
-- 
Junchao Sun <sunjunchao2870@gmail.com>