Messages in this thread | | | Subject | Re: [PATCH 5/5] locking/percpu-rwsem: Remove the embedded rwsem | From | Waiman Long <> | Date | Tue, 19 Nov 2019 08:50:11 -0500 |
| |
On 11/13/19 5:21 AM, Peter Zijlstra wrote: > +static int percpu_rwsem_wake_function(struct wait_queue_entry *wq_entry, > + unsigned int mode, int wake_flags, > + void *key) > +{ > + struct task_struct *p = get_task_struct(wq_entry->private); > + bool reader = wq_entry->flags & WQ_FLAG_CUSTOM; > + struct percpu_rw_semaphore *sem = key; > + > + /* concurrent against percpu_down_write(), can get stolen */ > + if (!__percpu_rwsem_trylock(sem, reader)) > + return 1; > + > + list_del_init(&wq_entry->entry); > + smp_store_release(&wq_entry->private, NULL); > + > + wake_up_process(p); > + put_task_struct(p); > + > + return !reader; /* wake 'all' readers and 1 writer */ > +} > + > +static void percpu_rwsem_wait(struct percpu_rw_semaphore *sem, bool reader) > +{ > + DEFINE_WAIT_FUNC(wq_entry, percpu_rwsem_wake_function); > + bool wait; > + > + spin_lock_irq(&sem->waiters.lock); > + /* > + * Serialize against the wakeup in percpu_up_write(), if we fail > + * the trylock, the wakeup must see us on the list. > + */ > + wait = !__percpu_rwsem_trylock(sem, reader); > + if (wait) { > + wq_entry.flags |= WQ_FLAG_EXCLUSIVE | reader * WQ_FLAG_CUSTOM; > + __add_wait_queue_entry_tail(&sem->waiters, &wq_entry); > + } > + spin_unlock_irq(&sem->waiters.lock); > + > + while (wait) { > + set_current_state(TASK_UNINTERRUPTIBLE); > + if (!smp_load_acquire(&wq_entry.private)) > + break; > + schedule(); > + }
If I read the function correctly, you are setting the WQ_FLAG_EXCLUSIVE for both readers and writers and __wake_up() is called with an exclusive count of one. So only one reader or writer is woken up each time. However, the comment above said we wake 'all' readers and 1 writer. That doesn't match the actual code, IMO. To match the comments, you should have set WQ_FLAG_EXCLUSIVE flag only on writer. In this case, you probably don't need WQ_FLAG_CUSTOM to differentiate between readers and writers.
Cheers, Longman
| |