Date: Tue, 7 Dec 2010 23:30:49 +0100 From: Ivan Voras <ivoras@freebsd.org> To: Pawel Jakub Dawidek <pjd@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r216230 - head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs Message-ID: <AANLkTinCepO7Uf_inNz6StyGOWVDTNHWw-Wk7HFrn5aF@mail.gmail.com> In-Reply-To: <20101207113152.GG1700@garage.freebsd.pl> References: <201012061218.oB6CI3oW032770@svn.freebsd.org> <20101206184453.GA1936@garage.freebsd.pl> <20101206192238.GB1936@garage.freebsd.pl> <AANLkTine9rGq_cM4ruFXYq=-F7cMXcQAr-zKHuWoQs2z@mail.gmail.com> <20101206195327.GD1936@garage.freebsd.pl> <AANLkTinUjzrUWc9Xn-iQYmSMZKVxCw9G-0HH868YonMy@mail.gmail.com> <20101207102104.GD1700@garage.freebsd.pl> <AANLkTi=TiqtFjjVGeheWV1yTq76jke9axbz2bvm4bQph@mail.gmail.com> <20101207113152.GG1700@garage.freebsd.pl>
next in thread | previous in thread | raw e-mail | index | archive | help
On 7 December 2010 12:31, Pawel Jakub Dawidek <pjd@freebsd.org> wrote: > On Tue, Dec 07, 2010 at 12:25:28PM +0100, Ivan Voras wrote: >> On 7 December 2010 11:21, Pawel Jakub Dawidek <pjd@freebsd.org> wrote: >> >> > PS. Do you know your change breaks all current ZFS installation if >> > stripesize is defined for a provider? >> > >> > =C2=A0 =C2=A0 =C2=A0 =C2=A0# zpool create tank ada0 >> > =C2=A0 =C2=A0 =C2=A0 =C2=A0(upgrade FreeBSD so that ada0 now reports 4= kB stripesize) >> > =C2=A0 =C2=A0 =C2=A0 =C2=A0# zpool import tank >> > =C2=A0 =C2=A0 =C2=A0 =C2=A0cannot import 'tank': invalid vdev configur= ation >> >> Actually I did test the patch and, similar to the Solaris people, >> found that ZFS records ashift in metadata and uses the recorded one >> instead of hardcoded / detected one. What you found here really >> shouldn't happen. Are you sure only stripesize was increased in your >> case, not sectorsize? > > =C2=A0 =C2=A0 =C2=A0 =C2=A0if (pp->stripesize > pp->sectorsize) > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0*ashift =3D highbi= t(MIN(pp->stripesize, SPA_MAXBLOCKSIZE)) - 1; > =C2=A0 =C2=A0 =C2=A0 =C2=A0else > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0*ashift =3D highbi= t(MAX(pp->sectorsize, SPA_MINBLOCKSIZE)) - 1; > > If stripesize will start to be 4096, the ashift will change. > > Not sure what you test was, but it only works the other way around - > create pool with 4kB ashift and import it when ashift is 512 bytes. My test was flawed, it made me conclude that ashift from metadata would be used in this case (this case is actually one of the more trivial ones - vdev created with ashift=3D9 used on device which is capable of ashift=3D9 but advertises ashift=3D12). In case it would be useful in the future, this is blocked in vdev_open(): http://fxr.watson.org/fxr/source/cddl/contrib/opensolaris/uts/common/fs/zfs= /vdev.c#L1075 I think that to solve this edge case properly, ZFS would have to know that both ashift values are "valid" for the device, i.e. basically it would at this point need to somehow know about our sectorsize and stripesize, not just ashift. OpenSolaris lists mention that pools can be created from devices with different ashift values, which is good news.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTinCepO7Uf_inNz6StyGOWVDTNHWw-Wk7HFrn5aF>