Messages in this thread | | | Date | Mon, 18 Nov 2019 17:28:38 +0100 | From | Oleg Nesterov <> | Subject | Re: [PATCH 4/5] locking/percpu-rwsem: Extract __percpu_down_read_trylock() |
| |
Hi Peter, sorry for delay.
I'll re-read this series tomorrow, but everything looks correct at first glance...
Except one very minor problem in this patch, see below.
On 11/13, Peter Zijlstra wrote: > > -bool __percpu_down_read(struct percpu_rw_semaphore *sem, bool try) > +static bool __percpu_down_read_trylock(struct percpu_rw_semaphore *sem) > { > __this_cpu_inc(*sem->read_count); > > @@ -70,14 +70,21 @@ bool __percpu_down_read(struct percpu_rw > * If !readers_block the critical section starts here, matched by the > * release in percpu_up_write(). > */ > - if (likely(!smp_load_acquire(&sem->readers_block))) > + if (likely(!atomic_read_acquire(&sem->readers_block)))
I don't think this can be compiled ;) ->readers_block is "int" until the next patch makes it atomic_t and renames to ->block.
And. I think __percpu_down_read_trylock() should do
if (atomic_read(&sem->block)) return false;
at the start, before __this_cpu_inc(read_count).
Suppose that the pending writer sleeps in rcuwait_wait_event(readers_active_check). If the new reader comes, it is better to not wake up that writer.
Oleg.
| |