Messages in this thread | | | Date | Mon, 5 Dec 2016 18:13:53 +0100 | From | Oleg Nesterov <> | Subject | Re: [PATCH v2 2/3] locking/percpu-rwsem: Rework writer block/wake to not use wait-queues |
| |
On 12/02, Davidlohr Bueso wrote: > > @@ -102,8 +103,13 @@ void __percpu_up_read(struct percpu_rw_semaphore *sem) > */ > __this_cpu_dec(*sem->read_count); > > + rcu_read_lock(); > + writer = rcu_dereference(sem->writer); > + > /* Prod writer to recheck readers_active */ > - wake_up(&sem->writer); > + if (writer) > + wake_up_process(writer); > + rcu_read_unlock();
This needs a barrier between __this_cpu_dec() and rcu_dereference(), I think.
> @@ -159,8 +165,18 @@ void percpu_down_write(struct percpu_rw_semaphore *sem) > * will wait for them. > */ > > - /* Wait for all now active readers to complete. */ > - wait_event(sem->writer, readers_active_check(sem)); > + WRITE_ONCE(sem->writer, current); > + for (;;) { > + set_current_state(TASK_UNINTERRUPTIBLE); > + > + if (readers_active_check(sem)) > + break;
This looks fine, we can rely on set_current_state() which inserts a barrier between WRITE_ONCE() and readers_active_check(). So we do not even need WRITE_ONCE().
And the fact this needs the barriers and the comments makes me think again you should add the new helpers.
Oleg.
| |