svn commit: r322969 - in head: sbin/mdconfig sys/dev/md sys/sys

[Ryan Libby]

On Mon, Aug 28, 2017 at 11:24 AM, Maxim Sobolev <sobomax at freebsd.org> wrote:
> Hi John,
>
> Thanks for your feedback! To address the points that you've raised:
>
> 1. I've tested on both 32 and 64 bit platforms, it seems not to be the
> case. See imp's comment and my reply here
> https://reviews.freebsd.org/D10457#216855 . Did I miss something? Can you
> post piece of C code that produces different sizeof(struct old) vs.
> sizeof(struct new) on some platform?
[...]
> On Mon, Aug 28, 2017 at 9:19 AM, John Baldwin <jhb at freebsd.org> wrote:
>
>> On Monday, August 28, 2017 03:54:08 PM Maxim Sobolev wrote:
>> > Author: sobomax
>> > Date: Mon Aug 28 15:54:07 2017
>> > New Revision: 322969
>> > URL: https://svnweb.freebsd.org/changeset/base/322969
>> >
>> > Log:
>> >   Add ability to label md(4) devices.
>> >
>> >   This feature comes from the fact that we rely memory-backed md(4)
>> >   in our build process heavily. However, if the build goes haywire
>> >   the allocated resources (i.e. swap and memory-backed md(4)'s) need
>> >   to be purged. It is extremely useful to have ability to attach
>> >   arbitrary labels to each of the virtual disks so that they can
>> >   be identified and GC'ed if neecessary.
>> >
>> >   MFC after:  4 weeks
>> >   Differential Revision:      https://reviews.freebsd.org/D10457
>> >
>> > Modified:
>> >   head/sbin/mdconfig/mdconfig.8
>> >   head/sbin/mdconfig/mdconfig.c
>> >   head/sys/dev/md/md.c
>> >   head/sys/sys/mdioctl.h
>> >
>> > Modified: head/sys/sys/mdioctl.h
>> > ============================================================
>> ==================
>> > --- head/sys/sys/mdioctl.h    Mon Aug 28 14:49:26 2017        (r322968)
>> > +++ head/sys/sys/mdioctl.h    Mon Aug 28 15:54:07 2017        (r322969)
>> > @@ -49,7 +49,7 @@ enum md_types {MD_MALLOC, MD_PRELOAD, MD_VNODE, MD_SWA
>> >   * Ioctl definitions for memory disk pseudo-device.
>> >   */
>> >
>> > -#define MDNPAD               97
>> > +#define MDNPAD               96
>> >  struct md_ioctl {
>> >       unsigned        md_version;     /* Structure layout version */
>> >       unsigned        md_unit;        /* unit number */
>> > @@ -61,6 +61,7 @@ struct md_ioctl {
>> >       u_int64_t       md_base;        /* base address */
>> >       int             md_fwheads;     /* firmware heads */
>> >       int             md_fwsectors;   /* firmware sectors */
>> > +     char            *md_label;      /* label of the device */
>> >       int             md_pad[MDNPAD]; /* padding for future ideas */
>> >  };
>>
>> This isn't correct on 64-bit platforms.  MDNPAD needs to be 95 on those
>> platforms.
[...]

Can you report sizeof(md_ioctl) before and after for 32-bit and 64-bit?
I think it may be:
32-bit before: 440
32-bit after:  440
64-bit before: 448
64-bit after:  448

In other words, it looks like it used to produce different sizes on the
different architectures, and still does.  It also looks like 32-bit
before and after and 64-bit before included some undeclared padding
after md_pad, so that this would fail:
CTASSERT(sizeof(md_ioctl) == offsetof(struct md_ioctl, md_pad) +
    sizeof(((struct md_ioctl *)NULL)->md_pad));

In any case a CTASSERT(sizeof(md_ioctl) == XXX) may increase confidence
in the ABI here.