Skip to content

Commit

Permalink
Add zio constructor/destructor
Browse files Browse the repository at this point in the history
Add a standard zio constructor and destructor.  Normally, this is
done to reduce to cost of allocating a new structure by reducing
expensive operations such as memory allocations.  However, in this
case none of the operations moved out of zio_create() were really
very expensive.

This change was principly made as a debug patch (and workaround)
for a zio_destroy() race.  The is good evidence that zio_create()
is reinitializing a mutex which is really still in use by another
thread.  This would completely explain the observed symptoms in
the issue report.

This patch doesn't fix the root cause of the race, but it should
make it less likely by only initializing the mutex once in the
constructor.  Also, this particular flaw might have gone unnoticed
in other zfs implementations due to the specific implementation
details of Linux ticket spinlocks.

Once the real root cause is determined and resolved this change
can be safely reverted.  Until then this should help workaround
the issue.

Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Issue openzfs#496
  • Loading branch information
behlendorf committed Mar 21, 2012
1 parent c8df415 commit 49be0cc
Showing 1 changed file with 63 additions and 15 deletions.
78 changes: 63 additions & 15 deletions module/zfs/zio.c
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,35 @@ int zio_buf_debug_limit = 0;

static inline void __zio_execute(zio_t *zio);

static int
zio_cons(void *arg, void *unused, int kmflag)
{
zio_t *zio = arg;

bzero(zio, sizeof (zio_t));

mutex_init(&zio->io_lock, NULL, MUTEX_DEFAULT, NULL);
cv_init(&zio->io_cv, NULL, CV_DEFAULT, NULL);

list_create(&zio->io_parent_list, sizeof (zio_link_t),
offsetof(zio_link_t, zl_parent_node));
list_create(&zio->io_child_list, sizeof (zio_link_t),
offsetof(zio_link_t, zl_child_node));

return (0);
}

static void
zio_dest(void *arg, void *unused)
{
zio_t *zio = arg;

mutex_destroy(&zio->io_lock);
cv_destroy(&zio->io_cv);
list_destroy(&zio->io_parent_list);
list_destroy(&zio->io_child_list);
}

void
zio_init(void)
{
Expand All @@ -108,8 +137,8 @@ zio_init(void)
#ifdef _KERNEL
data_alloc_arena = zio_alloc_arena;
#endif
zio_cache = kmem_cache_create("zio_cache",
sizeof (zio_t), 0, NULL, NULL, NULL, NULL, NULL, KMC_KMEM);
zio_cache = kmem_cache_create("zio_cache", sizeof (zio_t), 0,
zio_cons, zio_dest, NULL, NULL, NULL, KMC_KMEM);
zio_link_cache = kmem_cache_create("zio_link_cache",
sizeof (zio_link_t), 0, NULL, NULL, NULL, NULL, NULL, KMC_KMEM);

Expand Down Expand Up @@ -511,15 +540,6 @@ zio_create(zio_t *pio, spa_t *spa, uint64_t txg, const blkptr_t *bp,
ASSERT(vd || stage == ZIO_STAGE_OPEN);

zio = kmem_cache_alloc(zio_cache, KM_PUSHPAGE);
bzero(zio, sizeof (zio_t));

mutex_init(&zio->io_lock, NULL, MUTEX_DEFAULT, NULL);
cv_init(&zio->io_cv, NULL, CV_DEFAULT, NULL);

list_create(&zio->io_parent_list, sizeof (zio_link_t),
offsetof(zio_link_t, zl_parent_node));
list_create(&zio->io_child_list, sizeof (zio_link_t),
offsetof(zio_link_t, zl_child_node));

if (vd != NULL)
zio->io_child_type = ZIO_CHILD_VDEV;
Expand All @@ -531,6 +551,7 @@ zio_create(zio_t *pio, spa_t *spa, uint64_t txg, const blkptr_t *bp,
zio->io_child_type = ZIO_CHILD_LOGICAL;

if (bp != NULL) {
zio->io_logical = NULL;
zio->io_bp = (blkptr_t *)bp;
zio->io_bp_copy = *bp;
zio->io_bp_orig = *bp;
Expand All @@ -541,21 +562,52 @@ zio_create(zio_t *pio, spa_t *spa, uint64_t txg, const blkptr_t *bp,
zio->io_logical = zio;
if (zio->io_child_type > ZIO_CHILD_GANG && BP_IS_GANG(bp))
pipeline |= ZIO_GANG_STAGES;
} else {
zio->io_logical = NULL;
zio->io_bp = NULL;
bzero(&zio->io_bp_copy, sizeof (blkptr_t));
bzero(&zio->io_bp_orig, sizeof (blkptr_t));
}

zio->io_spa = spa;
zio->io_txg = txg;
zio->io_ready = NULL;
zio->io_done = done;
zio->io_private = private;
zio->io_prev_space_delta = 0;
zio->io_type = type;
zio->io_priority = priority;
zio->io_vd = vd;
zio->io_vsd = NULL;
zio->io_vsd_ops = NULL;
zio->io_offset = offset;
zio->io_deadline = 0;
zio->io_orig_data = zio->io_data = data;
zio->io_orig_size = zio->io_size = size;
zio->io_orig_flags = zio->io_flags = flags;
zio->io_orig_stage = zio->io_stage = stage;
zio->io_orig_pipeline = zio->io_pipeline = pipeline;
bzero(&zio->io_prop, sizeof (zio_prop_t));
zio->io_cmd = 0;
zio->io_reexecute = 0;
zio->io_bp_override = NULL;
zio->io_walk_link = NULL;
zio->io_transform_stack = NULL;
zio->io_delay = 0;
zio->io_error = 0;
zio->io_child_count = 0;
zio->io_parent_count = 0;
zio->io_stall = NULL;
zio->io_gang_leader = NULL;
zio->io_gang_tree = NULL;
zio->io_executor = NULL;
zio->io_waiter = NULL;
zio->io_cksum_report = NULL;
zio->io_ena = 0;
bzero(zio->io_child_error, sizeof (int) * ZIO_CHILD_TYPES);
bzero(zio->io_children,
sizeof (uint64_t) * ZIO_CHILD_TYPES * ZIO_WAIT_TYPES);
bzero(&zio->io_bookmark, sizeof (zbookmark_t));

zio->io_state[ZIO_WAIT_READY] = (stage >= ZIO_STAGE_READY);
zio->io_state[ZIO_WAIT_DONE] = (stage >= ZIO_STAGE_DONE);
Expand All @@ -579,10 +631,6 @@ zio_create(zio_t *pio, spa_t *spa, uint64_t txg, const blkptr_t *bp,
static void
zio_destroy(zio_t *zio)
{
list_destroy(&zio->io_parent_list);
list_destroy(&zio->io_child_list);
mutex_destroy(&zio->io_lock);
cv_destroy(&zio->io_cv);
kmem_cache_free(zio_cache, zio);
}

Expand Down

0 comments on commit 49be0cc

Please sign in to comment.