linux-kernel - Re: [PATCH 1/2] device.h: pack struct dev_links

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 27 Feb 2019 10:31:04 +0100
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Johan Hovold <johan@...nel.org>
Cc:     linux-kernel@...r.kernel.org,
        "Rafael J. Wysocki" <rafael.j.wysocki@...el.com>
Subject: Re: [PATCH 1/2] device.h: pack struct dev_links_info

On Wed, Feb 27, 2019 at 10:23:18AM +0100, Johan Hovold wrote:
> 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@...el.com>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> > ---
> >  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).

No, all this changes is the trailing "space" is gone.  The alignment of
the fields did not change at all as they are all naturally aligned
(list_head is just 2 pointers).

Here's the pahole output before and after this patch:

Before:
struct dev_links_info {
        struct list_head           suppliers;            /*     0    16 */
        struct list_head           consumers;            /*    16    16 */
        enum dl_dev_state          status;               /*    32     4 */

        /* size: 40, cachelines: 1, members: 3 */
        /* padding: 4 */
        /* last cacheline: 40 bytes */
};

After:
struct dev_links_info {
        struct list_head           suppliers;            /*     0    16 */
        struct list_head           consumers;            /*    16    16 */
        enum dl_dev_state          status;               /*    32     4 */

        /* size: 36, cachelines: 1, members: 3 */
        /* last cacheline: 36 bytes */
};


So this allows us to save 4 bytes in struct device by putting something in that
trailing "hole" that can be aligned with it better (i.e. an integer or
something else).

thanks,

greg k-h