From: David Laight <David.Laight@ACULAB.COM>
To: 'Waiman Long' <longman@redhat.com>, Davidlohr Bueso <dave@stgolabs.net>
Cc: Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>, Will Deacon <will@kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Phil Auld <pauld@redhat.com>
Subject: RE: [RFC PATCH 5/5] locking/rwsem: Remove reader optimistic spinning
Date: Fri, 20 Nov 2020 17:37:30 +0000 [thread overview]
Message-ID: <8e25d29ffee64b8ab377fd5ebf1c6bb2@AcuMS.aculab.com> (raw)
In-Reply-To: <68d07068-ff31-26b5-f90d-7ea8b4ee2389@redhat.com>
From: Waiman Long
> Sent: 20 November 2020 17:04
>
> On 11/20/20 8:11 AM, David Laight wrote:
> > From: Waiman Long
> >> Sent: 19 November 2020 18:40
> > ...
> >> My own testing also show not too much performance difference when
> >> removing reader spinning except in the lightly loaded cases.
> > I'm confused.
> >
> > I got massive performance improvements from changing a driver
> > we have to use mutex instead of the old semaphores (the driver
> > was written a long time ago).
> >
> > While these weren't 'rw' the same issue will apply.
> >
> > The problem was that the semaphore/mutex was typically only held over
> > a few instructions (eg to add an item to a list).
> > But with semaphore if you got contention the process always slept.
> > OTOH mutex spin 'for a while' before sleeping so the code rarely slept.
> >
> > So I really expect that readers need to spin (for a while) if
> > a rwsem (etc) is locked for writing.
> >
> > Clearly you really need a CBU (Crystal Ball Unit) to work out
> > whether to spin or not.
>
> That is the hard part. For short critical section and not many readers
> around, making the readers spin will likely improve performance. On the
> other hand, if the critical section is long with many readers, make
> readers sleep and then wake them all up at once can have better
> performance. There is no one-size-fit-all solution here.
Do the readers actually all wake up at the same time?
rwsem might be special, but if I cv_broadcast a userspace cv
then only one thread is woken.
Once it runs the next one is woken.
This is horrid if you actually want them all to run:
- It takes ages for the target cpu to come out of a low-power state.
- RT processes don't get scheduled if the cpu they last ran on is
'busy' in kernel.
I can't see why the number of readers is relevant.
They are more likely to start in 'lockstep' if they spin.
(Which I think is what you say is best).
You may want per-rwsem option of how long to spin.
Although there are probably only 2 useful values - 0 and lots.
Are there rw spinlocks?
They can be much better is the critical sections are short.
Especially if they really are short and RT kernels don't
break everything my making the sleep.
I was fixing some userspace code that does a lot of channels of
audio processing.
You can't afford to take a mutex because an interrupt might
come in while you hold it - stopping all the other threads
obtaining the same mutex.
One thread stopping is fine, but having all the threads stop
leads to processing overrun.
Since you can't disable interrupts in userspace (for a spinlock)
I had to replace locked linked lists with arrays indexed by
atomic counters.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
next prev parent reply other threads:[~2020-11-20 17:37 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-18 3:04 [PATCH 0/5] locking/rwsem: Rework reader optimistic spinning Waiman Long
2020-11-18 3:04 ` [PATCH 1/5] locking/rwsem: Pass the current atomic count to rwsem_down_read_slowpath() Waiman Long
2020-11-18 3:04 ` [PATCH 2/5] locking/rwsem: Prevent potential lock starvation Waiman Long
2020-11-20 14:44 ` Peter Zijlstra
2020-11-20 17:27 ` Waiman Long
2020-11-18 3:04 ` [PATCH 3/5] locking/rwsem: Enable reader optimistic lock stealing Waiman Long
2020-11-20 14:36 ` Peter Zijlstra
2020-11-20 17:26 ` Waiman Long
2020-12-08 3:53 ` Davidlohr Bueso
2020-11-18 3:04 ` [PATCH 4/5] locking/rwsem: Wake up all waiting readers if RWSEM_WAKE_READ_OWNED Waiman Long
2020-11-18 4:53 ` Davidlohr Bueso
2020-11-19 18:37 ` Waiman Long
2020-11-18 3:04 ` [RFC PATCH 5/5] locking/rwsem: Remove reader optimistic spinning Waiman Long
2020-11-18 5:35 ` Davidlohr Bueso
2020-11-19 18:40 ` Waiman Long
2020-11-20 13:11 ` David Laight
2020-11-20 17:04 ` Waiman Long
2020-11-20 17:37 ` David Laight [this message]
2020-11-20 21:38 ` Davidlohr Bueso
2020-11-21 11:50 ` David Laight
2020-11-20 14:44 ` Peter Zijlstra
2020-11-20 22:39 ` Waiman Long
2020-11-20 14:42 ` Peter Zijlstra
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8e25d29ffee64b8ab377fd5ebf1c6bb2@AcuMS.aculab.com \
--to=david.laight@aculab.com \
--cc=dave@stgolabs.net \
--cc=linux-kernel@vger.kernel.org \
--cc=longman@redhat.com \
--cc=mingo@redhat.com \
--cc=pauld@redhat.com \
--cc=peterz@infradead.org \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox