From owner-svn-src-all@freebsd.org Mon Aug 15 15:04:35 2016 Return-Path: Delivered-To: svn-src-all@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 1B2F8BB9ABD; Mon, 15 Aug 2016 15:04:35 +0000 (UTC) (envelope-from nwhitehorn@freebsd.org) Received: from c.mail.sonic.net (c.mail.sonic.net [64.142.111.80]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id E3FF311F4; Mon, 15 Aug 2016 15:04:34 +0000 (UTC) (envelope-from nwhitehorn@freebsd.org) Received: from zeppelin.tachypleus.net (75-101-50-44.static.sonic.net [75.101.50.44]) (authenticated bits=0) by c.mail.sonic.net (8.15.1/8.15.1) with ESMTPSA id u7FF4QsD010802 (version=TLSv1.2 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT); Mon, 15 Aug 2016 08:04:26 -0700 Subject: Re: svn commit: r304142 - head/usr.sbin/bsdinstall/partedit To: =?UTF-8?Q?Dag-Erling_Sm=c3=b8rgrav?= , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org References: <201608150930.u7F9UL1V069576@repo.freebsd.org> From: Nathan Whitehorn Message-ID: Date: Mon, 15 Aug 2016 08:04:26 -0700 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:45.0) Gecko/20100101 Thunderbird/45.1.0 MIME-Version: 1.0 In-Reply-To: <201608150930.u7F9UL1V069576@repo.freebsd.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-Sonic-CAuth: UmFuZG9tSVb1e6vFjtl//K5a9p8n/04csfrrJgbdILv/jOPBoIVnE7PDyDxFA6m2+zCbrWZpkexRPiif6eI+MLiJERfd5iuTHtm8rUBUCuM= X-Sonic-ID: C;pJyvjvli5hG3j6Dx2xNB0g== M;5rcBj/li5hG3j6Dx2xNB0g== X-Spam-Flag: No X-Sonic-Spam-Details: 0.0/5.0 by cerberusd X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 15 Aug 2016 15:04:35 -0000 As a note for people who weren't paying attention to the bug, we need to fix this in a better way outside of the constraints of getting 11.0 out the door. The system (gpart, the installer, ZFS, etc.) uses the reported GEOM stripesize for partition alignment and IO block size selection. If that is wrong, we should identify devices on which it is wrong and fix them, and maybe also add some global tunable that sets a floor on the numbers reported by GEOM_DISK. Hacking the installer like this is triage, which is fine, but not viable as a permanent solution to anything. -Nathan On 08/15/16 02:30, Dag-Erling Smørgrav wrote: > Author: des > Date: Mon Aug 15 09:30:21 2016 > New Revision: 304142 > URL: https://svnweb.freebsd.org/changeset/base/304142 > > Log: > Ensure that the sector size is a multiple of 4096 to avoid creating > unaligned partitions when the actual sector size is hidden from us. > > PR: 211361 > MFC after: 3 days > > Modified: > head/usr.sbin/bsdinstall/partedit/gpart_ops.c > > Modified: head/usr.sbin/bsdinstall/partedit/gpart_ops.c > ============================================================================== > --- head/usr.sbin/bsdinstall/partedit/gpart_ops.c Mon Aug 15 09:27:15 2016 (r304141) > +++ head/usr.sbin/bsdinstall/partedit/gpart_ops.c Mon Aug 15 09:30:21 2016 (r304142) > @@ -795,6 +795,7 @@ gpart_max_free(struct ggeom *geom, intma > { > struct gconfig *gc; > struct gprovider *pp, **providers; > + intmax_t sectorsize, stripesize, offset; > intmax_t lastend; > intmax_t start, end; > intmax_t maxsize, maxstart; > @@ -845,12 +846,25 @@ gpart_max_free(struct ggeom *geom, intma > > pp = LIST_FIRST(&geom->lg_consumer)->lg_provider; > > - /* Compute beginning of new partition and maximum available space */ > - if (pp->lg_stripesize > 0 && > - (maxstart*pp->lg_sectorsize % pp->lg_stripesize) != 0) { > - intmax_t offset = (pp->lg_stripesize - > - ((maxstart*pp->lg_sectorsize) % pp->lg_stripesize)) / > - pp->lg_sectorsize; > + /* > + * Round the start and size of the largest available space up to > + * the nearest multiple of the adjusted stripe size. > + * > + * The adjusted stripe size is the least common multiple of the > + * actual stripe size, or the sector size if no stripe size was > + * reported, and 4096. The reason for this is that contemporary > + * disks often have 4096-byte physical sectors but report 512 > + * bytes instead for compatibility with older / broken operating > + * systems and BIOSes. For the same reasons, virtualized storage > + * may also report a 512-byte stripe size, or none at all. > + */ > + sectorsize = pp->lg_sectorsize; > + if ((stripesize = pp->lg_stripesize) == 0) > + stripesize = sectorsize; > + while (stripesize % 4096 != 0) > + stripesize *= 2; > + if ((offset = maxstart * sectorsize % stripesize) != 0) { > + offset = (stripesize - offset) / sectorsize; > maxstart += offset; > maxsize -= offset; > } >