From 5b02d6c0841a47b3a2167bfbd04c9223f77dcf8a Mon Sep 17 00:00:00 2001 From: Brian Behlendorf Date: Mon, 21 Jun 2010 21:22:46 -0700 Subject: [PATCH 1/6] Zero struct for zdb dump_block_stats Accidentally dropped the zeroing of this structure in the gcc-missing-braces topic branch which was causing a fall positive space leak in ztest. Ensure the structure is zero'ed before use. --- cmd/zdb/zdb.c | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/zdb/zdb.c b/cmd/zdb/zdb.c index 1f4fcfef7a..82940712dd 100644 --- a/cmd/zdb/zdb.c +++ b/cmd/zdb/zdb.c @@ -2179,6 +2179,7 @@ dump_block_stats(spa_t *spa) * it's not part of any space map) is a double allocation, * reference to a freed block, or an unclaimed log block. */ + bzero(&zcb, sizeof(zdb_cb_t)); zdb_leak_init(spa, &zcb); /* From f9a7332118c0bfa6df0614935c9bbf4cc5d83c64 Mon Sep 17 00:00:00 2001 From: Brian Behlendorf Date: Mon, 21 Jun 2010 21:27:25 -0700 Subject: [PATCH 2/6] Update kmem_asprintf() and kmem_vasprintf() implementation On a Linux system simply use the native aprintf and vasprintf functions respectively. Also update the call points to correctly use va_copy() or va_start() as appropriate. --- lib/libzpool/include/sys/zfs_context.h | 1 + lib/libzpool/kernel.c | 28 ++++++++++++++------------ module/zfs/spa_history.c | 5 ++++- 3 files changed, 20 insertions(+), 14 deletions(-) diff --git a/lib/libzpool/include/sys/zfs_context.h b/lib/libzpool/include/sys/zfs_context.h index 7f736efb71..58fd12c04a 100644 --- a/lib/libzpool/include/sys/zfs_context.h +++ b/lib/libzpool/include/sys/zfs_context.h @@ -562,6 +562,7 @@ typedef struct callb_cpr { #define zone_dataset_visible(x, y) (1) #define INGLOBALZONE(z) (1) +extern char *kmem_vasprintf(const char *fmt, va_list adx); extern char *kmem_asprintf(const char *fmt, ...); #define strfree(str) kmem_free((str), strlen(str)+1) diff --git a/lib/libzpool/kernel.c b/lib/libzpool/kernel.c index f9488177e3..b542d1fc34 100644 --- a/lib/libzpool/kernel.c +++ b/lib/libzpool/kernel.c @@ -1086,25 +1086,27 @@ ksiddomain_rele(ksiddomain_t *ksid) umem_free(ksid, sizeof (ksiddomain_t)); } -/* - * Do not change the length of the returned string; it must be freed - * with strfree(). - */ +char * +kmem_vasprintf(const char *fmt, va_list adx) +{ + char *buf = NULL; + va_list adx_copy; + + va_copy(adx_copy, adx); + VERIFY(vasprintf(&buf, fmt, adx_copy) != -1); + va_end(adx_copy); + + return (buf); +} + char * kmem_asprintf(const char *fmt, ...) { - int size; + char *buf = NULL; va_list adx; - char *buf; va_start(adx, fmt); - size = vsnprintf(NULL, 0, fmt, adx) + 1; - va_end(adx); - - buf = kmem_alloc(size, KM_SLEEP); - - va_start(adx, fmt); - size = vsnprintf(buf, size, fmt, adx); + VERIFY(vasprintf(&buf, fmt, adx) != -1); va_end(adx); return (buf); diff --git a/module/zfs/spa_history.c b/module/zfs/spa_history.c index b7b5e32271..ce7d378c6f 100644 --- a/module/zfs/spa_history.c +++ b/module/zfs/spa_history.c @@ -428,6 +428,7 @@ log_internal(history_internal_events_t event, spa_t *spa, dmu_tx_t *tx, const char *fmt, va_list adx) { history_arg_t *ha; + va_list adx_copy; /* * If this is part of creating a pool, not everything is @@ -437,7 +438,9 @@ log_internal(history_internal_events_t event, spa_t *spa, return; ha = kmem_alloc(sizeof (history_arg_t), KM_SLEEP); - ha->ha_history_str = kmem_asprintf(fmt, adx); + va_copy(adx_copy, adx); + ha->ha_history_str = kmem_vasprintf(fmt, adx_copy); + va_end(adx_copy); ha->ha_log_type = LOG_INTERNAL; ha->ha_event = event; ha->ha_zone = NULL; From 643eada5f663e85076f06cfcdbdddf2cc028cc2f Mon Sep 17 00:00:00 2001 From: Brian Behlendorf Date: Thu, 24 Jun 2010 09:39:27 -0700 Subject: [PATCH 3/6] Disable zero-copy in zpios We need to update this code to use the new API. For now simply comment it out until it can be correctly implemented. --- module/zpios/zpios.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/module/zpios/zpios.c b/module/zpios/zpios.c index ff2db3b39b..9aa3b5c1bb 100644 --- a/module/zpios/zpios.c +++ b/module/zpios/zpios.c @@ -443,7 +443,7 @@ zpios_dmu_write(run_args_t *run_args, objset_t *os, uint64_t object, { struct dmu_tx *tx; int rc, how = TXG_WAIT; - int flags = 0; +// int flags = 0; if (run_args->flags & DMU_WRITE_NOWAIT) how = TXG_NOWAIT; @@ -467,10 +467,10 @@ zpios_dmu_write(run_args_t *run_args, objset_t *os, uint64_t object, break; } - if (run_args->flags & DMU_WRITE_ZC) - flags |= DMU_WRITE_ZEROCOPY; +// if (run_args->flags & DMU_WRITE_ZC) +// flags |= DMU_WRITE_ZEROCOPY; - dmu_write_impl(os, object, offset, size, buf, tx, flags); + dmu_write(os, object, offset, size, buf, tx); dmu_tx_commit(tx); return 0; @@ -482,8 +482,8 @@ zpios_dmu_read(run_args_t *run_args, objset_t *os, uint64_t object, { int flags = 0; - if (run_args->flags & DMU_READ_ZC) - flags |= DMU_READ_ZEROCOPY; +// if (run_args->flags & DMU_READ_ZC) +// flags |= DMU_READ_ZEROCOPY; if (run_args->flags & DMU_READ_NOPF) flags |= DMU_READ_NO_PREFETCH; From 4970b635e83b0eb53326db040f94c034d815f213 Mon Sep 17 00:00:00 2001 From: Brian Behlendorf Date: Mon, 21 Jun 2010 21:34:32 -0700 Subject: [PATCH 4/6] Revert to original debugging The ZFS defaults are fine, revert to them. --- module/zfs/spa_misc.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/module/zfs/spa_misc.c b/module/zfs/spa_misc.c index 6763ae8c9b..3999bf7032 100644 --- a/module/zfs/spa_misc.c +++ b/module/zfs/spa_misc.c @@ -233,13 +233,8 @@ kmem_cache_t *spa_buffer_pool; int spa_mode_global; #ifdef ZFS_DEBUG -#if defined(_KERNEL) && defined(HAVE_SPL) -/* All filtering done by the SPL */ -int zfs_flags = ~0; -#else /* Everything except dprintf is on by default in debug builds */ int zfs_flags = ~ZFS_DEBUG_DPRINTF; -#endif #else int zfs_flags = 0; #endif From 882ec504ea85b34b1ceffbd41db286e63b3779e4 Mon Sep 17 00:00:00 2001 From: Brian Behlendorf Date: Mon, 21 Jun 2010 21:24:54 -0700 Subject: [PATCH 5/6] Minor ztest fixes Move create/destroy function to correct places. I'm not sure why this wasn't caught upstream it should have been, regardless let's just fix it here. Personally I find it handy to be able to enable full debugging in zfs with the 'debug=' command line option so I'm enabled that as well. --- cmd/ztest/ztest.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/cmd/ztest/ztest.c b/cmd/ztest/ztest.c index eb31139f27..7bfa1ce55b 100644 --- a/cmd/ztest/ztest.c +++ b/cmd/ztest/ztest.c @@ -5274,6 +5274,11 @@ ztest_run(ztest_shared_t *zs) } kernel_fini(); + + list_destroy(&zcl.zcl_callbacks); + mutex_destroy(&zcl.zcl_callbacks_lock); + rw_destroy(&zs->zs_name_lock); + mutex_destroy(&zs->zs_vdev_lock); } static void @@ -5344,12 +5349,8 @@ ztest_freeze(ztest_shared_t *zs) spa_close(spa, FTAG); kernel_fini(); - list_destroy(&zcl.zcl_callbacks); - - (void) mutex_destroy(&zcl.zcl_callbacks_lock); - - (void) rw_destroy(&zs->zs_name_lock); - (void) mutex_destroy(&zs->zs_vdev_lock); + rw_destroy(&zs->zs_name_lock); + mutex_destroy(&zs->zs_vdev_lock); } void @@ -5452,6 +5453,7 @@ main(int argc, char **argv) ztest_random_fd = open("/dev/urandom", O_RDONLY); + dprintf_setup(&argc, argv); process_options(argc, argv); /* Override location of zpool.cache */ From f32ec2346b9f276ddb40ce92f49174127c2b3e48 Mon Sep 17 00:00:00 2001 From: Brian Behlendorf Date: Fri, 25 Jun 2010 14:13:37 -0700 Subject: [PATCH 6/6] Linux bcopy() requires word aligned memory, use memcpy() Much to my surprise bcopy() under Linux appears to copy the data in word sized chunks. It does the right thing but if you buffer is not a multiple of the word size you will be reading past the end of your buffer. Or at least that is what valgrind is reporting. We should be using mempcy() anyway on Linux so replace bcopy() with memcpy() to resolve the issue. ==305== Thread 211: ==305== Invalid read of size 8 ==305== at 0x3BCD28357D: _wordcopy_fwd_dest_aligned (in /lib64/libc-2.11.1.so) ==305== by 0x3BCD282B05: bcopy (in /lib64/libc-2.11.1.so) ==305== by 0x58D7FEF: dmu_write (dmu.c:730) ==305== by 0x591C942: spa_history_write (spa_history.c:165) ==305== by 0x591D255: spa_history_log_sync (spa_history.c:277) ==305== by 0x591D545: log_internal (spa_history.c:450) ==305== by 0x591D5EC: spa_history_log_internal (spa_history.c:475) ==305== by 0x5902319: dsl_prop_set_sync (dsl_prop.c:707) ==305== by 0x5906A7D: dsl_sync_task_group_sync (dsl_synctask.c:199) ==305== by 0x58FF4EC: dsl_pool_sync (dsl_pool.c:376) ==305== by 0x591744C: spa_sync (spa.c:5365) ==305== by 0x5922C85: txg_sync_thread (txg.c:414) --- module/zfs/dmu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/module/zfs/dmu.c b/module/zfs/dmu.c index dd09dda18e..53a1d3bc5b 100644 --- a/module/zfs/dmu.c +++ b/module/zfs/dmu.c @@ -727,7 +727,7 @@ dmu_write(objset_t *os, uint64_t object, uint64_t offset, uint64_t size, else dmu_buf_will_dirty(db, tx); - bcopy(buf, (char *)db->db_data + bufoff, tocpy); + (void) memcpy((char *)db->db_data + bufoff, buf, tocpy); if (tocpy == db->db_size) dmu_buf_fill_done(db, tx);