Date: Mon, 28 Aug 2017 12:46:48 -0700 From: Ryan Libby <rlibby@freebsd.org> To: Maxim Sobolev <sobomax@freebsd.org> Cc: John Baldwin <jhb@freebsd.org>, svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers <src-committers@freebsd.org> Subject: Re: svn commit: r322969 - in head: sbin/mdconfig sys/dev/md sys/sys Message-ID: <CAHgpiFzTuNvqrDpU-dmtAauHAFHXroHyxNRdignozUy_D=3fKQ@mail.gmail.com> In-Reply-To: <CAH7qZfv=RT%2BZ7uZ8EY_Dm07-gAS3fhcchBv8Wj_YMo6nmLF0qQ@mail.gmail.com> References: <201708281554.v7SFs8fr014268@repo.freebsd.org> <7384187.efIiCynxO3@ralph.baldwin.cx> <CAH7qZfv=RT%2BZ7uZ8EY_Dm07-gAS3fhcchBv8Wj_YMo6nmLF0qQ@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Aug 28, 2017 at 11:24 AM, Maxim Sobolev <sobomax@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@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.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAHgpiFzTuNvqrDpU-dmtAauHAFHXroHyxNRdignozUy_D=3fKQ>