Messages in this thread | | | Date | Thu, 11 Nov 2021 16:54:16 -0500 | Subject | Re: [BUG]locking/rwsem: only clean RWSEM_FLAG_HANDOFF when already set | From | Waiman Long <> |
| |
On 11/11/21 16:27, Peter Zijlstra wrote: > On Thu, Nov 11, 2021 at 03:45:30PM -0500, Waiman Long wrote: >> On 11/11/21 15:35, Peter Zijlstra wrote: >>> On Thu, Nov 11, 2021 at 02:36:52PM -0500, Waiman Long wrote: >>>> On 11/11/21 14:20, Peter Zijlstra wrote: >>>>> On Thu, Nov 11, 2021 at 02:14:48PM -0500, Waiman Long wrote: >>>>>> As for the PHASE_CHANGE name, we have to be consistent in both rwsem and >>>>>> mutex. Maybe a follow up patch if you think we should change the >>>>>> terminology. >>>>> Well, that's exactly the point, they do radically different things. >>>>> Having the same name for two different things is confusing. >>>>> >>>>> Anyway, let me go read that patch you sent. >>>> My understanding of handoff is to disable optimistic spinning to let waiters >>>> in the wait queue have an opportunity to acquire the lock. There are >>>> difference in details on how to do that in mutex and rwsem, though. >>> Ah, but the mutex does an actual hand-off, it hands the lock to a >>> specific waiting task. That is, unlock() sets owner, as opposed to >>> trylock(). >>> >>> The rwsem code doesn't, it just forces a phase change. Once a waiter has >>> been blocked too long, the handoff bit is set, causing new readers to be >>> blocked. Then we wait for existing readers to complete. At that point, >>> any next waiter (most likely a writer) should really get the lock (and >>> in that regards the rwsem code is a bit funny). >>> >>> So while both ensure fairness, the means of doing so is quite different. >>> One hands the lock ownership to a specific waiter, the other arranges >>> for a quiescent state such that the next waiter can proceed. >> That is a valid argument. However, the name PHASE_CHANGE sounds weird to me. >> I am not objecting to changing the term, but probably with a better name >> NO_OPTSPIN, NO_LOCKSTEALING or something like that to emphasize that fact >> that optimistic spinning or lock stealing should not be allowed. > RWSEM_FLAG_QUIESCE ?
I think that is a more acceptable term than PHASE_CHANGE. Will have a follow up patch later on. This one is more urgent and I want to get it done first.
Cheers, Longman.
| |