Date: Mon, 15 Aug 2016 08:04:26 -0700 From: Nathan Whitehorn <nwhitehorn@freebsd.org> To: =?UTF-8?Q?Dag-Erling_Sm=c3=b8rgrav?= <des@freebsd.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r304142 - head/usr.sbin/bsdinstall/partedit Message-ID: <e3454e8e-5d98-5bec-21de-8ea0db2b9b08@freebsd.org> In-Reply-To: <201608150930.u7F9UL1V069576@repo.freebsd.org> References: <201608150930.u7F9UL1V069576@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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; > } >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?e3454e8e-5d98-5bec-21de-8ea0db2b9b08>