From: Vitaliy Makkoveev <mvs@openbsd.org>
Subject: Re: tcp, sysctl: unlock TCPCTL_REASS_LIMIT and TCPCTL_SACKHOLE_LIMIT
To: Alexander Bluhm <bluhm@openbsd.org>
Cc: tech@openbsd.org
Date: Tue, 20 May 2025 13:22:14 +0000

On Tue, May 20, 2025 at 09:02:50PM +0900, Alexander Bluhm wrote:
> > > > +		if (error == 0 && oval != nval) {
> > > > +			extern struct rwlock sysctl_lock;
> > > > +
> > > > +			error = rw_enter(&sysctl_lock, RW_WRITE | RW_INTR);
> > > > +			if (error)
> > > > +				return (error);
> > > > +			if (nval != atomic_load_int(var)) {
> > > > +				error = pool_sethardlimit(pool, nval,
> > > > +				    NULL, 0);
> > > > +				if (error == 0)
> > > > +					atomic_store_int(var, nval);
> > > > +			}
> > > > +			rw_exit(&sysctl_lock);
> > > >  		}
> > 
> > I want to keep the `sysctl_lock' as is. I can't imagine something worse
> > than pool_sethardlimit() dry-run, but I prefer to keep this serialized
> > even with the pool_sethardlimit() serialized within. This is not the
> > fast path, the interruptible `sysctl_lock' doesn't make it worse, but we
> > 100% sure there is no race effects here.
> 
> I don't care much about this additional sysctl_lock.
> 

I suspect that this code running on fast core of hybrid CPU can be
executed between pool_sethardlimit() and atomic_store_int() executing
on slow core. So serialization is required.