On 13/07/2020 11:53, David Sterba wrote:
> On Mon, Jul 13, 2020 at 11:42:51AM +0200, David Sterba wrote:
>> On Fri, Jul 10, 2020 at 11:05:09PM +0900, Johannes Thumshirn wrote:
>>> @@ -261,7 +264,8 @@ struct btrfs_ioctl_fs_info_args {
>>> __u32 flags; /* in/out */
>>> __u16 csum_type; /* out */
>>> __u16 csum_size; /* out */
>>> - __u8 reserved[972]; /* pad to 1k */
>>> + __u32 generation; /* out */
>>> + __u8 reserved[968]; /* pad to 1k */
>>
>> I've tested the static assert by switching just the type but not the
>> remaining reserved bytes
>>
>> ./include/linux/build_bug.h:78:41: error: static assertion failed: "sizeof(struct btrfs_ioctl_fs_info_args) == 1024"
>> 78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
>> | ^~~~~~~~~~~~~~
>> ./include/linux/build_bug.h:77:34: note: in expansion of macro ‘__static_assert’
>> 77 | #define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr)
>> | ^~~~~~~~~~~~~~~
>> ./include/uapi/linux/btrfs.h:270:1: note: in expansion of macro ‘static_assert’
>> 270 | static_assert(sizeof(struct btrfs_ioctl_fs_info_args) == 1024);
>> | ^~~~~~~~~~~~~
>> make[2]: *** [scripts/Makefile.build:281: fs/btrfs/super.o] Error 1
>> make[1]: *** [scripts/Makefile.build:497: fs/btrfs] Error 2
>> make: *** [Makefile:1756: fs] Error 2
>>
>> Good.
>
> There's extra padding now required for u64:
>
> struct btrfs_ioctl_fs_info_args {
> __u64 max_id; /* 0 8 */
> __u64 num_devices; /* 8 8 */
> __u8 fsid[16]; /* 16 16 */
> __u32 nodesize; /* 32 4 */
> __u32 sectorsize; /* 36 4 */
> __u32 clone_alignment; /* 40 4 */
> __u32 flags; /* 44 4 */
> __u16 csum_type; /* 48 2 */
> __u16 csum_size; /* 50 2 */
>
> /* XXX 4 bytes hole, try to pack */
>
> __u64 generation; /* 56 8 */
> /* --- cacheline 1 boundary (64 bytes) --- */
> __u8 reserved[964]; /* 64 964 */
>
> /* size: 1032, cachelines: 17, members: 11 */
> /* sum members: 1024, holes: 1, sum holes: 4 */
> /* padding: 4 */
> /* last cacheline: 8 bytes */
> };
>
> What if, instead of inserting a padding/reserved field we switch the
> flags to u64 too. This unfortunatelly requires swapping order for flags
> and csum_type/_size but the result
>
> struct btrfs_ioctl_fs_info_args {
> __u64 max_id; /* 0 8 */
> __u64 num_devices; /* 8 8 */
> __u8 fsid[16]; /* 16 16 */
> __u32 nodesize; /* 32 4 */
> __u32 sectorsize; /* 36 4 */
> __u32 clone_alignment; /* 40 4 */
> __u16 csum_type; /* 44 2 */
> __u16 csum_size; /* 46 2 */
> __u64 flags; /* 48 8 */
> __u64 generation; /* 56 8 */
> /* --- cacheline 1 boundary (64 bytes) --- */
> __u8 reserved[960]; /* 64 960 */
>
> /* size: 1024, cachelines: 16, members: 11 */
> };
>
> does not require any padding and leaves the end member with 8 byte
> alignment.
>
The swapped order looks a bit odd, but I don't really see a way around it.
Can you fix that up or should I re-send all 4 patches?
