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.