On Tue, Nov 17, 2020 at 09:35:56PM -0800, Davidlohr Bueso wrote:Good to know. I am going to sent out v2 with some update commit logs and some !CONFIG_RWSEM_SPIN_ON_OWNER fixes.
On Tue, 17 Nov 2020, Waiman Long wrote:I'm fine with ripping it... It was finnicky to begin with.
The column "CS Load" represents the number of pause instructions issuedI'm not overly worried about the lightly loaded cases here as the users
in the locking critical section. A CS load of 1 is extremely short and
is not likey in real situations. A load of 20 (moderate) and 100 (long)
are more realistic.
It can be seen that the previous patches in this series have reduced
performance in general except in highly contended cases with moderate
or long critical sections that performance improves a bit. This change
is mostly caused by the "Prevent potential lock starvation" patch that
reduce reader optimistic spinning and hence reduce reader fragmentation.
The patch that further limit reader optimistic spinning doesn't seem to
have too much impact on overall performance as shown in the benchmark
data.
The patch that disables reader optimistic spinning shows reduced
performance at lightly loaded cases, but comparable or slightly better
performance on with heavier contention.
(mostly thinking mmap_sem) most likely won't care for real workloads,
not, ie: will-it-scale type things.
So at SUSE we also ran into this very same problem with reader optimistic
spinning and considering the fragmentation went with disabling it, much
like this patch - but without the reader optimistic lock stealing bits
you have. So far nothing has really shown to fall out in our performance
automation. And per your data a single reader spinner does not seem to be
worth the added complexity of keeping reader spinning vs ripping it out.