On Mon, Aug 28, 2017 at 11:24 AM, Maxim Sobolev <sobo...@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 <j...@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.
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
