On Tue, May 16, 2023 at 01:55:32PM +0300, Vitaliy Makkoveev wrote:
> Let's start to unlock (*pr_sysctl)() handlers. We have many of them, so
> introduce temporary PR_MPSAFE flag to mark MP safe instead of pushing
> kernel lock within handlers.
I had the same idea and flag name for pr_input functions in a pending
diff. Could you name your flag PR_MPSYSCTL? Then it is clear what
it is used for.
> Unlock ip_sysctl(). Still take kernel lock within IPCTL_MRTSTATS case.
> ok?
OK bluhm@
> This is not related to (*pr_sysctl)() unlocking, but if there is no
> objections, I want to replace tabs by space after #define in PR_* flags
> definitions.
Yes please. I also prefer #defineSPACE over #defineTAB as diff
formating does not jump around. Could you also use 0x0080 when
doing this. Flags is a short and we could need more flags for fine
grained unlocking.
I saw one issue in sysctl_niq(). Another CPU could write mq_maxlen
and our logic is inconsistent. Below is a fix with read once. Each
CPU detects its own change, last write wins. Or should we protect
everything with mq_mtx? Then sysctl 123 -> 345 would have the
correct old value on both CPUs.
bluhm
Index: kern/uipc_mbuf.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/uipc_mbuf.c,v
retrieving revision 1.285
diff -u -p -r1.285 uipc_mbuf.c
--- kern/uipc_mbuf.c 5 May 2023 01:19:51 -0000 1.285
+++ kern/uipc_mbuf.c 16 May 2023 15:05:52 -0000
@@ -1788,7 +1788,7 @@ int
sysctl_mq(int *name, u_int namelen, void *oldp, size_t *oldlenp,
void *newp, size_t newlen, struct mbuf_queue *mq)
{
- unsigned int maxlen;
+ unsigned int oldmax, newmax;
int error;
/* All sysctl names at this level are terminal. */
@@ -1799,10 +1799,10 @@ sysctl_mq(int *name, u_int namelen, void
case IFQCTL_LEN:
return (sysctl_rdint(oldp, oldlenp, newp, mq_len(mq)));
case IFQCTL_MAXLEN:
- maxlen = mq->mq_maxlen;
- error = sysctl_int(oldp, oldlenp, newp, newlen, &maxlen);
- if (!error && maxlen != mq->mq_maxlen)
- mq_set_maxlen(mq, maxlen);
+ oldmax = newmax = READ_ONCE(mq->mq_maxlen);
+ error = sysctl_int(oldp, oldlenp, newp, newlen, &newmax);
+ if (!error && oldmax != newmax)
+ mq_set_maxlen(mq, newmax);
return (error);
case IFQCTL_DROPS:
return (sysctl_rdint(oldp, oldlenp, newp, mq_drops(mq)));
