zfs zvol: set geom mediasize right at creation time
Steven Hartland
killing at multiplay.co.uk
Sun Sep 23 01:04:51 UTC 2012
----- Original Message -----
From: "Andriy Gapon" <avg at FreeBSD.org>
> Please review the following patch.
>
> In addition to what the description says I almost by accident sneaked another
> change into the patch. It's setting of stripesize to volblocksize. I think
> that the change should make sense, but it is really a different change.
>
>
> A side note: setting sectorsize to volblocksize seemed like an overkill and it
> would certainly mess the existing zvols in use. Maybe there should be another
> property like reportedblocksize or something.
>
> commit 1585e6cfb602c2a2647b9f802445bb174bc430a4
> Author: Andriy Gapon <avg at icyb.net.ua>
> Date: Wed Sep 19 20:49:28 2012 +0300
>
> zvol: set mediasize in geom provider right upon its creation
>
> ... instead of deferring the action until first open.
> Unlike upstream this has no benefit on FreeBSD.
> We know that as soon as the provider is created it is going to be tasted
> and thus opened. Initial mediasize of zero causes tasting failure
> and subsequent retasting because of the size change.
>
> diff --git a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zvol.c
> b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zvol.c
> index d47d270..6e9e7a3 100644
> --- a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zvol.c
> +++ b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zvol.c
> @@ -475,6 +475,7 @@ zvol_create_minor(const char *name)
> zvol_state_t *zv;
> objset_t *os;
> dmu_object_info_t doi;
> + uint64_t volblocksize, volsize;
> int error;
>
> ZFS_LOG(1, "Creating ZVOL %s...", name);
> @@ -535,9 +536,20 @@ zvol_create_minor(const char *name)
> zv = zs->zss_data = kmem_zalloc(sizeof (zvol_state_t), KM_SLEEP);
> #else /* !sun */
>
> + error = zap_lookup(os, ZVOL_ZAP_OBJ, "size", 8, 1, &volsize);
> + if (error) {
> + ASSERT(error == 0);
> + dmu_objset_disown(os, zvol_tag);
> + mutex_exit(&spa_namespace_lock);
> + return (error);
> + }
> +
> DROP_GIANT();
> g_topology_lock();
> zv = zvol_geom_create(name);
> + zv->zv_volsize = volsize;
> + zv->zv_provider->mediasize = zv->zv_volsize;
> +
> #endif /* !sun */
>
> (void) strlcpy(zv->zv_name, name, MAXPATHLEN);
> @@ -554,6 +566,7 @@ zvol_create_minor(const char *name)
> error = dmu_object_info(os, ZVOL_OBJ, &doi);
> ASSERT(error == 0);
> zv->zv_volblocksize = doi.doi_data_block_size;
> + zv->zv_provider->stripesize = zv->zv_volblocksize;
>
> if (spa_writeable(dmu_objset_spa(os))) {
> if (zil_replay_disable)
Do you know what the effect of the volblocksize change
will have on a volume who's disk block size changes?
e.g. via a quirk for a 4k disk being added
I ask as we've testing a patch here which changes ashift to
be based on stripesize instead of sectorsize but in its
current form it has some odd side effects on pools which
are boot pools.
Said patch is attached for reference.
Regards
Steve
================================================
This e.mail is private and confidential between Multiplay (UK) Ltd. and the person or entity to whom it is addressed. In the event of misdirection, the recipient is prohibited from using, copying, printing or otherwise disseminating it or any information contained in it.
In the event of misdirection, illegible or incomplete transmission please telephone +44 845 868 1337
or return the E.mail to postmaster at multiplay.co.uk.
-------------- next part --------------
Changes zfs zpool initial / desired ashift to be based off stripesize
instead of sectorsize making it compatible with drives marked with
the 4k sector size quirk.
Without the correct min block size BIO_DELETE requests passed to
a large number of current SSD's via TRIM don't actually perform
any LBA TRIM so its vital for the correct operation of TRIM to get
the correct min block size.
To do this we added the additional dashift (desired ashift) to
vdev_open_func_t calls. This was needed as just updating ashift to
be based off stripesize would mean that a devices reported minimum
transfer size (ashift) could increase and that in turn would cause
member devices to be unusable and hence break pools with error
ZFS-8000-5E.
--- sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/vdev_impl.h.orig 2012-07-03 11:48:22.353483879 +0000
+++ sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/vdev_impl.h 2012-07-03 11:59:17.195442033 +0000
@@ -55,7 +55,7 @@
/*
* Virtual device operations
*/
-typedef int vdev_open_func_t(vdev_t *vd, uint64_t *size, uint64_t *ashift);
+typedef int vdev_open_func_t(vdev_t *vd, uint64_t *size, uint64_t *ashift, uint64_t *dashift);
typedef void vdev_close_func_t(vdev_t *vd);
typedef uint64_t vdev_asize_func_t(vdev_t *vd, uint64_t psize);
typedef int vdev_io_start_func_t(zio_t *zio);
--- sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev.c.orig 2012-07-03 12:53:37.716867380 +0000
+++ sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev.c 2012-07-03 12:01:58.522455031 +0000
@@ -1122,6 +1122,7 @@
uint64_t osize = 0;
uint64_t asize, psize;
uint64_t ashift = 0;
+ uint64_t dashift = 0;
ASSERT(vd->vdev_open_thread == curthread ||
spa_config_held(spa, SCL_STATE_ALL, RW_WRITER) == SCL_STATE_ALL);
@@ -1151,7 +1152,7 @@
return (ENXIO);
}
- error = vd->vdev_ops->vdev_op_open(vd, &osize, &ashift);
+ error = vd->vdev_ops->vdev_op_open(vd, &osize, &ashift, &dashift);
/*
* Reset the vdev_reopening flag so that we actually close
@@ -1251,7 +1252,7 @@
* For testing purposes, a higher ashift can be requested.
*/
vd->vdev_asize = asize;
- vd->vdev_ashift = MAX(ashift, vd->vdev_ashift);
+ vd->vdev_ashift = MAX(MAX(ashift, dashift), vd->vdev_ashift);
} else {
/*
* Make sure the alignment requirement hasn't increased.
--- sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_disk.c.orig 2012-07-03 11:49:34.103219588 +0000
+++ sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_disk.c 2012-07-03 12:51:44.521525471 +0000
@@ -103,7 +103,7 @@
}
static int
-vdev_disk_open(vdev_t *vd, uint64_t *psize, uint64_t *ashift)
+vdev_disk_open(vdev_t *vd, uint64_t *psize, uint64_t *ashift, uint64_t *dashift)
{
spa_t *spa = vd->vdev_spa;
vdev_disk_t *dvd;
@@ -284,7 +284,7 @@
}
/*
- * Determine the device's minimum transfer size.
+ * Determine the device's minimum and desired transfer size.
* If the ioctl isn't supported, assume DEV_BSIZE.
*/
if (ldi_ioctl(dvd->vd_lh, DKIOCGMEDIAINFOEXT, (intptr_t)&dkmext,
@@ -292,6 +292,7 @@
dkmext.dki_pbsize = DEV_BSIZE;
*ashift = highbit(MAX(dkmext.dki_pbsize, SPA_MINBLOCKSIZE)) - 1;
+ *dashift = *ashift;
/*
* Clear the nowritecache bit, so that on a vdev_reopen() we will
--- sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_file.c.orig 2012-07-03 11:48:42.314740333 +0000
+++ sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_file.c 2012-07-03 11:57:22.579381320 +0000
@@ -47,7 +47,7 @@
}
static int
-vdev_file_open(vdev_t *vd, uint64_t *psize, uint64_t *ashift)
+vdev_file_open(vdev_t *vd, uint64_t *psize, uint64_t *ashift, uint64_t *dashift)
{
vdev_file_t *vf;
vnode_t *vp;
@@ -127,6 +127,7 @@
*psize = vattr.va_size;
*ashift = SPA_MINBLOCKSHIFT;
+ *dashift = SPA_MINBLOCKSHIFT;
return (0);
}
--- sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c.orig 2012-07-03 12:50:50.158161825 +0000
+++ sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c 2012-07-03 12:52:01.408085155 +0000
@@ -416,7 +416,7 @@
}
static int
-vdev_geom_open(vdev_t *vd, uint64_t *psize, uint64_t *ashift)
+vdev_geom_open(vdev_t *vd, uint64_t *psize, uint64_t *ashift, uint64_t *dashift)
{
struct g_provider *pp;
struct g_consumer *cp;
@@ -502,9 +502,10 @@
*psize = pp->mediasize;
/*
- * Determine the device's minimum transfer size.
+ * Determine the device's minimum and desired transfer size.
*/
*ashift = highbit(MAX(pp->sectorsize, SPA_MINBLOCKSIZE)) - 1;
+ *dashift = highbit(MAX(pp->stripesize, SPA_MINBLOCKSIZE)) - 1;
/*
* Clear the nowritecache settings, so that on a vdev_reopen()
--- sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_mirror.c.orig 2012-07-03 11:49:22.342245151 +0000
+++ sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_mirror.c 2012-07-03 11:58:02.161948585 +0000
@@ -127,7 +127,7 @@
}
static int
-vdev_mirror_open(vdev_t *vd, uint64_t *asize, uint64_t *ashift)
+vdev_mirror_open(vdev_t *vd, uint64_t *asize, uint64_t *ashift, uint64_t *dashift)
{
int numerrors = 0;
int lasterror = 0;
@@ -150,6 +150,7 @@
*asize = MIN(*asize - 1, cvd->vdev_asize - 1) + 1;
*ashift = MAX(*ashift, cvd->vdev_ashift);
+ *dashift = *ashift;
}
if (numerrors == vd->vdev_children) {
--- sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_missing.c.orig 2012-07-03 11:49:10.545275865 +0000
+++ sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_missing.c 2012-07-03 11:58:07.670470640 +0000
@@ -40,7 +40,7 @@
/* ARGSUSED */
static int
-vdev_missing_open(vdev_t *vd, uint64_t *psize, uint64_t *ashift)
+vdev_missing_open(vdev_t *vd, uint64_t *psize, uint64_t *ashift, uint64_t *dashift)
{
/*
* Really this should just fail. But then the root vdev will be in the
@@ -50,6 +50,7 @@
*/
*psize = 0;
*ashift = 0;
+ *dashift = 0;
return (0);
}
--- sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_raidz.c.orig 2012-07-03 11:49:03.675875505 +0000
+++ sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_raidz.c 2012-07-03 11:58:15.334806334 +0000
@@ -1447,7 +1447,7 @@
}
static int
-vdev_raidz_open(vdev_t *vd, uint64_t *asize, uint64_t *ashift)
+vdev_raidz_open(vdev_t *vd, uint64_t *asize, uint64_t *ashift, uint64_t *dashift)
{
vdev_t *cvd;
uint64_t nparity = vd->vdev_nparity;
@@ -1476,6 +1476,7 @@
*asize = MIN(*asize - 1, cvd->vdev_asize - 1) + 1;
*ashift = MAX(*ashift, cvd->vdev_ashift);
+ *dashift = *ashift;
}
*asize *= vd->vdev_children;
--- sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_root.c.orig 2012-07-03 11:49:27.901760380 +0000
+++ sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_root.c 2012-07-03 11:58:19.704427068 +0000
@@ -50,7 +50,7 @@
}
static int
-vdev_root_open(vdev_t *vd, uint64_t *asize, uint64_t *ashift)
+vdev_root_open(vdev_t *vd, uint64_t *asize, uint64_t *ashift, uint64_t *dashift)
{
int lasterror = 0;
int numerrors = 0;
@@ -78,6 +78,7 @@
*asize = 0;
*ashift = 0;
+ *dashift = 0;
return (0);
}
More information about the freebsd-fs
mailing list