Illumos 5818 - zfs {ref}compressratio is incorrect with 4k sector size

5818 zfs {ref}compressratio is incorrect with 4k sector size
Reviewed by: Alex Reece <alex@delphix.com>
Reviewed by: George Wilson <george@delphix.com>
Reviewed by: Richard Elling <richard.elling@richardelling.com>
Reviewed by: Steven Hartland <killing@multiplay.co.uk>
Approved by: Albert Lee <trisk@omniti.com>

References:
  https://www.illumos.org/issues/5818
  https://github.com/illumos/illumos-gate/commit/81cd5c5

Ported-by: Don Brady <don.brady@intel.com>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes #3432

diff --git a/cmd/ztest/ztest.c b/cmd/ztest/ztest.c
index 1dfc25b6a5115396bd8e582e5ffd746c31d43939..f1a8ff61d7ee2c5aefd73f2feea3f02778e6cca0 100644 (file)
--- a/cmd/ztest/ztest.c
+++ b/cmd/ztest/ztest.c
@@ -20,7 +20,7 @@
  */
 /*
  * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved.
- * Copyright (c) 2011, 2014 by Delphix. All rights reserved.
+ * Copyright (c) 2011, 2015 by Delphix. All rights reserved.
  * Copyright 2011 Nexenta Systems, Inc.  All rights reserved.
  * Copyright (c) 2013 Steven Hartland. All rights reserved.
  */
@@ -1022,21 +1022,6 @@ ztest_random_spa_version(uint64_t initial_version)
        return (version);
 }
 
-/*
- * Find the largest ashift used
- */
-static uint64_t
-ztest_spa_get_ashift(void) {
-       uint64_t i;
-       uint64_t ashift = SPA_MINBLOCKSHIFT;
-       vdev_t *rvd = ztest_spa->spa_root_vdev;
-
-       for (i = 0; i < rvd->vdev_children; i++) {
-               ashift = MAX(ashift, rvd->vdev_child[i]->vdev_ashift);
-       }
-       return (ashift);
-}
-
 static int
 ztest_random_blocksize(void)
 {
@@ -1047,7 +1032,8 @@ ztest_random_blocksize(void)
        int maxbs = SPA_OLD_MAXBLOCKSHIFT;
        if (spa_maxblocksize(ztest_spa) == SPA_MAXBLOCKSIZE)
                maxbs = 20;
-       uint64_t block_shift = ztest_random(maxbs - ztest_spa_get_ashift() + 1);
+       uint64_t block_shift =
+           ztest_random(maxbs - ztest_spa->spa_max_ashift + 1);
        return (1 << (SPA_MINBLOCKSHIFT + block_shift));
 }
 

diff --git a/include/sys/spa_impl.h b/include/sys/spa_impl.h
index 25f28ef84c732858f0e52fbf15c58009624611ea..0b49c7147b1086c840c06b1bcbfb93bcfd28fed9 100644 (file)
--- a/include/sys/spa_impl.h
+++ b/include/sys/spa_impl.h
@@ -20,7 +20,7 @@
  */
 /*
  * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved.
- * Copyright (c) 2011, 2014 by Delphix. All rights reserved.
+ * Copyright (c) 2011, 2015 by Delphix. All rights reserved.
  * Copyright 2011 Nexenta Systems, Inc.  All rights reserved.
  * Copyright (c) 2014 Spectra Logic Corporation, All rights reserved.
  */
@@ -150,6 +150,8 @@ struct spa {
        kcondvar_t      spa_evicting_os_cv;     /* Objset Eviction Completion */
        txg_list_t      spa_vdev_txg_list;      /* per-txg dirty vdev list */
        vdev_t          *spa_root_vdev;         /* top-level vdev container */
+       int             spa_min_ashift;         /* of vdevs in normal class */
+       int             spa_max_ashift;         /* of vdevs in normal class */
        uint64_t        spa_config_guid;        /* config pool guid */
        uint64_t        spa_load_guid;          /* spa_load initialized guid */
        uint64_t        spa_last_synced_guid;   /* last synced guid */

diff --git a/include/sys/vdev_impl.h b/include/sys/vdev_impl.h
index 43c4c79415305e44eb6427314a0daf8ea7f919f5..6b27e75ae54d6e4b7ce0f646626f9cc1c28b3ecf 100644 (file)
--- a/include/sys/vdev_impl.h
+++ b/include/sys/vdev_impl.h
@@ -20,7 +20,7 @@
  */
 /*
  * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved.
- * Copyright (c) 2012, 2014 by Delphix. All rights reserved.
+ * Copyright (c) 2011, 2015 by Delphix. All rights reserved.
  */
 
 #ifndef _SYS_VDEV_IMPL_H
@@ -210,7 +210,7 @@ struct vdev {
        boolean_t       vdev_isl2cache; /* was a l2cache device         */
        vdev_queue_t    vdev_queue;     /* I/O deadline schedule queue  */
        vdev_cache_t    vdev_cache;     /* physical block cache         */
-       spa_aux_vdev_t  *vdev_aux;      /* for l2cache vdevs            */
+       spa_aux_vdev_t  *vdev_aux;      /* for l2cache and spares vdevs */
        zio_t           *vdev_probe_zio; /* root of current probe       */
        vdev_aux_t      vdev_label_aux; /* on-disk aux state            */
 

diff --git a/module/zfs/spa.c b/module/zfs/spa.c
index c37feb733413412de213c2122b9b4a0f2a35aec5..ad109a641c4625dd29326f8d5fd625792a32d747 100644 (file)
--- a/module/zfs/spa.c
+++ b/module/zfs/spa.c
@@ -2245,6 +2245,8 @@ spa_load_impl(spa_t *spa, uint64_t pool_guid, nvlist_t *config,
                return (error);
 
        ASSERT(spa->spa_root_vdev == rvd);
+       ASSERT3U(spa->spa_min_ashift, >=, SPA_MINBLOCKSHIFT);
+       ASSERT3U(spa->spa_max_ashift, <=, SPA_MAXBLOCKSHIFT);
 
        if (type != SPA_IMPORT_ASSEMBLE) {
                ASSERT(spa_guid(spa) == pool_guid);

diff --git a/module/zfs/spa_misc.c b/module/zfs/spa_misc.c
index b46b2d0a4ba32f16a1badd2430ba00b6acc45edf..26b6f1731a85c8fef13c18aaa8a7b4344f3079ec 100644 (file)
--- a/module/zfs/spa_misc.c
+++ b/module/zfs/spa_misc.c
@@ -20,7 +20,7 @@
  */
 /*
  * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved.
- * Copyright (c) 2011, 2014 by Delphix. All rights reserved.
+ * Copyright (c) 2011, 2015 by Delphix. All rights reserved.
  * Copyright 2011 Nexenta Systems, Inc.  All rights reserved.
  * Copyright (c) 2014 Spectra Logic Corporation, All rights reserved.
  */
@@ -623,6 +623,9 @@ spa_add(const char *name, nvlist_t *config, const char *altroot)
 
        spa->spa_debug = ((zfs_flags & ZFS_DEBUG_SPA) != 0);
 
+       spa->spa_min_ashift = INT_MAX;
+       spa->spa_max_ashift = 0;
+
        /*
         * As a pool is being created, treat all features as disabled by
         * setting SPA_FEATURE_DISABLED for all entries in the feature

diff --git a/module/zfs/vdev.c b/module/zfs/vdev.c
index d53537103b7998eca7029fbdddb972eddd377469..fe2ecdc72b07c0c6f8aa5f4186bbdec8b712e41c 100644 (file)
--- a/module/zfs/vdev.c
+++ b/module/zfs/vdev.c
@@ -22,7 +22,7 @@
 /*
  * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved.
  * Copyright 2011 Nexenta Systems, Inc.  All rights reserved.
- * Copyright (c) 2011, 2014 by Delphix. All rights reserved.
+ * Copyright (c) 2011, 2015 by Delphix. All rights reserved.
  */
 
 #include <sys/zfs_context.h>
@@ -206,8 +206,9 @@ vdev_add_child(vdev_t *pvd, vdev_t *cvd)
        size_t oldsize, newsize;
        uint64_t id = cvd->vdev_id;
        vdev_t **newchild;
+       spa_t *spa = cvd->vdev_spa;
 
-       ASSERT(spa_config_held(cvd->vdev_spa, SCL_ALL, RW_WRITER) == SCL_ALL);
+       ASSERT(spa_config_held(spa, SCL_ALL, RW_WRITER) == SCL_ALL);
        ASSERT(cvd->vdev_parent == NULL);
 
        cvd->vdev_parent = pvd;
@@ -1336,6 +1337,17 @@ vdev_open(vdev_t *vd)
                return (error);
        }
 
+       /*
+        * Track the min and max ashift values for normal data devices.
+        */
+       if (vd->vdev_top == vd && vd->vdev_ashift != 0 &&
+           !vd->vdev_islog && vd->vdev_aux == NULL) {
+               if (vd->vdev_ashift > spa->spa_max_ashift)
+                       spa->spa_max_ashift = vd->vdev_ashift;
+               if (vd->vdev_ashift < spa->spa_min_ashift)
+                       spa->spa_min_ashift = vd->vdev_ashift;
+       }
+
        /*
         * If a leaf vdev has a DTL, and seems healthy, then kick off a
         * resilver.  But don't do this if we are doing a reopen for a scrub,

diff --git a/module/zfs/zio.c b/module/zfs/zio.c
index 2b338f2a717961d3addcbb6b0994378b40776e3e..1e5be8bfc8380855094dedfcd109db653589006b 100644 (file)
--- a/module/zfs/zio.c
+++ b/module/zfs/zio.c
@@ -20,7 +20,7 @@
  */
 /*
  * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved.
- * Copyright (c) 2011, 2014 by Delphix. All rights reserved.
+ * Copyright (c) 2011, 2015 by Delphix. All rights reserved.
  * Copyright (c) 2011 Nexenta Systems, Inc. All rights reserved.
  */
 
@@ -1195,19 +1195,26 @@ zio_write_bp_init(zio_t *zio)
                        return (ZIO_PIPELINE_CONTINUE);
                } else {
                        /*
-                        * Round up compressed size to MINBLOCKSIZE and
-                        * zero the tail.
+                        * Round up compressed size up to the ashift
+                        * of the smallest-ashift device, and zero the tail.
+                        * This ensures that the compressed size of the BP
+                        * (and thus compressratio property) are correct,
+                        * in that we charge for the padding used to fill out
+                        * the last sector.
                         */
-                       size_t rounded =
-                           P2ROUNDUP(psize, (size_t)SPA_MINBLOCKSIZE);
-                       if (rounded > psize) {
-                               bzero((char *)cbuf + psize, rounded - psize);
-                               psize = rounded;
-                       }
-                       if (psize == lsize) {
+                       size_t rounded;
+
+                       ASSERT3U(spa->spa_min_ashift, >=, SPA_MINBLOCKSHIFT);
+
+                       rounded = (size_t)P2ROUNDUP(psize,
+                           1ULL << spa->spa_min_ashift);
+                       if (rounded >= lsize) {
                                compress = ZIO_COMPRESS_OFF;
                                zio_buf_free(cbuf, lsize);
+                               psize = lsize;
                        } else {
+                               bzero((char *)cbuf + psize, rounded - psize);
+                               psize = rounded;
                                zio_push_transform(zio, cbuf,
                                    psize, lsize, NULL);
                        }