Messages in this thread | | | Date | Mon, 13 Sep 2021 13:20:32 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH] locking: Remove rt_rwlock_is_contended() |
| |
On Fri, Sep 10, 2021 at 06:37:04PM +0200, Sebastian Andrzej Siewior wrote: > On 2021-09-10 18:16:14 [+0200], Peter Zijlstra wrote: > > On Tue, Sep 07, 2021 at 12:34:58PM +0200, Sebastian Andrzej Siewior wrote:
> Yes. I got arguments against it after sleeping :)
Sleep is magical :-)
> > AFAICT the _is_contended() can still use useful even with preemption, > > the typicla use case is a long lock-holder deciding to drop the lock in > > order to let someone else in. That still works with preemptible locks, > > no? > > Sure. We can do that. Then we should look into: > - fixing rwsem_is_contended() for the writer. The writer always observes > true even with no waiter around.
Right, that function does look somewhat dodgy. I'm thinking the current function returns true if there's more than a single reader present (or a writer) present, which is not the same.
I suppose it shoud return something like:
for a writer: rt_mutex_is_contended(&rwb->rtmutex); for a reader: rt_mutex_is_locked(&rwb->rtmutex);
However, given the below arguments,,,
> - checking the top waiter list vs priority of the lock owner/current. If > the current lock owner has the highest priority then the unlock+lock > is probably pointless as he regains the lock. > For the spin_lock() case, if the owner is SCHED_OTHER and the waiter > is SCHED_OTHER then unlock+lock will give the lock to the previous > owner due to rt_mutex_steal() working in his favour. Unless there is a > preemption.
That is a good argument against all this; I had not considered that.
> - reader checking for contention is probably pointless. It works with a > pending writer and one reader since a second reader will hold-off the > writer from acquiring the lock. Also if the reader does unlock+lock > then writer might not be quick enough.
Should be fixable with a handoff, but yeah.
OK, I suppose the safe and easy option is to never report contention as per your latest patch, and if/when someone complains about it, they can sort through these issues :-)
| |