Re: [PATCH 4/5] locking/percpu-rwsem: Extract __percpu_down_read_trylock()
From: Oleg Nesterov
Date: Mon Nov 18 2019 - 11:28:54 EST
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.