From aba5b019cb15630f7eeda804d8492a3c4fb6a7fd Mon Sep 17 00:00:00 2001 From: Mark Johnston Date: Fri, 18 Sep 2020 08:45:54 -0400 Subject: [PATCH] Annontate FreeBSD sysctls with CTLFLAG_MPSAFE Without this, the sysctl system calls will acquire a global lock before invoking the handler. This is noticeable in some situations when running top(1). The global lock is mostly vestigal but continues to see some use and so contention is still a problem; until the default sense of the MPSAFE flag changes, we have to annotate each and every handler. Reviewed-by: Allan Jude Reviewed-by: Ryan Moeller Signed-off-by: Mark Johnston Closes #10836 --- module/os/freebsd/spl/spl_kstat.c | 40 +++++++++++++++---------------- module/os/freebsd/zfs/sysctl_os.c | 17 ++++++++----- 2 files changed, 31 insertions(+), 26 deletions(-) diff --git a/module/os/freebsd/spl/spl_kstat.c b/module/os/freebsd/spl/spl_kstat.c index 655fe5a626..f215751420 100644 --- a/module/os/freebsd/spl/spl_kstat.c +++ b/module/os/freebsd/spl/spl_kstat.c @@ -349,50 +349,50 @@ kstat_install_named(kstat_t *ksp) SYSCTL_ADD_PROC(&ksp->ks_sysctl_ctx, SYSCTL_CHILDREN(ksp->ks_sysctl_root), OID_AUTO, namelast, - CTLTYPE_S32 | CTLFLAG_RD, ksp, i, - kstat_sysctl, "I", namelast); + CTLTYPE_S32 | CTLFLAG_RD | CTLFLAG_MPSAFE, + ksp, i, kstat_sysctl, "I", namelast); break; case KSTAT_DATA_UINT32: SYSCTL_ADD_PROC(&ksp->ks_sysctl_ctx, SYSCTL_CHILDREN(ksp->ks_sysctl_root), OID_AUTO, namelast, - CTLTYPE_U32 | CTLFLAG_RD, ksp, i, - kstat_sysctl, "IU", namelast); + CTLTYPE_U32 | CTLFLAG_RD | CTLFLAG_MPSAFE, + ksp, i, kstat_sysctl, "IU", namelast); break; case KSTAT_DATA_INT64: SYSCTL_ADD_PROC(&ksp->ks_sysctl_ctx, SYSCTL_CHILDREN(ksp->ks_sysctl_root), OID_AUTO, namelast, - CTLTYPE_S64 | CTLFLAG_RD, ksp, i, - kstat_sysctl, "Q", namelast); + CTLTYPE_S64 | CTLFLAG_RD | CTLFLAG_MPSAFE, + ksp, i, kstat_sysctl, "Q", namelast); break; case KSTAT_DATA_UINT64: SYSCTL_ADD_PROC(&ksp->ks_sysctl_ctx, SYSCTL_CHILDREN(ksp->ks_sysctl_root), OID_AUTO, namelast, - CTLTYPE_U64 | CTLFLAG_RD, ksp, i, - kstat_sysctl, "QU", namelast); + CTLTYPE_U64 | CTLFLAG_RD | CTLFLAG_MPSAFE, + ksp, i, kstat_sysctl, "QU", namelast); break; case KSTAT_DATA_LONG: SYSCTL_ADD_PROC(&ksp->ks_sysctl_ctx, SYSCTL_CHILDREN(ksp->ks_sysctl_root), OID_AUTO, namelast, - CTLTYPE_LONG | CTLFLAG_RD, ksp, i, - kstat_sysctl, "L", namelast); + CTLTYPE_LONG | CTLFLAG_RD | CTLFLAG_MPSAFE, + ksp, i, kstat_sysctl, "L", namelast); break; case KSTAT_DATA_ULONG: SYSCTL_ADD_PROC(&ksp->ks_sysctl_ctx, SYSCTL_CHILDREN(ksp->ks_sysctl_root), OID_AUTO, namelast, - CTLTYPE_ULONG | CTLFLAG_RD, ksp, i, - kstat_sysctl, "LU", namelast); + CTLTYPE_ULONG | CTLFLAG_RD | CTLFLAG_MPSAFE, + ksp, i, kstat_sysctl, "LU", namelast); break; case KSTAT_DATA_STRING: SYSCTL_ADD_PROC(&ksp->ks_sysctl_ctx, SYSCTL_CHILDREN(ksp->ks_sysctl_root), OID_AUTO, namelast, - CTLTYPE_STRING | CTLFLAG_RD, ksp, i, - kstat_sysctl_string, "A", namelast); + CTLTYPE_STRING | CTLFLAG_RD | CTLFLAG_MPSAFE, + ksp, i, kstat_sysctl_string, "A", namelast); break; default: panic("unsupported type: %d", typelast); @@ -417,14 +417,14 @@ kstat_install(kstat_t *ksp) root = SYSCTL_ADD_PROC(&ksp->ks_sysctl_ctx, SYSCTL_CHILDREN(ksp->ks_sysctl_root), OID_AUTO, ksp->ks_name, - CTLTYPE_STRING | CTLFLAG_RD, ksp, 0, - kstat_sysctl_raw, "A", ksp->ks_name); + CTLTYPE_STRING | CTLFLAG_RD | CTLFLAG_MPSAFE, + ksp, 0, kstat_sysctl_raw, "A", ksp->ks_name); } else { root = SYSCTL_ADD_PROC(&ksp->ks_sysctl_ctx, SYSCTL_CHILDREN(ksp->ks_sysctl_root), OID_AUTO, ksp->ks_name, - CTLTYPE_OPAQUE | CTLFLAG_RD, ksp, 0, - kstat_sysctl_raw, "", ksp->ks_name); + CTLTYPE_OPAQUE | CTLFLAG_RD | CTLFLAG_MPSAFE, + ksp, 0, kstat_sysctl_raw, "", ksp->ks_name); } VERIFY(root != NULL); break; @@ -432,8 +432,8 @@ kstat_install(kstat_t *ksp) root = SYSCTL_ADD_PROC(&ksp->ks_sysctl_ctx, SYSCTL_CHILDREN(ksp->ks_sysctl_root), OID_AUTO, ksp->ks_name, - CTLTYPE_STRING | CTLFLAG_RD, ksp, 0, - kstat_sysctl_io, "A", ksp->ks_name); + CTLTYPE_STRING | CTLFLAG_RD | CTLFLAG_MPSAFE, + ksp, 0, kstat_sysctl_io, "A", ksp->ks_name); break; case KSTAT_TYPE_TIMER: case KSTAT_TYPE_INTR: diff --git a/module/os/freebsd/zfs/sysctl_os.c b/module/os/freebsd/zfs/sysctl_os.c index b3cb7e7e43..c9b350a540 100644 --- a/module/os/freebsd/zfs/sysctl_os.c +++ b/module/os/freebsd/zfs/sysctl_os.c @@ -243,8 +243,9 @@ sysctl_vfs_zfs_arc_no_grow_shift(SYSCTL_HANDLER_ARGS) return (0); } -SYSCTL_PROC(_vfs_zfs, OID_AUTO, arc_no_grow_shift, CTLTYPE_U32 | CTLFLAG_RWTUN, - 0, sizeof (uint32_t), sysctl_vfs_zfs_arc_no_grow_shift, "U", +SYSCTL_PROC(_vfs_zfs, OID_AUTO, arc_no_grow_shift, + CTLTYPE_U32 | CTLFLAG_RWTUN | CTLFLAG_MPSAFE, 0, sizeof (uint32_t), + sysctl_vfs_zfs_arc_no_grow_shift, "U", "log2(fraction of ARC which must be free to allow growing)"); int @@ -275,10 +276,12 @@ param_set_arc_int(SYSCTL_HANDLER_ARGS) return (0); } -SYSCTL_PROC(_vfs_zfs, OID_AUTO, arc_min, CTLTYPE_ULONG | CTLFLAG_RWTUN, +SYSCTL_PROC(_vfs_zfs, OID_AUTO, arc_min, + CTLTYPE_ULONG | CTLFLAG_RWTUN | CTLFLAG_MPSAFE, &zfs_arc_min, sizeof (zfs_arc_min), param_set_arc_long, "LU", "min arc size (LEGACY)"); -SYSCTL_PROC(_vfs_zfs, OID_AUTO, arc_max, CTLTYPE_ULONG | CTLFLAG_RWTUN, +SYSCTL_PROC(_vfs_zfs, OID_AUTO, arc_max, + CTLTYPE_ULONG | CTLFLAG_RWTUN | CTLFLAG_MPSAFE, &zfs_arc_max, sizeof (zfs_arc_max), param_set_arc_long, "LU", "max arc size (LEGACY)"); @@ -558,11 +561,13 @@ param_set_max_auto_ashift(SYSCTL_HANDLER_ARGS) return (0); } -SYSCTL_PROC(_vfs_zfs, OID_AUTO, min_auto_ashift, CTLTYPE_U64 | CTLFLAG_RWTUN, +SYSCTL_PROC(_vfs_zfs, OID_AUTO, min_auto_ashift, + CTLTYPE_U64 | CTLFLAG_RWTUN | CTLFLAG_MPSAFE, &zfs_vdev_min_auto_ashift, sizeof (zfs_vdev_min_auto_ashift), param_set_min_auto_ashift, "QU", "Min ashift used when creating new top-level vdev. (LEGACY)"); -SYSCTL_PROC(_vfs_zfs, OID_AUTO, max_auto_ashift, CTLTYPE_U64 | CTLFLAG_RWTUN, +SYSCTL_PROC(_vfs_zfs, OID_AUTO, max_auto_ashift, + CTLTYPE_U64 | CTLFLAG_RWTUN | CTLFLAG_MPSAFE, &zfs_vdev_max_auto_ashift, sizeof (zfs_vdev_max_auto_ashift), param_set_max_auto_ashift, "QU", "Max ashift used when optimizing for logical -> physical sector size on "