From owner-svn-src-head@freebsd.org Mon Aug 28 23:40:51 2017 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 77E1FE16319 for ; Mon, 28 Aug 2017 23:40:51 +0000 (UTC) (envelope-from sobomax@sippysoft.com) Received: from mail-vk0-x22c.google.com (mail-vk0-x22c.google.com [IPv6:2607:f8b0:400c:c05::22c]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 34C3C698C3 for ; Mon, 28 Aug 2017 23:40:51 +0000 (UTC) (envelope-from sobomax@sippysoft.com) Received: by mail-vk0-x22c.google.com with SMTP id d124so5538555vkf.3 for ; Mon, 28 Aug 2017 16:40:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sippysoft-com.20150623.gappssmtp.com; s=20150623; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc; bh=iY6P06brFrJgQXuoV+1RP0+rBiUBAXu6aMu7sMvSFf4=; b=jg7EZfM/cp2sH5FqUYky45z2KdMABQ4w6QZ0xXzACiXfqnVFUvCvg4VnjyjQDdBXUT yRK2wyPwn//CR/ItFUtcO3SOdi/VJtgC1xIjg6AIKvOQ3IwGD9WJ9E+scqBVSwo8M2Yh WUhEKwd/HVGFl4kGDcXYsvoSkY3gkvo9Q4umVjCuhbvE2CO6vhUu+47B51c+5AseKJHe do1UTj4/qXKX5tVu3ZqkdyFCZOWoOMSdZ0kZHGvnqiJj6Dzv7WxooZt9Ttx2OkezV1tA MWqUfgPSGqv1THpOKrECJ6Yn8VE7ooq1J7I2KzCPHs4J/bKq2iYjJgXO1RB3MsFU2QhX DXUA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:sender:in-reply-to:references:from :date:message-id:subject:to:cc; bh=iY6P06brFrJgQXuoV+1RP0+rBiUBAXu6aMu7sMvSFf4=; b=XYM3GnH3nc8oesuRzVvEoGrRMYI98wbQs7UauTZghJshYEme/qSfL9ekwkJt98bb50 7Zgr7PYnWh9fKdH6+bxhUss0cVUJ4z2MqDmohtSEFIF0KacLgSr+BMv9PZsoJPFxxo32 MF2nfpVJ99ofiQPBGBNRs0lFT7CPL9P9+0TN9K5qB6k4M3rPEuKkVTwb/E2XGLu6gCy4 zA+1TF9oTwY1hycPbXPGFUeOoRXyMx0J1kX4bMwJER/RSZv/+SjePWV9YhbUQpepxS+k 7UJQoiRCSkd+GFEuKU0v7UiChwNaoR9g4c1IJh4coUeyF8y6hqhgq7dAttkcOpLJ/eHl qz7A== X-Gm-Message-State: AHYfb5iLe9wNE9/7Ow7IBx0A4mTCcuJRomnnE4fbB5CUIMX1Ks9gRXAr GIWVBYjAjsZAHazf+Af+w8wtNAy+mlsL X-Received: by 10.31.96.212 with SMTP id u203mr1461924vkb.30.1503963650158; Mon, 28 Aug 2017 16:40:50 -0700 (PDT) MIME-Version: 1.0 Sender: sobomax@sippysoft.com Received: by 10.176.6.137 with HTTP; Mon, 28 Aug 2017 16:40:49 -0700 (PDT) In-Reply-To: <2937323.CvTEtZnL2T@ralph.baldwin.cx> References: <201708281554.v7SFs8fr014268@repo.freebsd.org> <2937323.CvTEtZnL2T@ralph.baldwin.cx> From: Maxim Sobolev Date: Mon, 28 Aug 2017 16:40:49 -0700 X-Google-Sender-Auth: mnulmzlxIK1moCgL5QPPXodZvOs Message-ID: Subject: Re: svn commit: r322969 - in head: sbin/mdconfig sys/dev/md sys/sys To: John Baldwin Cc: Ryan Libby , svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.23 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 28 Aug 2017 23:40:51 -0000 John, well, this depends on how you look at it. The padding element size is "int", which when you account for the alignment has the nice property on both 32 and 64-bit arches that no matter what kind of element you add (char, short, int or void *), you only need to bring down MDNPAD by 1 to keep the structure size the same. It also has a second interesting property that number of padding elements needed stays the same no matter if sizeof(void *) is 64 or 32, otherwise you would have to have MDNPAD diverge on 32 and 64 bit arches after adding pointer which has different sizes (just like you suggested it should originally). I am not 100% sure if it was intentionally designed like this by PHK from the day one, but I found it quite interesting. I am not quite sure if having sizeof(md_ioctl) is ever been required to stay the same between 64 and 32 bit arches. I don't think there is any support for having 32-bit mdconfig run on 64-bit kernel. -Max On Mon, Aug 28, 2017 at 1:49 PM, John Baldwin wrote: > On Monday, August 28, 2017 12:46:48 PM Ryan Libby wrote: > > On Mon, Aug 28, 2017 at 11:24 AM, Maxim Sobolev > 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 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)); > > Ugh, yes. To me that means that MDNPAD is actually wrong and should be > fixed to account for the implicit padding. That probably would result in > requiring separate values for MDNPAD. The current change as-is certainly > looks wrong (and would be wrong if the padding were accurate) so it needs > to be fixed to reflect reality. > > -- > John Baldwin > >