svn commit: r236380 - head/sys/vm

[Bruce Evans]

On Fri, 1 Jun 2012 mdf at FreeBSD.org wrote:

> On Fri, Jun 1, 2012 at 2:14 AM, Bruce Evans <brde at optusnet.com.au> wrote:
>>>> +SYSCTL_OID(_vm, OID_AUTO, swap_free,
>>>> CTLTYPE_INT|CTLFLAG_RD|CTLFLAG_MPSAFE,
>>>> +               NULL, 0, sysctl_vm_swap_free, "Q",
>>>> +               "Blocks of free swap storage.");
>>
>> Bug 9 is a style bug.  I didn't even know that the raw SYSCTL_OID() could
>> be misused like this.  The normal SYSCTL_PROC() is identical with
>> SYSCTL_OID() except it checks that the access flags are not 0.  Few or no
>> SYSCTL_FOO()s have no access flags, and this is not one.  It has rather
>> excessive access flags (I think CTLFLAG_MPSAFE is unnecessary.
>
> I wanted to correct this one point.  CTLFLAG_MPSAFE is helpful,
> because its use prevents kern_sysctl from taking Giant before calling
> the sysctl handler.  It's probably nearing the case, now, that any
> sysctl *without* CTLFLAG_MPSAFE is incorrect, except perhaps for a few
> that set/get text strings that don't want to roll their own
> serialization.

The magic is that SYSCTL_FOO() adds CTFLAG_MPSAFE for most or all
simple integer SYSCTL_FOO()s like SYSCTL_INT(), but it doesn't do this
for any non-integer SYSCTL_FOO().  Not for SYSCTL_PROC(), and especially
not for the raw SYSCTL_OID().  There must be a lot of SYSCTL_PROC()s
that don't bother with this, although many return an integer after
calculating it.  Perhaps the calculation isn't properly locked, but
Giant will rarely help and no locking helps much for read-only sysctls
of dynamic data (the value may change before it is returned).  In kern,
there are 113 lines matching SYSCTL_PROC, and only 4 of these match
CTLFLAG_MPSAFE.

Bruce