Re: [PATCH 1/1] kernel/locking: Add mutual exclusion self-test

[Haakon Bugge]

On 14 Sep 2026, at 11:35, David Laight <david.laight.linux@xxxxxxxxx> wrote:
> 
> On Mon, 17 Aug 2026 15:02:36 +0200
> Håkon Bugge <haakon.bugge@xxxxxxxxxx> wrote:
> 
> > Add a stress test for locking and atomic synchronization primitives.
> > 
> > The test maintains an array of elements containing counters
> > initialized to zero. It creates four worker threads per online CPU. On
> > each iteration, a worker chooses two elements at random, increments
> > the first counter, and decrements the second. Each completed pair of
> > updates therefore preserves the sum of all counters.
> ...
> 
> I think 1000 elements/locks/counters is far too many.

I do not have a strong opinion. My experience is that it doesn't
matter much. For example, modifying the "atomic_xchg" test to:

Lock acquire:
	while (atomic_xchg_acquire(&el->mx_atomic_xchg.lock, 1) != 0)
		cpu_relax();

Lock release:
	atomic_set(&el->mx_atomic_xchg.lock, 0);

(that is, removing "_release" above).

This fails within one second of runtime for all tested number of
elements:

2
32
512
8192
131072
2097152
33554432

on a 160 CPU Arm BM system. What _is_ paramount, is the padding. None
of the failing tests fails if padding is zero. So, in other words, on
this particular micro architecture, there must be cases where the lock
and the counter resides in different cache-lines. Trust me, I got
severely confused after Matthew Wilcox' commit 25500ba7e77c
("locking/mutex: Remove the list_head from struct mutex"). Suddenly,
none of my known failing cases failed anymore...                                                                                                                                                                                                                                                                                                               

> You are trying to hit contention so you want a relatively small number.
> Possibly similar to the number of threads running the test, but maybe less.
> Perhaps nearer the number of cpus.
> 
> Also you are only going to see an issue if the RMW of the 'counter += 1'
> is split by the same RMW of a different thread.
> That is pretty unlikely even without a lock.

Not in my opinion. I have the "mx_busted" test which does exactly
this, and it fails in all cases. It is actually used by the
selftest script to verify that the test harness is able to detect
errors.

Also, the added selftest script permutates many of the parameters.

> It would be better to force a short delay between the R and W.

That may be a good idea. But unless someone can come up with a case
where it is needed, I am alluding to not put in any delays.

> If you use change the MX_ATOMIC_ADD to use the atomic_long functions
> (I've forgotten the exact name) then all the counter are the same type
> and can be removed from the union.
> The default 'just use +=' code can then be moved to the bottom of mx_add().

That is a good idea, but we then misses test coverage for atomic_t. But,
what about:

struct mx_elem {
        union {
                spinlock_t spin_lock;
		:
                struct mutex mutex;
        };
        union {
                long counter;
                atomic_t atom_counter;
                atomic64_t atom64_counter;
        };
};

Then all lock-full cases will have a single " += " code.


Thxs, Håkon

>