Re: [PATCH 1/2] device.h: pack struct dev_links_info

[Johan Hovold]

On Tue, Feb 26, 2019 at 03:41:07PM +0100, Greg Kroah-Hartman wrote:
> The dev_links_info structure has 4 bytes of padding at the end of it
> when embedded in struct device (which is the only place it lives).  To
> help reduce the size of struct device pack this structure so we can take
> advantage of the hole with later structure reorganizations.
> 
> Cc: "Rafael J. Wysocki" <rafael.j.wysocki@xxxxxxxxx>
> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> ---
>  include/linux/device.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 6cb4640b6160..b63165276a09 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -884,7 +884,7 @@ struct dev_links_info {
>  	struct list_head suppliers;
>  	struct list_head consumers;
>  	enum dl_dev_state status;
> -};
> +} __packed;

This seems like a bad idea. You're changing the alignment of these
fields to one byte, something which may cause the compiler to generate
less efficient code to deal with unaligned accesses (even if they happen
to currently be naturally aligned in struct device).

I don't think we should mess with __packed unless for things that
actually require it (e.g. data going on to the wire) even if it means
wasting 4 bytes on 64-bit archs.

Johan