From 0091d66f4ebfeb83f70212ab92a4d5a8e4a9c4dc Mon Sep 17 00:00:00 2001 From: Olaf Faaland Date: Fri, 21 Apr 2017 14:26:43 -0700 Subject: [PATCH] Correct lock ASSERTs in vdev_label_read/write The existing assertions in vdev_label_read() and vdev_label_write(), testing which config locks are held, are incorrect. The assertions test for locks which exceed what is required for safety. Both vdev_label_{read,write}() are changed to assert SCL_STATE is held as RW_READER or RW_WRITER. This is safe because: Changes to the vdev tree occur under SCL_ALL as RW_WRITER, via spa_vdev_enter() and spa_vdev_exit(). Changes to vdev state occur under SCL_STATE_ALL as RW_WRITER, via spa_vdev_state_enter() and spa_vdev_state_exit(). Therefore, the new assertions guarantee that the vdev cannot change out from under a zio, and I/O to a specified leaf vdev's label is safe. Furthermore, this is consistent with the SPA locking discussion in spa_misc.c, "For any zio operation that takes an explicit vdev_t argument ... zio_read_phys(), or zio_write_phys() ... SCL_STATE as reader suffices." Reviewed-by: Chunwei Chen Reviewed-by: Brian Behlendorf Signed-off-by: Olaf Faaland Closes #5983 --- module/zfs/vdev_label.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/module/zfs/vdev_label.c b/module/zfs/vdev_label.c index 54c54237bfd5..b50a297bcb8b 100644 --- a/module/zfs/vdev_label.c +++ b/module/zfs/vdev_label.c @@ -182,8 +182,9 @@ static void vdev_label_read(zio_t *zio, vdev_t *vd, int l, abd_t *buf, uint64_t offset, uint64_t size, zio_done_func_t *done, void *private, int flags) { - ASSERT(spa_config_held(zio->io_spa, SCL_STATE_ALL, RW_WRITER) == - SCL_STATE_ALL); + ASSERT( + spa_config_held(zio->io_spa, SCL_STATE, RW_READER) == SCL_STATE || + spa_config_held(zio->io_spa, SCL_STATE, RW_WRITER) == SCL_STATE); ASSERT(flags & ZIO_FLAG_CONFIG_WRITER); zio_nowait(zio_read_phys(zio, vd, @@ -196,10 +197,9 @@ static void vdev_label_write(zio_t *zio, vdev_t *vd, int l, abd_t *buf, uint64_t offset, uint64_t size, zio_done_func_t *done, void *private, int flags) { - ASSERT(spa_config_held(zio->io_spa, SCL_ALL, RW_WRITER) == SCL_ALL || - (spa_config_held(zio->io_spa, SCL_CONFIG | SCL_STATE, RW_READER) == - (SCL_CONFIG | SCL_STATE) && - dsl_pool_sync_context(spa_get_dsl(zio->io_spa)))); + ASSERT( + spa_config_held(zio->io_spa, SCL_STATE, RW_READER) == SCL_STATE || + spa_config_held(zio->io_spa, SCL_STATE, RW_WRITER) == SCL_STATE); ASSERT(flags & ZIO_FLAG_CONFIG_WRITER); zio_nowait(zio_write_phys(zio, vd,