Skip to content

Commit

Permalink
Annontate FreeBSD sysctls with CTLFLAG_MPSAFE
Browse files Browse the repository at this point in the history 
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 <[email protected]>
Reviewed-by: Ryan Moeller <[email protected]>
Signed-off-by: Mark Johnston <[email protected]>
Closes #10836
  • Loading branch information
@markjdb @behlendorf
markjdb authored and behlendorf committed Sep 21, 2020
1 parent c50f3c9 commit 6bdb095
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 26 deletions.
40 changes: 20 additions & 20 deletions module/os/freebsd/spl/spl_kstat.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -417,23 +417,23 @@ 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;
case KSTAT_TYPE_IO:
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:
Expand Down
17 changes: 11 additions & 6 deletions module/os/freebsd/zfs/sysctl_os.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)");

Expand Down Expand Up @@ -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 "
Expand Down

0 comments on commit 6bdb095

Please sign in to comment.