Messages in this thread | | | Date | Tue, 17 Sep 2013 08:53:24 +0200 | From | Ingo Molnar <> | Subject | Re: [PATCH] rwsem: add rwsem_is_contended |
| |
* Andrew Morton <akpm@linux-foundation.org> wrote:
> On Fri, 30 Aug 2013 10:14:01 -0400 Josef Bacik <jbacik@fusionio.com> wrote: > > > Btrfs uses an rwsem to control access to its extent tree. Threads > > will hold a read lock on this rwsem while they scan the extent tree, > > and if need_resched() they will drop the lock and schedule. The > > transaction commit needs to take a write lock for this rwsem for a > > very short period to switch out the commit roots. If there are a lot > > of threads doing this caching operation we can starve out the > > committers which slows everybody out. To address this we want to add > > this functionality to see if our rwsem has anybody waiting to take a > > write lock so we can drop it and schedule for a bit to allow the > > commit to continue. Thanks, > > This sounds rather nasty and hacky. Rather then working around a > locking shortcoming in a caller it would be better to fix/enhance the > core locking code. What would such a change need to do? > > Presently rwsem waiters are fifo-queued, are they not? So the commit > thread will eventually get that lock. Apparently that's not working > adequately for you but I don't fully understand what it is about these > dynamics which is causing observable problems.
It would be nice to see the whole solution, together with the btrfs patch.
The problem I have is that this new primitive is only superficially like spin_is_contended(): in the spinlock case dropping the lock will guarantee some sort of progress, because another CPU will almost certainly pick up the lock if we cpu_relax().
In the rwsem case there's no such guarantee of progress, especially if a read-lock is dropped. So I'd like to see how it's implemented in practice.
Thanks,
Ingo
| |