| 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 |
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 > >
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 > > > >
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.
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 --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.
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(-)