From a631283b745efc8354ad680b3a3c1f7bff38c4f8 Mon Sep 17 00:00:00 2001 From: Ryan Moeller Date: Tue, 16 Mar 2021 13:04:58 +0000 Subject: [PATCH] Move zfsdev_state_{init,destroy} to common code Reviewed-by: Brian Behlendorf Signed-off-by: Ryan Moeller Closes #11833 --- include/os/freebsd/zfs/sys/zfs_znode_impl.h | 2 - include/sys/zfs_ioctl.h | 1 - include/sys/zfs_ioctl_impl.h | 4 ++ module/os/freebsd/zfs/kmod_core.c | 60 +++-------------- module/os/linux/zfs/zfs_ioctl_os.c | 63 ++--------------- module/zfs/zfs_ioctl.c | 75 ++++++++++++++++++++- 6 files changed, 96 insertions(+), 109 deletions(-) diff --git a/include/os/freebsd/zfs/sys/zfs_znode_impl.h b/include/os/freebsd/zfs/sys/zfs_znode_impl.h index d7cdb360c..9a1d3ae26 100644 --- a/include/os/freebsd/zfs/sys/zfs_znode_impl.h +++ b/include/os/freebsd/zfs/sys/zfs_znode_impl.h @@ -77,8 +77,6 @@ typedef struct zfs_soft_state { void *zss_data; } zfs_soft_state_t; -extern minor_t zfsdev_minor_alloc(void); - /* * Range locking rules * -------------------- diff --git a/include/sys/zfs_ioctl.h b/include/sys/zfs_ioctl.h index 8834c5299..41c978a3f 100644 --- a/include/sys/zfs_ioctl.h +++ b/include/sys/zfs_ioctl.h @@ -567,7 +567,6 @@ typedef struct zfsdev_state { extern void *zfsdev_get_state(minor_t minor, enum zfsdev_state_type which); extern int zfsdev_getminor(int fd, minor_t *minorp); -extern minor_t zfsdev_minor_alloc(void); extern uint_t zfs_fsyncer_key; extern uint_t zfs_allow_log_key; diff --git a/include/sys/zfs_ioctl_impl.h b/include/sys/zfs_ioctl_impl.h index 787475cf3..23262a35d 100644 --- a/include/sys/zfs_ioctl_impl.h +++ b/include/sys/zfs_ioctl_impl.h @@ -91,6 +91,10 @@ void zfs_vfs_rele(zfsvfs_t *); long zfsdev_ioctl_common(uint_t, zfs_cmd_t *, int); int zfsdev_attach(void); void zfsdev_detach(void); +void zfsdev_private_set_state(void *, zfsdev_state_t *); +zfsdev_state_t *zfsdev_private_get_state(void *); +int zfsdev_state_init(void *); +void zfsdev_state_destroy(void *); int zfs_kmod_init(void); void zfs_kmod_fini(void); diff --git a/module/os/freebsd/zfs/kmod_core.c b/module/os/freebsd/zfs/kmod_core.c index a2a71cf0f..d9096575d 100644 --- a/module/os/freebsd/zfs/kmod_core.c +++ b/module/os/freebsd/zfs/kmod_core.c @@ -113,7 +113,6 @@ static int zfs__fini(void); static void zfs_shutdown(void *, int); static eventhandler_tag zfs_shutdown_event_tag; -extern zfsdev_state_t *zfsdev_state_list; #define ZFS_MIN_KSTACK_PAGES 4 @@ -182,66 +181,29 @@ out: static void zfsdev_close(void *data) { - zfsdev_state_t *zs = data; - - ASSERT(zs != NULL); - ASSERT3S(zs->zs_minor, >, 0); - - zfs_onexit_destroy(zs->zs_onexit); - zfs_zevent_destroy(zs->zs_zevent); - zs->zs_onexit = NULL; - zs->zs_zevent = NULL; - membar_producer(); - zs->zs_minor = -1; + zfsdev_state_destroy(data); } -static int -zfs_ctldev_init(struct cdev *devp) +void +zfsdev_private_set_state(void *priv __unused, zfsdev_state_t *zs) { - boolean_t newzs = B_FALSE; - minor_t minor; - zfsdev_state_t *zs, *zsprev = NULL; - - ASSERT(MUTEX_HELD(&zfsdev_state_lock)); - - minor = zfsdev_minor_alloc(); - if (minor == 0) - return (SET_ERROR(ENXIO)); - - for (zs = zfsdev_state_list; zs != NULL; zs = zs->zs_next) { - if (zs->zs_minor == -1) - break; - zsprev = zs; - } - - if (!zs) { - zs = kmem_zalloc(sizeof (zfsdev_state_t), KM_SLEEP); - newzs = B_TRUE; - } - devfs_set_cdevpriv(zs, zfsdev_close); +} - zfs_onexit_init((zfs_onexit_t **)&zs->zs_onexit); - zfs_zevent_init((zfs_zevent_t **)&zs->zs_zevent); - - if (newzs) { - zs->zs_minor = minor; - wmb(); - zsprev->zs_next = zs; - } else { - wmb(); - zs->zs_minor = minor; - } - return (0); +zfsdev_state_t * +zfsdev_private_get_state(void *priv) +{ + return (priv); } static int -zfsdev_open(struct cdev *devp, int flag, int mode, struct thread *td) +zfsdev_open(struct cdev *devp __unused, int flag __unused, int mode __unused, + struct thread *td __unused) { int error; mutex_enter(&zfsdev_state_lock); - error = zfs_ctldev_init(devp); + error = zfsdev_state_init(NULL); mutex_exit(&zfsdev_state_lock); return (error); diff --git a/module/os/linux/zfs/zfs_ioctl_os.c b/module/os/linux/zfs/zfs_ioctl_os.c index e9ae4c2a5..54a0cb02f 100644 --- a/module/os/linux/zfs/zfs_ioctl_os.c +++ b/module/os/linux/zfs/zfs_ioctl_os.c @@ -87,69 +87,20 @@ zfs_vfs_rele(zfsvfs_t *zfsvfs) deactivate_super(zfsvfs->z_sb); } -static int -zfsdev_state_init(struct file *filp) +void +zfsdev_private_set_state(void *priv, zfsdev_state_t *zs) { - zfsdev_state_t *zs, *zsprev = NULL; - minor_t minor; - boolean_t newzs = B_FALSE; - - ASSERT(MUTEX_HELD(&zfsdev_state_lock)); - - minor = zfsdev_minor_alloc(); - if (minor == 0) - return (SET_ERROR(ENXIO)); - - for (zs = zfsdev_state_list; zs != NULL; zs = zs->zs_next) { - if (zs->zs_minor == -1) - break; - zsprev = zs; - } - - if (!zs) { - zs = kmem_zalloc(sizeof (zfsdev_state_t), KM_SLEEP); - newzs = B_TRUE; - } + struct file *filp = priv; filp->private_data = zs; - - zfs_onexit_init((zfs_onexit_t **)&zs->zs_onexit); - zfs_zevent_init((zfs_zevent_t **)&zs->zs_zevent); - - /* - * In order to provide for lock-free concurrent read access - * to the minor list in zfsdev_get_state(), new entries - * must be completely written before linking them into the - * list whereas existing entries are already linked; the last - * operation must be updating zs_minor (from -1 to the new - * value). - */ - if (newzs) { - zs->zs_minor = minor; - smp_wmb(); - zsprev->zs_next = zs; - } else { - smp_wmb(); - zs->zs_minor = minor; - } - - return (0); } -static void -zfsdev_state_destroy(struct file *filp) +zfsdev_state_t * +zfsdev_private_get_state(void *priv) { - zfsdev_state_t *zs = filp->private_data; + struct file *filp = priv; - ASSERT(zs != NULL); - ASSERT3S(zs->zs_minor, >, 0); - - zfs_onexit_destroy(zs->zs_onexit); - zfs_zevent_destroy(zs->zs_zevent); - zs->zs_onexit = NULL; - zs->zs_zevent = NULL; - membar_producer(); - zs->zs_minor = -1; + return (filp->private_data); } static int diff --git a/module/zfs/zfs_ioctl.c b/module/zfs/zfs_ioctl.c index 53f16cd3d..df96378e2 100644 --- a/module/zfs/zfs_ioctl.c +++ b/module/zfs/zfs_ioctl.c @@ -7378,7 +7378,7 @@ zfsdev_get_state(minor_t minor, enum zfsdev_state_type which) * Find a free minor number. The zfsdev_state_list is expected to * be short since it is only a list of currently open file handles. */ -minor_t +static minor_t zfsdev_minor_alloc(void) { static minor_t last_minor = 0; @@ -7398,6 +7398,79 @@ zfsdev_minor_alloc(void) return (0); } +int +zfsdev_state_init(void *priv) +{ + zfsdev_state_t *zs, *zsprev = NULL; + minor_t minor; + boolean_t newzs = B_FALSE; + + ASSERT(MUTEX_HELD(&zfsdev_state_lock)); + + minor = zfsdev_minor_alloc(); + if (minor == 0) + return (SET_ERROR(ENXIO)); + + for (zs = zfsdev_state_list; zs != NULL; zs = zs->zs_next) { + if (zs->zs_minor == -1) + break; + zsprev = zs; + } + + if (!zs) { + zs = kmem_zalloc(sizeof (zfsdev_state_t), KM_SLEEP); + newzs = B_TRUE; + } + + zfsdev_private_set_state(priv, zs); + + zfs_onexit_init((zfs_onexit_t **)&zs->zs_onexit); + zfs_zevent_init((zfs_zevent_t **)&zs->zs_zevent); + + /* + * In order to provide for lock-free concurrent read access + * to the minor list in zfsdev_get_state(), new entries + * must be completely written before linking them into the + * list whereas existing entries are already linked; the last + * operation must be updating zs_minor (from -1 to the new + * value). + */ + if (newzs) { + zs->zs_minor = minor; + membar_producer(); + zsprev->zs_next = zs; + } else { + membar_producer(); + zs->zs_minor = minor; + } + + return (0); +} + +void +zfsdev_state_destroy(void *priv) +{ + zfsdev_state_t *zs = zfsdev_private_get_state(priv); + + ASSERT(zs != NULL); + ASSERT3S(zs->zs_minor, >, 0); + + /* + * The last reference to this zfsdev file descriptor is being dropped. + * We don't have to worry about lookup grabbing this state object, and + * zfsdev_state_init() will not try to reuse this object until it is + * invalidated by setting zs_minor to -1. Invalidation must be done + * last, with a memory barrier to ensure ordering. This lets us avoid + * taking the global zfsdev state lock around destruction. + */ + zfs_onexit_destroy(zs->zs_onexit); + zfs_zevent_destroy(zs->zs_zevent); + zs->zs_onexit = NULL; + zs->zs_zevent = NULL; + membar_producer(); + zs->zs_minor = -1; +} + long zfsdev_ioctl_common(uint_t vecnum, zfs_cmd_t *zc, int flag) {