Re: [PATCH 6/9] btrfs: reduce compressed_bio members' types

[Anand Jain <anand.jain@oracle.com>]

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 */
>