| Message ID | 1a3c6cddd909d922948a22ac1f287293e0deb665.1621961965.git.dsterba@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Misc fixups and cleanups | expand |
On 26/05/2021 01:08, David Sterba wrote: > Several members of compressed_bio are of type that's unnecessarily big > for the values that they'd hold: > > - the size of the uncompressed and compressed data is 128K now, we can > keep is as int > - same for number of pages > - the compress type fits to a byte > - the errors is 0/1 > > The size of the unpatched structure is 80 bytes with several holes. > Reordering nr_pages next to the pages the hole after pending_bios is > filled and the resulting size is 56 bytes. This keeps the csums array > aligned to 8 bytes, which is nice. Further size optimizations may be > possible but right now it looks good to me: > > struct compressed_bio { > refcount_t pending_bios; /* 0 4 */ > unsigned int nr_pages; /* 4 4 */ > struct page * * compressed_pages; /* 8 8 */ > struct inode * inode; /* 16 8 */ > u64 start; /* 24 8 */ > unsigned int len; /* 32 4 */ > unsigned int compressed_len; /* 36 4 */ > u8 compress_type; /* 40 1 */ > u8 errors; /* 41 1 */ > > /* XXX 2 bytes hole, try to pack */ > > int mirror_num; /* 44 4 */ > struct bio * orig_bio; /* 48 8 */ > u8 sums[]; /* 56 0 */ > > /* size: 56, cachelines: 1, members: 12 */ > /* sum members: 54, holes: 1, sum holes: 2 */ > /* last cacheline: 56 bytes */ > }; > The following member types of struct compressed_bio are changed here: - unsigned long len; + unsigned int len; - int compress_type; + u8 compress_type; - unsigned long nr_pages; + unsigned int nr_pages; - unsigned long compressed_len; + unsigned int compressed_len; - int errors; + u8 errors; There are no warnings. But in btrfs_submit_compressed_write() Essentially struct compressed_bio is updated from struct async_extent. struct async_extent { u64 start; u64 ram_size; u64 compressed_size; struct page **pages; unsigned long nr_pages; int compress_type; struct list_head list; }; which can be looked into later. For now, with the patch btrfs: optimize users of members of the struct compressed_bio which is sent to ML. Reviewed-by: Anand Jain <anand.jain@oracle.com> > Signed-off-by: David Sterba <dsterba@suse.com> > --- > fs/btrfs/compression.c | 2 +- > fs/btrfs/compression.h | 20 ++++++++++---------- > 2 files changed, 11 insertions(+), 11 deletions(-) > > diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c > index 9a0c26e4e389..c006f5d81c2a 100644 > --- a/fs/btrfs/compression.c > +++ b/fs/btrfs/compression.c > @@ -507,7 +507,7 @@ blk_status_t btrfs_submit_compressed_write(struct btrfs_inode *inode, u64 start, > } > if (bytes_left < PAGE_SIZE) { > btrfs_info(fs_info, > - "bytes left %lu compress len %lu nr %lu", > + "bytes left %lu compress len %u nr %u", > bytes_left, cb->compressed_len, cb->nr_pages); > } > bytes_left -= PAGE_SIZE; > diff --git a/fs/btrfs/compression.h b/fs/btrfs/compression.h > index 8001b700ea3a..00d8439048c9 100644 > --- a/fs/btrfs/compression.h > +++ b/fs/btrfs/compression.h > @@ -31,6 +31,9 @@ struct compressed_bio { > /* number of bios pending for this compressed extent */ > refcount_t pending_bios; > > + /* Number of compressed pages in the array */ > + unsigned int nr_pages; > + > /* the pages with the compressed data on them */ > struct page **compressed_pages; > > @@ -40,20 +43,17 @@ struct compressed_bio { > /* starting offset in the inode for our pages */ > u64 start; > > - /* number of bytes in the inode we're working on */ > - unsigned long len; > - > - /* number of bytes on disk */ > - unsigned long compressed_len; > + /* Number of bytes in the inode we're working on */ > + unsigned int len; > > - /* the compression algorithm for this bio */ > - int compress_type; > + /* Number of bytes on disk */ > + unsigned int compressed_len; > > - /* number of compressed pages in the array */ > - unsigned long nr_pages; > + /* The compression algorithm for this bio */ > + u8 compress_type; > > /* IO errors */ > - int errors; > + u8 errors; > int mirror_num; > > /* for reads, this is the bio we are copying the data into */ >
diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c index 9a0c26e4e389..c006f5d81c2a 100644 --- a/fs/btrfs/compression.c +++ b/fs/btrfs/compression.c @@ -507,7 +507,7 @@ blk_status_t btrfs_submit_compressed_write(struct btrfs_inode *inode, u64 start, } if (bytes_left < PAGE_SIZE) { btrfs_info(fs_info, - "bytes left %lu compress len %lu nr %lu", + "bytes left %lu compress len %u nr %u", bytes_left, cb->compressed_len, cb->nr_pages); } bytes_left -= PAGE_SIZE; diff --git a/fs/btrfs/compression.h b/fs/btrfs/compression.h index 8001b700ea3a..00d8439048c9 100644 --- a/fs/btrfs/compression.h +++ b/fs/btrfs/compression.h @@ -31,6 +31,9 @@ struct compressed_bio { /* number of bios pending for this compressed extent */ refcount_t pending_bios; + /* Number of compressed pages in the array */ + unsigned int nr_pages; + /* the pages with the compressed data on them */ struct page **compressed_pages; @@ -40,20 +43,17 @@ struct compressed_bio { /* starting offset in the inode for our pages */ u64 start; - /* number of bytes in the inode we're working on */ - unsigned long len; - - /* number of bytes on disk */ - unsigned long compressed_len; + /* Number of bytes in the inode we're working on */ + unsigned int len; - /* the compression algorithm for this bio */ - int compress_type; + /* Number of bytes on disk */ + unsigned int compressed_len; - /* number of compressed pages in the array */ - unsigned long nr_pages; + /* The compression algorithm for this bio */ + u8 compress_type; /* IO errors */ - int errors; + u8 errors; int mirror_num; /* for reads, this is the bio we are copying the data into */
Several members of compressed_bio are of type that's unnecessarily big for the values that they'd hold: - the size of the uncompressed and compressed data is 128K now, we can keep is as int - same for number of pages - the compress type fits to a byte - the errors is 0/1 The size of the unpatched structure is 80 bytes with several holes. Reordering nr_pages next to the pages the hole after pending_bios is filled and the resulting size is 56 bytes. This keeps the csums array aligned to 8 bytes, which is nice. Further size optimizations may be possible but right now it looks good to me: struct compressed_bio { refcount_t pending_bios; /* 0 4 */ unsigned int nr_pages; /* 4 4 */ struct page * * compressed_pages; /* 8 8 */ struct inode * inode; /* 16 8 */ u64 start; /* 24 8 */ unsigned int len; /* 32 4 */ unsigned int compressed_len; /* 36 4 */ u8 compress_type; /* 40 1 */ u8 errors; /* 41 1 */ /* XXX 2 bytes hole, try to pack */ int mirror_num; /* 44 4 */ struct bio * orig_bio; /* 48 8 */ u8 sums[]; /* 56 0 */ /* size: 56, cachelines: 1, members: 12 */ /* sum members: 54, holes: 1, sum holes: 2 */ /* last cacheline: 56 bytes */ }; Signed-off-by: David Sterba <dsterba@suse.com> --- fs/btrfs/compression.c | 2 +- fs/btrfs/compression.h | 20 ++++++++++---------- 2 files changed, 11 insertions(+), 11 deletions(-)