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).

Reviewed-by: Tony Hutter <hutter2@llnl.gov>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Thomas Bertschinger <bertschinger@lanl.gov>
Closes #15360

diff --git a/include/os/freebsd/spl/sys/mutex.h b/include/os/freebsd/spl/sys/mutex.h
index e757d12c15020b23eadc903c30086276b9fe2e34..8cfe56c753090f6b448c47cdf30991833309ffb2 100644 (file)
--- 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 6b61c59c48e202bf3db4931cdfa4a2d7badac3fd..b4eaa0266d20c0514887e775325261ba3588e5d6 100644 (file)
--- 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 18062d3f2a95c25d3c2a252da3ddd314c29d2e31..88ef510b744b43b99b5a907b84ee77dd62937006 100644 (file)
--- 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 6a337b49edf3fef31d086a08fde881a63fa3bd79..750ca612b962c0653851322e56c91f89bda2ccf5 100644 (file)
--- 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 a9b9bf4c2ce5474162c1ec61376591e013ad70af..ffad7fc02bc98c0eef666d1ea735feb50afb58dc 100644 (file)
--- 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 636c04d9f7857848f9ac71075107904e767f4d9a..a77874ea0dd339f7d0c14cf52bfae521d5cb3af6 100644 (file)
--- 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 f91a2f3bbca55a6b7597a5b3333e8a5d4e43db03..2738385e260b16df9a425a907bed355f866c7257 100644 (file)
--- 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);