Add ddt_object_count() error handling

The interface for the ddt_zap_count() function assumes it can
never fail.  However, internally ddt_zap_count() is implemented
with zap_count() which can potentially fail.  Now because there
was no way to return the error to the caller a VERIFY was used
to ensure this case never happens.

Unfortunately, it has been observed that pools can be damaged in
such a way that zap_count() fails.  The result is that the pool can
not be imported without hitting the VERIFY and crashing the system.

This patch reworks ddt_object_count() so the error can be safely
caught and returned to the caller.  This allows a pool which has
be damaged in this way to be safely rewound for import.

Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes #910

diff --git a/cmd/zdb/zdb.c b/cmd/zdb/zdb.c
index 4a36732..ce6318e 100644 (file)
--- a/cmd/zdb/zdb.c
+++ b/cmd/zdb/zdb.c
@@ -698,7 +698,9 @@ dump_ddt(ddt_t *ddt, enum ddt_type type, enum ddt_class class)
                return;
        ASSERT(error == 0);
 
-       if ((count = ddt_object_count(ddt, type, class)) == 0)
+       error = ddt_object_count(ddt, type, class, &count);
+       ASSERT(error == 0);
+       if (count == 0)
                return;
 
        dspace = doi.doi_physical_blocks_512 << 9;

diff --git a/include/sys/ddt.h b/include/sys/ddt.h
index 9724d6e..6943259 100644 (file)
--- a/include/sys/ddt.h
+++ b/include/sys/ddt.h
@@ -163,7 +163,7 @@ typedef struct ddt_ops {
            dmu_tx_t *tx);
        int (*ddt_op_walk)(objset_t *os, uint64_t object, ddt_entry_t *dde,
            uint64_t *walk);
-       uint64_t (*ddt_op_count)(objset_t *os, uint64_t object);
+       int (*ddt_op_count)(objset_t *os, uint64_t object, uint64_t *count);
 } ddt_ops_t;
 
 #define        DDT_NAMELEN     80
@@ -172,8 +172,8 @@ extern void ddt_object_name(ddt_t *ddt, enum ddt_type type,
     enum ddt_class class, char *name);
 extern int ddt_object_walk(ddt_t *ddt, enum ddt_type type,
     enum ddt_class class, uint64_t *walk, ddt_entry_t *dde);
-extern uint64_t ddt_object_count(ddt_t *ddt, enum ddt_type type,
-    enum ddt_class class);
+extern int ddt_object_count(ddt_t *ddt, enum ddt_type type,
+    enum ddt_class class, uint64_t *count);
 extern int ddt_object_info(ddt_t *ddt, enum ddt_type type,
     enum ddt_class class, dmu_object_info_t *);
 extern boolean_t ddt_object_exists(ddt_t *ddt, enum ddt_type type,

diff --git a/module/zfs/ddt.c b/module/zfs/ddt.c
index 895422b..6221157 100644 (file)
--- a/module/zfs/ddt.c
+++ b/module/zfs/ddt.c
@@ -82,13 +82,14 @@ ddt_object_destroy(ddt_t *ddt, enum ddt_type type, enum ddt_class class,
        spa_t *spa = ddt->ddt_spa;
        objset_t *os = ddt->ddt_os;
        uint64_t *objectp = &ddt->ddt_object[type][class];
+       uint64_t count;
        char name[DDT_NAMELEN];
 
        ddt_object_name(ddt, type, class, name);
 
        ASSERT(*objectp != 0);
-       ASSERT(ddt_object_count(ddt, type, class) == 0);
        ASSERT(ddt_histogram_empty(&ddt->ddt_histogram[type][class]));
+       VERIFY(ddt_object_count(ddt, type, class, &count) == 0 && count == 0);
        VERIFY(zap_remove(os, DMU_POOL_DIRECTORY_OBJECT, name, tx) == 0);
        VERIFY(zap_remove(os, spa->spa_ddt_stat_object, name, tx) == 0);
        VERIFY(ddt_ops[type]->ddt_op_destroy(os, *objectp, tx) == 0);
@@ -102,6 +103,7 @@ ddt_object_load(ddt_t *ddt, enum ddt_type type, enum ddt_class class)
 {
        ddt_object_t *ddo = &ddt->ddt_object_stats[type][class];
        dmu_object_info_t doi;
+       uint64_t count;
        char name[DDT_NAMELEN];
        int error;
 
@@ -124,7 +126,11 @@ ddt_object_load(ddt_t *ddt, enum ddt_type type, enum ddt_class class)
        if (error)
                return (error);
 
-       ddo->ddo_count = ddt_object_count(ddt, type, class);
+       error = ddt_object_count(ddt, type, class, &count);
+       if (error)
+               return (error);
+
+       ddo->ddo_count = count;
        ddo->ddo_dspace = doi.doi_physical_blocks_512 << 9;
        ddo->ddo_mspace = doi.doi_fill_count * doi.doi_data_block_size;
 
@@ -138,6 +144,7 @@ ddt_object_sync(ddt_t *ddt, enum ddt_type type, enum ddt_class class,
 {
        ddt_object_t *ddo = &ddt->ddt_object_stats[type][class];
        dmu_object_info_t doi;
+       uint64_t count;
        char name[DDT_NAMELEN];
 
        ddt_object_name(ddt, type, class, name);
@@ -150,8 +157,9 @@ ddt_object_sync(ddt_t *ddt, enum ddt_type type, enum ddt_class class,
         * Cache DDT statistics; this is the only time they'll change.
         */
        VERIFY(ddt_object_info(ddt, type, class, &doi) == 0);
+       VERIFY(ddt_object_count(ddt, type, class, &count) == 0);
 
-       ddo->ddo_count = ddt_object_count(ddt, type, class);
+       ddo->ddo_count = count;
        ddo->ddo_dspace = doi.doi_physical_blocks_512 << 9;
        ddo->ddo_mspace = doi.doi_fill_count * doi.doi_data_block_size;
 }
@@ -208,13 +216,14 @@ ddt_object_walk(ddt_t *ddt, enum ddt_type type, enum ddt_class class,
            ddt->ddt_object[type][class], dde, walk));
 }
 
-uint64_t
-ddt_object_count(ddt_t *ddt, enum ddt_type type, enum ddt_class class)
+int
+ddt_object_count(ddt_t *ddt, enum ddt_type type, enum ddt_class class,
+    uint64_t *count)
 {
        ASSERT(ddt_object_exists(ddt, type, class));
 
        return (ddt_ops[type]->ddt_op_count(ddt->ddt_os,
-           ddt->ddt_object[type][class]));
+           ddt->ddt_object[type][class], count));
 }
 
 int
@@ -1124,11 +1133,13 @@ ddt_sync_table(ddt_t *ddt, dmu_tx_t *tx, uint64_t txg)
        }
 
        for (type = 0; type < DDT_TYPES; type++) {
-               uint64_t count = 0;
+               uint64_t add, count = 0;
                for (class = 0; class < DDT_CLASSES; class++) {
                        if (ddt_object_exists(ddt, type, class)) {
                                ddt_object_sync(ddt, type, class, tx);
-                               count += ddt_object_count(ddt, type, class);
+                               VERIFY(ddt_object_count(ddt, type, class,
+                                   &add) == 0);
+                               count += add;
                        }
                }
                for (class = 0; class < DDT_CLASSES; class++) {

diff --git a/module/zfs/ddt_zap.c b/module/zfs/ddt_zap.c
index 1733448..65b14ab 100644 (file)
--- a/module/zfs/ddt_zap.c
+++ b/module/zfs/ddt_zap.c
@@ -138,14 +138,10 @@ ddt_zap_walk(objset_t *os, uint64_t object, ddt_entry_t *dde, uint64_t *walk)
        return (error);
 }
 
-static uint64_t
-ddt_zap_count(objset_t *os, uint64_t object)
+static int
+ddt_zap_count(objset_t *os, uint64_t object, uint64_t *count)
 {
-       uint64_t count = 0;
-
-       VERIFY(zap_count(os, object, &count) == 0);
-
-       return (count);
+       return zap_count(os, object, count);
 }
 
 const ddt_ops_t ddt_zap_ops = {