From e8fd45a0f975c6b8ae8cd644714fc21f14fac2bf Mon Sep 17 00:00:00 2001 From: Brian Behlendorf Date: Fri, 26 Oct 2012 10:01:49 -0700 Subject: [PATCH] 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 Closes #910 --- cmd/zdb/zdb.c | 4 +++- include/sys/ddt.h | 6 +++--- module/zfs/ddt.c | 27 +++++++++++++++++++-------- module/zfs/ddt_zap.c | 10 +++------- 4 files changed, 28 insertions(+), 19 deletions(-) diff --git a/cmd/zdb/zdb.c b/cmd/zdb/zdb.c index 4a36732..ce6318e 100644 --- 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 --- 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 --- 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 --- 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 = { -- 1.8.3.1