2024-06-03 13:36:07

by JunChao Sun

[permalink] [raw]
Subject: [PATCH] ext4: Adjust the layout of the ext4_inode_info structure to save memory.

Using pahole, we can see that there are some padding holes
in the current ext4_inode_info structure. Adjusting the
layout of ext4_inode_info can reduce these holes,
resulting in the size of the structure decreasing
from 2424 bytes to 2408 bytes.

Signed-off-by: Junchao Sun <[email protected]>
---
fs/ext4/ext4.h | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 023571f8dd1b..42bcd4f749a8 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1055,6 +1055,7 @@ struct ext4_inode_info {

/* Number of ongoing updates on this inode */
atomic_t i_fc_updates;
+ atomic_t i_unwritten; /* Nr. of inflight conversions pending */

/* Fast commit wait queue for this inode */
wait_queue_head_t i_fc_wait;
@@ -1103,6 +1104,10 @@ struct ext4_inode_info {

/* mballoc */
atomic_t i_prealloc_active;
+
+ /* allocation reservation info for delalloc */
+ /* In case of bigalloc, this refer to clusters rather than blocks */
+ unsigned int i_reserved_data_blocks;
struct rb_root i_prealloc_node;
rwlock_t i_prealloc_lock;

@@ -1119,10 +1124,6 @@ struct ext4_inode_info {
/* ialloc */
ext4_group_t i_last_alloc_group;

- /* allocation reservation info for delalloc */
- /* In case of bigalloc, this refer to clusters rather than blocks */
- unsigned int i_reserved_data_blocks;
-
/* pending cluster reservations for bigalloc file systems */
struct ext4_pending_tree i_pending_tree;

@@ -1146,7 +1147,6 @@ struct ext4_inode_info {
*/
struct list_head i_rsv_conversion_list;
struct work_struct i_rsv_conversion_work;
- atomic_t i_unwritten; /* Nr. of inflight conversions pending */

spinlock_t i_block_reservation_lock;

--
2.39.2



2024-06-03 14:22:14

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCH] ext4: Adjust the layout of the ext4_inode_info structure to save memory.

On Mon 03-06-24 21:15:24, Junchao Sun wrote:
> Using pahole, we can see that there are some padding holes
> in the current ext4_inode_info structure. Adjusting the
> layout of ext4_inode_info can reduce these holes,
> resulting in the size of the structure decreasing
> from 2424 bytes to 2408 bytes.

But AFAICT this will save two holes 4 bytes each so only 8 bytes in total?
Not 16?

>
> Signed-off-by: Junchao Sun <[email protected]>

Otherwise looks good. Feel free to add:

Reviewed-by: Jan Kara <[email protected]>

Honza

> ---
> fs/ext4/ext4.h | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 023571f8dd1b..42bcd4f749a8 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -1055,6 +1055,7 @@ struct ext4_inode_info {
>
> /* Number of ongoing updates on this inode */
> atomic_t i_fc_updates;
> + atomic_t i_unwritten; /* Nr. of inflight conversions pending */
>
> /* Fast commit wait queue for this inode */
> wait_queue_head_t i_fc_wait;
> @@ -1103,6 +1104,10 @@ struct ext4_inode_info {
>
> /* mballoc */
> atomic_t i_prealloc_active;
> +
> + /* allocation reservation info for delalloc */
> + /* In case of bigalloc, this refer to clusters rather than blocks */
> + unsigned int i_reserved_data_blocks;
> struct rb_root i_prealloc_node;
> rwlock_t i_prealloc_lock;
>
> @@ -1119,10 +1124,6 @@ struct ext4_inode_info {
> /* ialloc */
> ext4_group_t i_last_alloc_group;
>
> - /* allocation reservation info for delalloc */
> - /* In case of bigalloc, this refer to clusters rather than blocks */
> - unsigned int i_reserved_data_blocks;
> -
> /* pending cluster reservations for bigalloc file systems */
> struct ext4_pending_tree i_pending_tree;
>
> @@ -1146,7 +1147,6 @@ struct ext4_inode_info {
> */
> struct list_head i_rsv_conversion_list;
> struct work_struct i_rsv_conversion_work;
> - atomic_t i_unwritten; /* Nr. of inflight conversions pending */
>
> spinlock_t i_block_reservation_lock;
>
> --
> 2.39.2
>
--
Jan Kara <[email protected]>
SUSE Labs, CR

2024-06-03 14:52:50

by JunChao Sun

[permalink] [raw]
Subject: Re: [PATCH] ext4: Adjust the layout of the ext4_inode_info structure to save memory.

Jan Kara <[email protected]> 于2024年6月3日周一 22:07写道:
>
> On Mon 03-06-24 21:15:24, Junchao Sun wrote:
> > Using pahole, we can see that there are some padding holes
> > in the current ext4_inode_info structure. Adjusting the
> > layout of ext4_inode_info can reduce these holes,
> > resulting in the size of the structure decreasing
> > from 2424 bytes to 2408 bytes.
>
>
> > But AFAICT this will save two holes 4 bytes each so only 8 bytes in total?
> > Not 16?

Indeed it's 16.
Consider the layout int a; hole 0; int b; hole 1; And then move int b
to hole 0 position, this adjustment resulted in saving 8 bytes. There
are two adjustments like this, so it's 16. And GDB confirmed this.

>
> >
> > Signed-off-by: Junchao Sun <[email protected]>
>
> Otherwise looks good. Feel free to add:
>
> Reviewed-by: Jan Kara <[email protected]>
>
> Honza
>
> > ---
> > fs/ext4/ext4.h | 10 +++++-----
> > 1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> > index 023571f8dd1b..42bcd4f749a8 100644
> > --- a/fs/ext4/ext4.h
> > +++ b/fs/ext4/ext4.h
> > @@ -1055,6 +1055,7 @@ struct ext4_inode_info {
> >
> > /* Number of ongoing updates on this inode */
> > atomic_t i_fc_updates;
> > + atomic_t i_unwritten; /* Nr. of inflight conversions pending */
> >
> > /* Fast commit wait queue for this inode */
> > wait_queue_head_t i_fc_wait;
> > @@ -1103,6 +1104,10 @@ struct ext4_inode_info {
> >
> > /* mballoc */
> > atomic_t i_prealloc_active;
> > +
> > + /* allocation reservation info for delalloc */
> > + /* In case of bigalloc, this refer to clusters rather than blocks */
> > + unsigned int i_reserved_data_blocks;
> > struct rb_root i_prealloc_node;
> > rwlock_t i_prealloc_lock;
> >
> > @@ -1119,10 +1124,6 @@ struct ext4_inode_info {
> > /* ialloc */
> > ext4_group_t i_last_alloc_group;
> >
> > - /* allocation reservation info for delalloc */
> > - /* In case of bigalloc, this refer to clusters rather than blocks */
> > - unsigned int i_reserved_data_blocks;
> > -
> > /* pending cluster reservations for bigalloc file systems */
> > struct ext4_pending_tree i_pending_tree;
> >
> > @@ -1146,7 +1147,6 @@ struct ext4_inode_info {
> > */
> > struct list_head i_rsv_conversion_list;
> > struct work_struct i_rsv_conversion_work;
> > - atomic_t i_unwritten; /* Nr. of inflight conversions pending */
> >
> > spinlock_t i_block_reservation_lock;
> >
> > --
> > 2.39.2
> >
> --
> Jan Kara <[email protected]>
> SUSE Labs, CR

2024-06-03 15:24:30

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCH] ext4: Adjust the layout of the ext4_inode_info structure to save memory.

On Mon 03-06-24 22:43:16, JunChao Sun wrote:
> Jan Kara <[email protected]> 于2024年6月3日周一 22:07写道:
> >
> > On Mon 03-06-24 21:15:24, Junchao Sun wrote:
> > > Using pahole, we can see that there are some padding holes
> > > in the current ext4_inode_info structure. Adjusting the
> > > layout of ext4_inode_info can reduce these holes,
> > > resulting in the size of the structure decreasing
> > > from 2424 bytes to 2408 bytes.
> >
> >
> > > But AFAICT this will save two holes 4 bytes each so only 8 bytes in total?
> > > Not 16?
>
> Indeed it's 16.
> Consider the layout int a; hole 0; int b; hole 1; And then move int b
> to hole 0 position, this adjustment resulted in saving 8 bytes. There
> are two adjustments like this, so it's 16. And GDB confirmed this.

Aha, OK. Thanks for clarification!

Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR