Re: [-next] FATAL: drivers/gpu/drm/udl/udl: sizeof(struct usb_device_id)=24 is not a modulo of the size of section __mod_usb_device_table=44.

[Geert Uytterhoeven <geert@linux-m68k.org>]

Hi Greg,

On Sat, Jun 16, 2012 at 1:12 AM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Fri, Jun 15, 2012 at 11:02:55PM +0200, Geert Uytterhoeven wrote:
>> On Fri, Jun 15, 2012 at 10:10 PM, Greg Kroah-Hartman
>> <gregkh@linuxfoundation.org> wrote:
>> > On Fri, Jun 15, 2012 at 07:42:08PM +0200, Geert Uytterhoeven wrote:
>> >> commit 81df2d594340dcb6d1a02191976be88a1ca8120c ("USB: allow match
>> >> on bInterfaceNumber") added a byte to the interior of struct usb_device_id,
>> >> enabling implicit padding:
>> >>
>> >> --- a/include/linux/mod_devicetable.h
>> >> +++ b/include/linux/mod_devicetable.h
>> >> @@ -115,6 +118,9 @@ struct usb_device_id {
>> >>         __u8            bInterfaceSubClass;
>> >>         __u8            bInterfaceProtocol;
>> >>
>> >> +       /* Used for vendor-specific interface matches */
>> >> +       __u8            bInterfaceNumber;
>> >> +
>> >>         /* not matched against */
>> >>         kernel_ulong_t  driver_info;
>> >>  };
>> >>
>> >> On m68k, this causes failures like:
>> >>
>> >> | FATAL: drivers/gpu/drm/udl/udl: sizeof(struct usb_device_id)=24 is
>> >> not a modulo of the size of section __mod_usb_device_table=44.
>> >> | Fix definition of struct usb_device_id in mod_devicetable.h
>> >>
>> >> M68k is special in that it uses 2 for the alignment of 32-bit entities, hence
>> >> sizeof(struct usb_device_id) = 22.
>> >>
>> >> However, when cross-compiling on amd64, sizeof(struct usb_device_id) = 24
>> >> in scripts/mod/file2alias.c.
>> >
>> > I don't really understand, what is the problem here?  The fact that we
>> > added one byte to the structure?  And now something breaks?  How?
>> >
>> > What can we do to resolve it?
>>
>> As "kernel_ulong_t  driver_info" is no longer naturally aligned, the
>> compiler will
>> add implicit padding. But the padding depends on the architecture.
>
> Ah, so we were "lucky" before, nice.
>
>> It can be fixed by adding explicit padding. Probably it should be padded by
>> 7 bytes (not 3), as kernel_ulong_t may require 8-byte alignment on some 64-bit
>> platforms. Or by an explicit alignment attribute.
>>
>> See also
>>   * commit 8175fe2dda1c93a9c596921c8ed4a0b4baccdefe ("HID: fix
>> hid_device_id for cross compiling")
>>   * commit 7492d4a416d68ab4bd254b36ffcc4e0138daa8ff ("sdio: fix module
>> device table definition for m68k")
>>   * commit 9e2d3cd34a159948dc753a14573e16bffc04dba8 ("[PATCH]
>> mod_devicetable.h fixes")
>
> So would the patch below fix this?  It should force alignment of the
> driver_data field, which is all you want here, right?

Yes, it's fixes it, so

Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>

>> Still, there's a bug in file2alias (which is compiled by the host
>> compiler), in that
>> it may use different padding than the target platform when cross-compiling.
>
> That's not good, but outside of this specific issue, right?  Have we
> just been fortunate it hasn't really hit us yet?
>
> thanks,
>
> greg k-h
>
> diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
> index 7771d45..6955045 100644
> --- a/include/linux/mod_devicetable.h
> +++ b/include/linux/mod_devicetable.h
> @@ -122,7 +122,8 @@ struct usb_device_id {
>        __u8            bInterfaceNumber;
>
>        /* not matched against */
> -       kernel_ulong_t  driver_info;
> +       kernel_ulong_t  driver_info
> +               __attribute__((aligned(sizeof(kernel_ulong_t))));
>  };
>
>  /* Some useful macros to use to create struct usb_device_id */

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds