From 4f64b0f063664060571632204c027c21b7e5f1d3 Mon Sep 17 00:00:00 2001 From: Thomas Bertschinger Date: Tue, 22 Aug 2023 20:42:48 -0600 Subject: [PATCH] Add mutex_enter_interruptible() for interruptible sleeping IOCTLs Many long-running ZFS ioctls lock the spa_namespace_lock, forcing concurrent ioctls to sleep for the mutex. Previously, the only option is to call mutex_enter() which sleeps uninterruptibly. This is a usability issue for sysadmins, for example, if the admin runs `zpool status` while a slow `zpool import` is ongoing, the admin's shell will be locked in uninterruptible sleep for a long time. This patch resolves this admin usability issue by introducing mutex_enter_interruptible() which sleeps interruptibly while waiting to acquire a lock. It is implemented for both Linux and FreeBSD. The ZFS_IOC_POOL_CONFIGS ioctl, used by `zpool status`, is changed to use this new macro so that the command can be interrupted if it is issued during a concurrent `zpool import` (or other long-running operation). Signed-off-by: Thomas Bertschinger --- include/os/freebsd/spl/sys/mutex.h | 1 + include/os/linux/spl/sys/mutex.h | 21 +++++++++++++-------- include/sys/spa.h | 2 +- include/sys/zfs_context.h | 2 ++ lib/libzpool/kernel.c | 9 +++++++++ module/zfs/spa_config.c | 17 +++++++++-------- module/zfs/zfs_ioctl.c | 5 +++-- 7 files changed, 38 insertions(+), 19 deletions(-) diff --git a/include/os/freebsd/spl/sys/mutex.h b/include/os/freebsd/spl/sys/mutex.h index e757d12c1502..8cfe56c75309 100644 --- a/include/os/freebsd/spl/sys/mutex.h +++ b/include/os/freebsd/spl/sys/mutex.h @@ -64,6 +64,7 @@ typedef enum { } while (0) #define mutex_destroy(lock) sx_destroy(lock) #define mutex_enter(lock) sx_xlock(lock) +#define mutex_enter_interruptible(lock) sx_xlock_sig(lock) #define mutex_enter_nested(lock, type) sx_xlock(lock) #define mutex_tryenter(lock) sx_try_xlock(lock) #define mutex_exit(lock) sx_xunlock(lock) diff --git a/include/os/linux/spl/sys/mutex.h b/include/os/linux/spl/sys/mutex.h index 6b61c59c48e2..b4eaa0266d20 100644 --- a/include/os/linux/spl/sys/mutex.h +++ b/include/os/linux/spl/sys/mutex.h @@ -128,7 +128,6 @@ spl_mutex_lockdep_on_maybe(kmutex_t *mp) \ #define NESTED_SINGLE 1 -#ifdef CONFIG_DEBUG_LOCK_ALLOC #define mutex_enter_nested(mp, subclass) \ { \ ASSERT3P(mutex_owner(mp), !=, current); \ @@ -137,16 +136,22 @@ spl_mutex_lockdep_on_maybe(kmutex_t *mp) \ spl_mutex_lockdep_on_maybe(mp); \ spl_mutex_set_owner(mp); \ } -#else /* CONFIG_DEBUG_LOCK_ALLOC */ -#define mutex_enter_nested(mp, subclass) \ -{ \ + +#define mutex_enter_interruptible(mp) \ +/* CSTYLED */ \ +({ \ + int _rc_; \ + \ ASSERT3P(mutex_owner(mp), !=, current); \ spl_mutex_lockdep_off_maybe(mp); \ - mutex_lock(MUTEX(mp)); \ + _rc_ = mutex_lock_interruptible(MUTEX(mp)); \ spl_mutex_lockdep_on_maybe(mp); \ - spl_mutex_set_owner(mp); \ -} -#endif /* CONFIG_DEBUG_LOCK_ALLOC */ + if (!_rc_) { \ + spl_mutex_set_owner(mp); \ + } \ + \ + _rc_; \ +}) #define mutex_enter(mp) mutex_enter_nested((mp), 0) diff --git a/include/sys/spa.h b/include/sys/spa.h index 18062d3f2a95..88ef510b744b 100644 --- a/include/sys/spa.h +++ b/include/sys/spa.h @@ -837,7 +837,7 @@ extern kmutex_t spa_namespace_lock; extern void spa_write_cachefile(spa_t *, boolean_t, boolean_t, boolean_t); extern void spa_config_load(void); -extern nvlist_t *spa_all_configs(uint64_t *); +extern int spa_all_configs(uint64_t *generation, nvlist_t **pools); extern void spa_config_set(spa_t *spa, nvlist_t *config); extern nvlist_t *spa_config_generate(spa_t *spa, vdev_t *vd, uint64_t txg, int getstats); diff --git a/include/sys/zfs_context.h b/include/sys/zfs_context.h index 0d31195447d1..5181e5ef83b6 100644 --- a/include/sys/zfs_context.h +++ b/include/sys/zfs_context.h @@ -274,11 +274,13 @@ typedef struct kmutex { extern void mutex_init(kmutex_t *mp, char *name, int type, void *cookie); extern void mutex_destroy(kmutex_t *mp); extern void mutex_enter(kmutex_t *mp); +extern int mutex_enter_check_return(kmutex_t *mp); extern void mutex_exit(kmutex_t *mp); extern int mutex_tryenter(kmutex_t *mp); #define NESTED_SINGLE 1 #define mutex_enter_nested(mp, class) mutex_enter(mp) +#define mutex_enter_interruptible(mp) mutex_enter_check_return(mp) /* * RW locks */ diff --git a/lib/libzpool/kernel.c b/lib/libzpool/kernel.c index a9b9bf4c2ce5..ffad7fc02bc9 100644 --- a/lib/libzpool/kernel.c +++ b/lib/libzpool/kernel.c @@ -205,6 +205,15 @@ mutex_enter(kmutex_t *mp) mp->m_owner = pthread_self(); } +int +mutex_enter_check_return(kmutex_t *mp) +{ + int error = pthread_mutex_lock(&mp->m_lock); + if (error == 0) + mp->m_owner = pthread_self(); + return (error); +} + int mutex_tryenter(kmutex_t *mp) { diff --git a/module/zfs/spa_config.c b/module/zfs/spa_config.c index 636c04d9f785..a77874ea0dd3 100644 --- a/module/zfs/spa_config.c +++ b/module/zfs/spa_config.c @@ -367,23 +367,24 @@ spa_write_cachefile(spa_t *target, boolean_t removing, boolean_t postsysevent, * So we have to invent the ZFS_IOC_CONFIG ioctl to grab the configuration * information for all pool visible within the zone. */ -nvlist_t * -spa_all_configs(uint64_t *generation) +int +spa_all_configs(uint64_t *generation, nvlist_t **pools) { - nvlist_t *pools; spa_t *spa = NULL; if (*generation == spa_config_generation) - return (NULL); + return (SET_ERROR(EEXIST)); - pools = fnvlist_alloc(); + int error = mutex_enter_interruptible(&spa_namespace_lock); + if (error) + return (SET_ERROR(EINTR)); - mutex_enter(&spa_namespace_lock); + *pools = fnvlist_alloc(); while ((spa = spa_next(spa)) != NULL) { if (INGLOBALZONE(curproc) || zone_dataset_visible(spa_name(spa), NULL)) { mutex_enter(&spa->spa_props_lock); - fnvlist_add_nvlist(pools, spa_name(spa), + fnvlist_add_nvlist(*pools, spa_name(spa), spa->spa_config); mutex_exit(&spa->spa_props_lock); } @@ -391,7 +392,7 @@ spa_all_configs(uint64_t *generation) *generation = spa_config_generation; mutex_exit(&spa_namespace_lock); - return (pools); + return (0); } void diff --git a/module/zfs/zfs_ioctl.c b/module/zfs/zfs_ioctl.c index f91a2f3bbca5..2738385e260b 100644 --- a/module/zfs/zfs_ioctl.c +++ b/module/zfs/zfs_ioctl.c @@ -1582,8 +1582,9 @@ zfs_ioc_pool_configs(zfs_cmd_t *zc) nvlist_t *configs; int error; - if ((configs = spa_all_configs(&zc->zc_cookie)) == NULL) - return (SET_ERROR(EEXIST)); + error = spa_all_configs(&zc->zc_cookie, &configs); + if (error) + return (error); error = put_nvlist(zc, configs);