On Tue, May 20, 2025 at 01:22:14PM +0000, Vitaliy Makkoveev wrote:
> 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.

Yes, there is the race.  Thanks for explaining.