Messages in this thread | | | Date | Mon, 16 Sep 2013 17:29:11 -0700 | From | David Daney <> | Subject | Re: [PATCH] rwsem: add rwsem_is_contended |
| |
On 09/16/2013 05:05 PM, Josef Bacik wrote: > On Mon, Sep 16, 2013 at 04:05:47PM -0700, Andrew Morton 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. >> > > So the problem is not that its normal lock starvation, it's more our particular > use case that is causing the starvation. We can have lots of people holding > readers and simply never give them up for long periods of time, which is why we > need this is_contended helper so we know to drop things and let the committer > through. Thanks,
You could easily achieve the same thing by putting an "is_contending" flag in parallel with the rwsem and testing that:
DECLARE_RWSEM(foo); atomic_t is_contended = ATOMIC_INIT(0); . . .
/* writing context */ atomic_inc(&is_contended); down_write(&foo); do_writing_action(); up_write(&foo); atomic_dec(&is_contended);
/* reading context */ down_read(&foo); while (!atomic_read(&is_contended)) do_reading_actions(); up_read(&foo);
David Daney
| |