Re: [PATCH] bcachefs: omit alignment attribute on big endian struct bkey

[Brian Foster <bfoster@redhat.com>]

On Thu, Feb 15, 2024 at 09:19:57AM -0700, Thomas Bertschinger wrote:
> This is needed for building Rust bindings on big endian architectures
> like s390x. Currently this is only done in userspace, but it might
> happen in-kernel in the future. When creating a Rust binding for struct
> bkey, the "packed" attribute is needed to get a type with the correct
> member offsets. However, rustc does not allow types to have both a
> "packed" and "align" attribute. Thus, in order to get a Rust type
> compatible with the C type, on big endian systems we must omit the
> "aligned" attribute in C.
> 
> This does not affect the struct's size or member offsets, only its
> toplevel alignment, which should be an acceptable impact.
> 
> Signed-off-by: Thomas Bertschinger <tahbertschinger@gmail.com>
> ---
>  fs/bcachefs/bcachefs_format.h | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/bcachefs/bcachefs_format.h b/fs/bcachefs/bcachefs_format.h
> index 1bb24aa73528..fc12efe8f8cd 100644
> --- a/fs/bcachefs/bcachefs_format.h
> +++ b/fs/bcachefs/bcachefs_format.h
> @@ -222,7 +222,11 @@ struct bkey {
>  
>  	__u8		pad[1];
>  #endif
> -} __packed __aligned(8);
> +} __packed

I can't really comment on the Rust context, but could you add a comment
to explain why the ifdef is here? I.e., even something as simple as
"Rust disallows packed and aligned on ..." is useful.

Also out of curiosity, is there some reason these two attributes
together presumably isn't a problem for LE?

Brian

> +#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
> +__aligned(8)
> +#endif
> +;
>  
>  struct bkey_packed {
>  	__u64		_data[0];
> -- 
> 2.43.0
>