On Wed, Sep 13, 2017 at 10:33:01AM +0200, Martin Pieuchot wrote:
> diff -u -p -r1.512 if.c
> --- net/if.c 22 Aug 2017 15:02:34 -0000 1.512
> +++ net/if.c 13 Sep 2017 08:28:27 -0000
> @@ -2666,10 +2666,14 @@ ifpromisc(struct ifnet *ifp, int pswitch
> return ((*ifp->if_ioctl)(ifp, SIOCSIFFLAGS, (caddr_t)&ifr));
> }
>
> +/* XXX move to kern/uipc_mbuf.c */
> 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;
> + int error;
> +
> /* All sysctl names at this level are terminal. */
> if (namelen != 1)
> return (ENOTDIR);
> @@ -2678,8 +2682,14 @@ sysctl_mq(int *name, u_int namelen, void
> case IFQCTL_LEN:
> return (sysctl_rdint(oldp, oldlenp, newp, mq_len(mq)));
> case IFQCTL_MAXLEN:
> - return (sysctl_int(oldp, oldlenp, newp, newlen,
> - &mq->mq_maxlen)); /* XXX directly accessing maxlen */
> + maxlen = mq->mq_maxlen;
> + error = sysctl_int(oldp, oldlenp, newp, newlen, &maxlen);
> + if (!error && maxlen != mq->mq_maxlen) {
> + mtx_enter(&mq->mq_mtx);
> + mq->mq_maxlen = maxlen;
> + mtx_leave(&mq->mq_mtx);
> + }
> + return (error);
> case IFQCTL_DROPS:
> return (sysctl_rdint(oldp, oldlenp, newp, mq_drops(mq)));
> default:
Is it safe to access int values that are used by more than one CPU
without lock or memory barrier?
- is it safe for all architectures?
- is it safe for reading?
- is it safe for writing?
mq_len(), mq_drops() and mq_set_maxlen() are provided as public
interfaces without lock. This implies that it should be safe.
So you should use mq_set_maxlen() instead of using the mutex here.
If using this function without lock is wrong, the defines in mbuf.h
should be converted to functions with proper locking.
Checking "maxlen != mq->mq_maxlen" without lock and assigning
"mq->mq_maxlen = maxlen" with a mutex just looks wrong. Reading
the old "maxlen = mq->mq_maxlen" without guarantee this is the value
that will be changed by the current CPU feels strange.
You can commit it anyway, it is not worse than before.
OK bluhm@
