From 6bdb09510bd27038ee85c372035b27120a4f46c6 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 655fe5a62..f21575142 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 b3cb7e7e4..c9b350a54 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 "