Date: Wed, 10 Jul 2013 13:50:33 -0600 From: "Justin T. Gibbs" <gibbs@FreeBSD.org> To: "Steven Hartland" <killing@multiplay.co.uk> Cc: freebsd-fs@freebsd.org, =?iso-8859-1?Q?Dag-Erling_Sm=F8rgrav?= <des@des.no>, d@delphij.net, ivoras@freebsd.org, freebsd-hackers@freebsd.org Subject: Re: Make ZFS use the physical sector size when computing initial ashift Message-ID: <00205B20-742F-44F6-B538-3B809D8BC03F@FreeBSD.org> In-Reply-To: <7BB4167807A4434A9CD5FB0F1600439F@multiplay.co.uk> References: <86zjtupz3r.fsf@nine.des.no> <51DD9801.4090808@delphij.net> <2B9367B6-8759-45C9-B120-9D00A381228F@FreeBSD.org> <97E5A0A8DFBF4F75AAE8EDEFDF849EB0@multiplay.co.uk> <0A3A05F7-7859-4285-B15A-5E7DDB751062@FreeBSD.org> <7BB4167807A4434A9CD5FB0F1600439F@multiplay.co.uk>
next in thread | previous in thread | raw e-mail | index | archive | help
On Jul 10, 2013, at 1:42 PM, "Steven Hartland" <killing@multiplay.co.uk> = wrote: >=20 > ----- Original Message ----- From: "Justin T. Gibbs" >> On Jul 10, 2013, at 1:06 PM, "Steven Hartland" wrote: >>> ----- Original Message ----- From: "Justin T. Gibbs"=20 >>>> I'm sure lots of folks have "some solution" to this. Here is an >>>> old version of what we use at Spectra: >>>> http://people.freebsd.org/~gibbs/zfs_patches/zfs_auto_ashift.diff >>>> The above patch is missing some cleanup that was motivated by my >>>> discussions with George Wilson about this change in April. I'll >>>> dig that up later tonight. Even if you don't read the full diff, >>>> please read the included checkin comment since it explains the >>>> motivation behind this particular solution. >>>> This is on my list of things to upstream in the next week or so = after >>>> I add logic to the userspace tools to report whether or not the >>>> TLVs in a pool are using an optimal allocation size. This is only >>>> possible if you actually make ZFS fully aware of logical, physical, >>>> and the configured allocation size. All of the other patches I've = seen >>>> just treat physical as logical. >>> Reading through your patch it seems that your logical_ashift equates = to >>> the current ashift values which for geom devices is based off = sectorsize >>> and your physical_ashift is based stripesize. >>> This is almost identical to the approach I used adding a "desired = ashift", >>> which equates to your physical_ashift, along side the standard = ashift >>> i.e. required aka logical_ashift value :) >>=20 >> Yes, the approaches are similar. Our current version records the = logical >> access size in the vdev structure too, which might relate to the = issue >> below. >>=20 >> > One issue I did spot in your patch is that you currently expose >> > zfs_max_auto_ashift as a sysctl but don't clamp its value which = would >> > cause problems should a user configure values > 13. >>=20 >> I would expect the zio pipeline to simply insert an ashift aligned = thunking >> buffer for these operations, but I haven't tried going past an ashift = of 13 in >> my tests. If it is an issue, it seems the restriction should be = based on >> logical access size, not optimal access size. >=20 > Yes with your methodology you'll only see the issue if = zfs_max_auto_ashift > and physical_ashift are both > 13, but this can be the case for = example > on a RAID controller with large stripsize. I'm not sure I follow. logical_ashift is available in our latest code, = as is the physical_ashift. But even without the logical_ashift, why doesn't the = zio pipeline properly thunk zio_phys_read() access based on the configured = ashift? -- Justin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?00205B20-742F-44F6-B538-3B809D8BC03F>