diff mbox series

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

Message ID 20240618113505.476072-1-sunjunchao2870@gmail.com (mailing list archive)
State New
Headers show
Series [1/2] xfs: reorder xfs_inode structure elements to remove unneeded padding. | expand

Commit Message

Julian Sun June 18, 2024, 11:35 a.m. UTC
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(-)

Comments

Darrick J. Wong June 18, 2024, 4:23 p.m. UTC | #1
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
> 
>
Julian Sun June 18, 2024, 4:40 p.m. UTC | #2
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
> >
> >
Matthew Wilcox June 18, 2024, 4:51 p.m. UTC | #3
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.
Julian Sun June 19, 2024, 10:05 a.m. UTC | #4
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
diff mbox series

Patch

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.