Messages in this thread | | | Date | Tue, 18 Oct 2022 13:37:20 -0400 | Subject | Re: [PATCH v3 4/5] locking/rwsem: Enable direct rwsem lock handoff | From | Waiman Long <> |
| |
On 10/18/22 10:13, Mukesh Ojha wrote: > Hi, > > On 10/18/2022 4:44 PM, Hillf Danton wrote: >> On 17 Oct 2022 17:13:55 -0400 Waiman Long <longman@redhat.com> >>> @@ -1067,13 +1119,33 @@ rwsem_down_read_slowpath(struct rw_semaphore >>> *sem, long count, unsigned int stat >>> return sem; >>> } >>> adjustment += RWSEM_FLAG_WAITERS; >>> + } else if ((count & RWSEM_FLAG_HANDOFF) && >>> + ((count & RWSEM_LOCK_MASK) == RWSEM_READER_BIAS)) { >> >> Could a couple of CPUs go read slow path in parallel? This is under wait_lock protection. So no parallel execution is possible. >> >>> + /* >>> + * If the waiter to be handed off is a reader, this reader >>> + * can piggyback on top of top of that. >>> + */ >>> + if (rwsem_first_waiter(sem)->type == RWSEM_WAITING_FOR_READ) >>> + adjustment = 0; >>> + rwsem_handoff(sem, adjustment, &wake_q); >>> + >>> + if (!adjustment) { >>> + raw_spin_unlock_irq(&sem->wait_lock); >>> + wake_up_q(&wake_q); >>> + return sem; >>> + } >>> + adjustment = 0; >>> } >>> rwsem_add_waiter(sem, &waiter); >> >> Why can this acquirer pigyback without becoming a waiter? The idea is to have as much reader parallelism as possible without writer starvation. In other word, a continuous stream of readers is not allowed to block out writer. However, there are places where allow one more reader to get the lock won't cause writer starvation. >> >>> - /* we're now waiting on the lock, but no longer actively >>> locking */ >>> - count = atomic_long_add_return(adjustment, &sem->count); >>> - >>> - rwsem_cond_wake_waiter(sem, count, &wake_q); >>> + if (adjustment) { >>> + /* >>> + * We are now waiting on the lock with no handoff, but no >>> + * longer actively locking. >>> + */ >>> + count = atomic_long_add_return(adjustment, &sem->count); >>> + rwsem_cond_wake_waiter(sem, count, &wake_q); >>> + } >>> raw_spin_unlock_irq(&sem->wait_lock); >>> if (!wake_q_empty(&wake_q)) >>> @@ -1120,7 +1192,6 @@ static struct rw_semaphore __sched * >>> rwsem_down_write_slowpath(struct rw_semaphore *sem, int state) >>> { >>> struct rwsem_waiter waiter; >>> - int null_owner_retries; >> >> This reverts 2/5 and feel free to drop it directly. > > I think, he intents to tag the first two patches to go to stable > branches.
This patch is too disruptive to go to the stable branches. Yes, I do intend the first 2 patches to go into stable branches.
Cheers, Longman
| |