tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: New cpu_count races; seqlocks?



A bit off on a tangent, but my intent was to keep this stuff very simple and
distinct from the general (evcnt) type case because these are special to my
mind due to their well established use and how clustered they are.

On Tue, Dec 17, 2019 at 05:07:12AM +0000, Taylor R Campbell wrote:

> It is great to get rid of global contention over a bunch of counters
> that don't need to be read exactly.  However, the new cpu_count logic
> is racy -- particularly on 32-bit platforms, where it's not simply a
> matter of reading stale values but a matter of reading total garbage:

I'm not adverse to solving the problem at all but my thinking is that in
most cases it doesn't really matter too much since they're sampled with some
regularity, which is how we've been running without a stable/reliable view
of these numbers for 10+ years now.  However having thought it over again
perhaps we've grown a new problem that we simply don't need.

On a 32-bit platform, I don't see why any of these actually need to be
64-bit.  They've been wrappable since we've had them, they're still
wrappable in 64-bit form, and I'd be quite happy to say that if you're
running 32-bit then you get 32-bit wraparound.  Making them uintptr_t's
internally in the implementation at least would mean we don't introduce
any new problems for the user nor ourselves.

> 	/* Claim exclusive access for modifications.  */
> 	do {
> 		while (1 & (gen = atomic_load_relaxed(&cpu_count_gen)))
> 			SPINLOCK_BACKOFF_HOOK;
> 	} while (atomic_cas_32(&cpu_count_gen, gen, gen | 1) != gen);

I actually quite like this from the POV of making sure no more than one LWP
at a time tries to update the sum totals since the cost is huge.

> Perhaps we should have a seqlock(9) API for broader use.  I drafted an
> implementation of Linux's seqlock API for a drm import from Linux
> 4.19:
> 
> https://github.com/riastradh/netbsd-src/blob/riastradh-drm419/sys/external/bsd/drm2/include/linux/seqlock.h

I think we are very likely to run into a need for such a thing for the file
system and network code.

Andrew


Home | Main Index | Thread Index | Old Index