From f1d99c0653911ddfc13414626b288ebada118ba2 Mon Sep 17 00:00:00 2001 From: Brian Behlendorf Date: Thu, 9 Jul 2009 12:10:52 -0700 Subject: [PATCH 1/3] Revert to original debugging code and wrap any variabled used only within an ASSERT with the ASSERTV macro which will ensure it will be removed when the ASSERTs are commented out. This makes gcc much happier, makes the variables usage explicit, and removes the need for the compiler to detect it is unused and do the right thing. --- cmd/ztest/ztest.c | 2 +- module/zfs/dbuf.c | 28 ++++++++++++++++------------ module/zfs/dmu.c | 6 ++++-- module/zfs/dmu_objset.c | 6 +++--- module/zfs/dnode.c | 4 +--- module/zfs/dnode_sync.c | 14 +++++--------- module/zfs/dsl_dataset.c | 12 +++++++----- module/zfs/dsl_dir.c | 3 ++- module/zfs/dsl_prop.c | 2 +- module/zfs/spa.c | 8 +++++--- module/zfs/vdev.c | 5 +++-- module/zfs/vdev_cache.c | 3 ++- module/zfs/zap_micro.c | 4 ++-- module/zfs/zio.c | 2 +- 14 files changed, 53 insertions(+), 46 deletions(-) diff --git a/cmd/ztest/ztest.c b/cmd/ztest/ztest.c index 746db0c070..5c70bb69f6 100644 --- a/cmd/ztest/ztest.c +++ b/cmd/ztest/ztest.c @@ -1149,7 +1149,7 @@ ztest_vdev_attach_detach(ztest_args_t *za) vdev_t * grow_vdev(vdev_t *vd, void *arg) { - spa_t *spa = vd->vdev_spa; + ASSERTV(spa_t *spa = vd->vdev_spa); size_t *newsize = arg; size_t fsize; int fd; diff --git a/module/zfs/dbuf.c b/module/zfs/dbuf.c index 271d0a8021..ff4cedd42c 100644 --- a/module/zfs/dbuf.c +++ b/module/zfs/dbuf.c @@ -316,12 +316,12 @@ dbuf_verify(dmu_buf_impl_t *db) * dnode_set_blksz(). */ if (db->db_level == 0 && db->db.db_object == DMU_META_DNODE_OBJECT) { + ASSERTV(dbuf_dirty_record_t *dr = db->db_data_pending); /* * It should only be modified in syncing context, so * make sure we only have one copy of the data. */ - ASSERT(db->db_data_pending == NULL || - db->db_data_pending->dt.dl.dr_data == db->db_buf); + ASSERT(dr == NULL || dr->dt.dl.dr_data == db->db_buf); } /* verify db->db_blkptr */ @@ -337,6 +337,8 @@ dbuf_verify(dmu_buf_impl_t *db) &dn->dn_phys->dn_blkptr[db->db_blkid]); } else { /* db is pointed to by an indirect block */ + ASSERTV(int epb = db->db_parent->db.db_size >> + SPA_BLKPTRSHIFT); ASSERT3U(db->db_parent->db_level, ==, db->db_level+1); ASSERT3U(db->db_parent->db.db_object, ==, db->db.db_object); @@ -348,9 +350,7 @@ dbuf_verify(dmu_buf_impl_t *db) if (RW_WRITE_HELD(&db->db_dnode->dn_struct_rwlock)) { ASSERT3P(db->db_blkptr, ==, ((blkptr_t *)db->db_parent->db.db_data + - db->db_blkid % - (db->db_parent->db.db_size >> - SPA_BLKPTRSHIFT))); + db->db_blkid % epb)); } } } @@ -363,10 +363,11 @@ dbuf_verify(dmu_buf_impl_t *db) * data when we evict this buffer. */ if (db->db_dirtycnt == 0) { + ASSERTV(uint64_t *buf = db->db.db_data); int i; for (i = 0; i < db->db.db_size >> 3; i++) { - ASSERT(((uint64_t *)db->db.db_data)[i] == 0); + ASSERT(buf[i] == 0); } } } @@ -1793,7 +1794,8 @@ dbuf_create_bonus(dnode_t *dn) void dbuf_add_ref(dmu_buf_impl_t *db, void *tag) { - VERIFY(refcount_add(&db->db_holds, tag) > 1); + ASSERTV(int64_t holds = refcount_add(&db->db_holds, tag)); + ASSERT(holds > 1); } #pragma weak dmu_buf_rele = dbuf_rele @@ -2387,15 +2389,17 @@ dbuf_write_done(zio_t *zio, arc_buf_t *buf, void *vdb) ASSERT(arc_released(db->db_buf)); } } else { + ASSERTV(dnode_t *dn = db->db_dnode); + ASSERT(list_head(&dr->dt.di.dr_children) == NULL); - ASSERT3U(db->db.db_size, ==, - 1<db_dnode->dn_phys->dn_indblkshift); + ASSERT3U(db->db.db_size, ==, 1<dn_phys->dn_indblkshift); if (!BP_IS_HOLE(db->db_blkptr)) { + ASSERTV(int epbs = dn->dn_phys->dn_indblkshift - + SPA_BLKPTRSHIFT); ASSERT3U(BP_GET_LSIZE(db->db_blkptr), ==, db->db.db_size); - ASSERT3U(db->db_dnode->dn_phys->dn_maxblkid >> (db->db_level * - (db->db_dnode->dn_phys->dn_indblkshift - SPA_BLKPTRSHIFT)), - >=, db->db_blkid); + ASSERT3U(dn->dn_phys->dn_maxblkid + >> (db->db_level * epbs), >=, db->db_blkid); arc_set_callback(db->db_buf, dbuf_do_evict, db); } mutex_destroy(&dr->dt.di.dr_mtx); diff --git a/module/zfs/dmu.c b/module/zfs/dmu.c index c0390b37d3..08320f6d57 100644 --- a/module/zfs/dmu.c +++ b/module/zfs/dmu.c @@ -879,8 +879,10 @@ dmu_sync_ready(zio_t *zio, arc_buf_t *buf, void *varg) blkptr_t *bp = zio->io_bp; if (!BP_IS_HOLE(bp)) { - ASSERT(BP_GET_TYPE(bp) == ((dmu_sync_arg_t *) - varg)->dr->dr_dbuf->db_dnode->dn_type); + ASSERTV(dmu_sync_arg_t *in = varg); + ASSERTV(dbuf_dirty_record_t *dr = in->dr); + ASSERTV(dmu_buf_impl_t *db = dr->dr_dbuf); + ASSERT(BP_GET_TYPE(bp) == db->db_dnode->dn_type); ASSERT(BP_GET_LEVEL(bp) == 0); bp->blk_fill = 1; } diff --git a/module/zfs/dmu_objset.c b/module/zfs/dmu_objset.c index 12a52ffed6..d4b4d5720c 100644 --- a/module/zfs/dmu_objset.c +++ b/module/zfs/dmu_objset.c @@ -891,9 +891,9 @@ static void ready(zio_t *zio, arc_buf_t *abuf, void *arg) { blkptr_t *bp = zio->io_bp; - blkptr_t *bp_orig = &zio->io_bp_orig; objset_impl_t *os = arg; dnode_phys_t *dnp = &os->os_phys->os_meta_dnode; + ASSERTV(blkptr_t *bp_orig = &zio->io_bp_orig); ASSERT(bp == os->os_rootbp); ASSERT(BP_GET_TYPE(bp) == DMU_OT_OBJSET); @@ -910,7 +910,7 @@ ready(zio_t *zio, arc_buf_t *abuf, void *arg) bp->blk_fill += dnp->dn_blkptr[i].blk_fill; if (zio->io_flags & ZIO_FLAG_IO_REWRITE) { - VERIFY(DVA_EQUAL(BP_IDENTITY(bp), BP_IDENTITY(bp_orig))); + ASSERT(DVA_EQUAL(BP_IDENTITY(bp), BP_IDENTITY(bp_orig))); } else { if (zio->io_bp_orig.blk_birth == os->os_synctx->tx_txg) (void) dsl_dataset_block_kill(os->os_dsl_dataset, @@ -1038,7 +1038,7 @@ dmu_objset_do_userquota_callbacks(objset_impl_t *os, dmu_tx_t *tx) { dnode_t *dn; list_t *list = &os->os_synced_dnodes; - static const char zerobuf[DN_MAX_BONUSLEN] = {0}; + ASSERTV(static const char zerobuf[DN_MAX_BONUSLEN] = {0}); ASSERT(list_head(list) == NULL || dmu_objset_userused_enabled(os)); diff --git a/module/zfs/dnode.c b/module/zfs/dnode.c index aeb44434bc..9615971018 100644 --- a/module/zfs/dnode.c +++ b/module/zfs/dnode.c @@ -40,9 +40,7 @@ static int free_range_compar(const void *node1, const void *node2); static kmem_cache_t *dnode_cache; -#ifndef NDEBUG -static dnode_phys_t dnode_phys_zero; -#endif +ASSERTV(static dnode_phys_t dnode_phys_zero); int zfs_default_bs = SPA_MINBLOCKSHIFT; int zfs_default_ibs = DN_MAX_INDBLKSHIFT; diff --git a/module/zfs/dnode_sync.c b/module/zfs/dnode_sync.c index 426aea0511..383eac8543 100644 --- a/module/zfs/dnode_sync.c +++ b/module/zfs/dnode_sync.c @@ -317,10 +317,8 @@ dnode_sync_free_range(dnode_t *dn, uint64_t blkid, uint64_t nblks, dmu_tx_t *tx) ASSERT3U(blkid + nblks, <=, dn->dn_phys->dn_nblkptr); (void) free_blocks(dn, bp + blkid, nblks, tx); if (trunc) { -#ifndef NDEBUG - uint64_t off = (dn->dn_phys->dn_maxblkid + 1) * - (dn->dn_phys->dn_datablkszsec << SPA_MINBLOCKSHIFT); -#endif + ASSERTV(uint64_t off = (dn->dn_phys->dn_maxblkid + 1) * + (dn->dn_phys->dn_datablkszsec<dn_phys->dn_maxblkid = (blkid ? blkid - 1 : 0); ASSERT(off < dn->dn_phys->dn_maxblkid || dn->dn_phys->dn_maxblkid == 0 || @@ -349,10 +347,8 @@ dnode_sync_free_range(dnode_t *dn, uint64_t blkid, uint64_t nblks, dmu_tx_t *tx) dbuf_rele(db, FTAG); } if (trunc) { -#ifndef NDEBUG - uint64_t off = (dn->dn_phys->dn_maxblkid + 1) * - (dn->dn_phys->dn_datablkszsec << SPA_MINBLOCKSHIFT); -#endif + ASSERTV(uint64_t off = (dn->dn_phys->dn_maxblkid + 1) * + (dn->dn_phys->dn_datablkszsec << SPA_MINBLOCKSHIFT)); dn->dn_phys->dn_maxblkid = (blkid ? blkid - 1 : 0); ASSERT(off < dn->dn_phys->dn_maxblkid || dn->dn_phys->dn_maxblkid == 0 || @@ -516,7 +512,7 @@ dnode_sync(dnode_t *dn, dmu_tx_t *tx) dnode_phys_t *dnp = dn->dn_phys; int txgoff = tx->tx_txg & TXG_MASK; list_t *list = &dn->dn_dirty_records[txgoff]; - static const dnode_phys_t zerodn = { 0 }; + ASSERTV(static const dnode_phys_t zerodn = { 0 }); ASSERT(dmu_tx_is_syncing(tx)); ASSERT(dnp->dn_type != DMU_OT_NONE || dn->dn_allocated_txg); diff --git a/module/zfs/dsl_dataset.c b/module/zfs/dsl_dataset.c index bafd6bacc9..1b78fd00f2 100644 --- a/module/zfs/dsl_dataset.c +++ b/module/zfs/dsl_dataset.c @@ -988,7 +988,7 @@ dsl_dataset_destroy(dsl_dataset_t *ds, void *tag) */ if (ds->ds_phys->ds_bp.blk_fill == 0 && dmu_objset_userused_enabled(os->os)) { - uint64_t count; + ASSERTV(uint64_t count); ASSERT(zap_count(os, DMU_USERUSED_OBJECT, &count) != 0 || count == 0); @@ -1725,8 +1725,8 @@ dsl_dataset_destroy_sync(void *arg1, void *tag, cred_t *cr, dmu_tx_t *tx) cr, "dataset = %llu", ds->ds_object); if (ds->ds_phys->ds_next_clones_obj != 0) { - uint64_t count; - VERIFY(0 == zap_count(mos, + ASSERTV(uint64_t count); + ASSERT(0 == zap_count(mos, ds->ds_phys->ds_next_clones_obj, &count) && count == 0); VERIFY(0 == dmu_object_free(mos, ds->ds_phys->ds_next_clones_obj, tx)); @@ -2027,8 +2027,10 @@ dsl_dataset_space(dsl_dataset_t *ds, boolean_t dsl_dataset_modified_since_lastsnap(dsl_dataset_t *ds) { - ASSERT(RW_LOCK_HELD(&(ds->ds_dir->dd_pool)->dp_config_rwlock) || - dsl_pool_sync_context(ds->ds_dir->dd_pool)); + ASSERTV(dsl_pool_t *dp = ds->ds_dir->dd_pool); + + ASSERT(RW_LOCK_HELD(&dp->dp_config_rwlock) || + dsl_pool_sync_context(dp)); if (ds->ds_prev == NULL) return (B_FALSE); if (ds->ds_phys->ds_bp.blk_birth > diff --git a/module/zfs/dsl_dir.c b/module/zfs/dsl_dir.c index 574b6155a5..00bbb1ee21 100644 --- a/module/zfs/dsl_dir.c +++ b/module/zfs/dsl_dir.c @@ -48,10 +48,11 @@ static void dsl_dir_evict(dmu_buf_t *db, void *arg) { dsl_dir_t *dd = arg; + ASSERTV(dsl_pool_t *dp = dd->dd_pool;) int t; for (t = 0; t < TXG_SIZE; t++) { - ASSERT(!txg_list_member(&dd->dd_pool->dp_dirty_dirs, dd, t)); + ASSERT(!txg_list_member(&dp->dp_dirty_dirs, dd, t)); ASSERT(dd->dd_tempreserved[t] == 0); ASSERT(dd->dd_space_towrite[t] == 0); } diff --git a/module/zfs/dsl_prop.c b/module/zfs/dsl_prop.c index 9bc1226743..665fed53d1 100644 --- a/module/zfs/dsl_prop.c +++ b/module/zfs/dsl_prop.c @@ -368,7 +368,7 @@ dsl_prop_set_sync(void *arg1, void *arg2, cred_t *cr, dmu_tx_t *tx) if (psa->numints == 0) { int err = zap_remove(mos, zapobj, psa->name, tx); - VERIFY(0 == err || ENOENT == err); + VERIFY(err == 0 || err == ENOENT); if (isint) { VERIFY(0 == dsl_prop_get_ds(ds, psa->name, 8, 1, &intval, NULL)); diff --git a/module/zfs/spa.c b/module/zfs/spa.c index 524c6f66b2..9b02b29e3e 100644 --- a/module/zfs/spa.c +++ b/module/zfs/spa.c @@ -2988,6 +2988,7 @@ int spa_vdev_attach(spa_t *spa, uint64_t guid, nvlist_t *nvroot, int replacing) { uint64_t txg, open_txg; + ASSERTV(vdev_t *rvd = spa->spa_root_vdev;) vdev_t *oldvd, *newvd, *newrootvd, *pvd, *tvd; vdev_ops_t *pvops; dmu_tx_t *tx; @@ -3104,7 +3105,7 @@ spa_vdev_attach(spa_t *spa, uint64_t guid, nvlist_t *nvroot, int replacing) if (pvd->vdev_ops != pvops) pvd = vdev_add_parent(oldvd, pvops); - ASSERT(pvd->vdev_top->vdev_parent == spa->spa_root_vdev); + ASSERT(pvd->vdev_top->vdev_parent == rvd); ASSERT(pvd->vdev_ops == pvops); ASSERT(oldvd->vdev_parent == pvd); @@ -3117,7 +3118,7 @@ spa_vdev_attach(spa_t *spa, uint64_t guid, nvlist_t *nvroot, int replacing) tvd = newvd->vdev_top; ASSERT(pvd->vdev_top == tvd); - ASSERT(tvd->vdev_parent == spa->spa_root_vdev); + ASSERT(tvd->vdev_parent == rvd); vdev_config_dirty(tvd); @@ -3179,6 +3180,7 @@ spa_vdev_detach(spa_t *spa, uint64_t guid, uint64_t pguid, int replace_done) { uint64_t txg; int error; + ASSERTV(vdev_t *rvd = spa->spa_root_vdev;) vdev_t *vd, *pvd, *cvd, *tvd; boolean_t unspare = B_FALSE; uint64_t unspare_guid; @@ -3322,7 +3324,7 @@ spa_vdev_detach(spa_t *spa, uint64_t guid, uint64_t pguid, int replace_done) * may have been the previous top-level vdev. */ tvd = cvd->vdev_top; - ASSERT(tvd->vdev_parent == spa->spa_root_vdev); + ASSERT(tvd->vdev_parent == rvd); /* * Reevaluate the parent vdev state. diff --git a/module/zfs/vdev.c b/module/zfs/vdev.c index 4635bf1ef4..c78992ee94 100644 --- a/module/zfs/vdev.c +++ b/module/zfs/vdev.c @@ -1239,8 +1239,9 @@ vdev_validate(vdev_t *vd) void vdev_close(vdev_t *vd) { - ASSERT(spa_config_held(vd->vdev_spa, SCL_STATE_ALL, RW_WRITER) == - SCL_STATE_ALL); + ASSERTV(spa_t *spa = vd->vdev_spa); + + ASSERT(spa_config_held(spa, SCL_STATE_ALL, RW_WRITER) == SCL_STATE_ALL); vd->vdev_ops->vdev_op_close(vd); diff --git a/module/zfs/vdev_cache.c b/module/zfs/vdev_cache.c index b02821b66a..9fbea035df 100644 --- a/module/zfs/vdev_cache.c +++ b/module/zfs/vdev_cache.c @@ -246,6 +246,7 @@ vdev_cache_read(zio_t *zio) vdev_cache_t *vc = &zio->io_vd->vdev_cache; vdev_cache_entry_t *ve, ve_search; uint64_t cache_offset = P2ALIGN(zio->io_offset, VCBS); + ASSERTV(uint64_t cache_phase = P2PHASE(zio->io_offset, VCBS);) zio_t *fio; ASSERT(zio->io_type == ZIO_TYPE_READ); @@ -262,7 +263,7 @@ vdev_cache_read(zio_t *zio) if (P2BOUNDARY(zio->io_offset, zio->io_size, VCBS)) return (EXDEV); - ASSERT(P2PHASE(zio->io_offset, VCBS) + zio->io_size <= VCBS); + ASSERT(cache_phase + zio->io_size <= VCBS); mutex_enter(&vc->vc_lock); diff --git a/module/zfs/zap_micro.c b/module/zfs/zap_micro.c index d882364977..95a5730f12 100644 --- a/module/zfs/zap_micro.c +++ b/module/zfs/zap_micro.c @@ -747,8 +747,8 @@ mzap_addent(zap_name_t *zn, uint64_t value) #ifdef ZFS_DEBUG for (i = 0; i < zap->zap_m.zap_num_chunks; i++) { - ASSERT(strcmp(zn->zn_name_orij, - (&zap->zap_m.zap_phys->mz_chunk[i])->mze_name) != 0); + ASSERTV(mzap_ent_phys_t *mze=&zap->zap_m.zap_phys->mz_chunk[i]); + ASSERT(strcmp(zn->zn_name_orij, mze->mze_name) != 0); } #endif diff --git a/module/zfs/zio.c b/module/zfs/zio.c index a2bdab9a7a..ea24216e70 100644 --- a/module/zfs/zio.c +++ b/module/zfs/zio.c @@ -1508,7 +1508,7 @@ static void zio_write_gang_member_ready(zio_t *zio) { zio_t *pio = zio_unique_parent(zio); - zio_t *gio = zio->io_gang_leader; + ASSERTV(zio_t *gio = zio->io_gang_leader); dva_t *cdva = zio->io_bp->blk_dva; dva_t *pdva = pio->io_bp->blk_dva; uint64_t asize; From a19906fe65312e94901166d6e9431d7599b904e2 Mon Sep 17 00:00:00 2001 From: Brian Behlendorf Date: Thu, 9 Jul 2009 12:13:56 -0700 Subject: [PATCH 2/3] Unitialized variables should be handled in the gcc-uninit topic branch. --- module/nvpair/nvpair.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/module/nvpair/nvpair.c b/module/nvpair/nvpair.c index e6344a9b72..77891bf776 100644 --- a/module/nvpair/nvpair.c +++ b/module/nvpair/nvpair.c @@ -1560,7 +1560,7 @@ nvlist_lookup_nvpair_ei_sep(nvlist_t *nvl, const char *name, const char sep, { nvpair_t *nvp; const char *np; - char *sepp = NULL; + char *sepp; char *idxp, *idxep; nvlist_t **nva; long idx; From a551134b2f9cb505018b22d62e1b778f229f4e45 Mon Sep 17 00:00:00 2001 From: Brian Behlendorf Date: Thu, 9 Jul 2009 12:14:56 -0700 Subject: [PATCH 3/3] Unitialized variables should be handled in the gcc-uninit topic branch. --- module/nvpair/nvpair.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/module/nvpair/nvpair.c b/module/nvpair/nvpair.c index 2cf1c51a8c..3e147617b0 100644 --- a/module/nvpair/nvpair.c +++ b/module/nvpair/nvpair.c @@ -1560,7 +1560,7 @@ nvlist_lookup_nvpair_ei_sep(nvlist_t *nvl, const char *name, const char sep, { nvpair_t *nvp; const char *np; - char *sepp; + char *sepp = NULL; char *idxp, *idxep; nvlist_t **nva; long idx = 0;