Re: [RFC] locking: rwbase: Take care of ordering guarantee for fastpath reader

From: Boqun Feng
Date: Wed Sep 08 2021 - 09:10:27 EST


On Wed, Sep 08, 2021 at 01:51:24PM +0200, Peter Zijlstra wrote:
[...]
> @@ -201,23 +207,30 @@ static int __sched rwbase_write_lock(struct rwbase_rt *rwb,
> {
> struct rt_mutex_base *rtm = &rwb->rtmutex;
> unsigned long flags;
> + int readers;
>
> /* Take the rtmutex as a first step */
> if (rwbase_rtmutex_lock_state(rtm, state))
> return -EINTR;
>
> /* Force readers into slow path */
> - atomic_sub(READER_BIAS, &rwb->readers);
> + readers = atomic_sub_return_relaxed(READER_BIAS, &rwb->readers);
>
> - raw_spin_lock_irqsave(&rtm->wait_lock, flags);
> /*
> * set_current_state() for rw_semaphore
> * current_save_and_set_rtlock_wait_state() for rwlock
> */
> rwbase_set_and_save_current_state(state);

rwbase_set_and_save_current_state() may eventually call
current_save_and_set_rtlock_wait_state(), which requires being called
with irq-off, while rwbase_write_lock() may be called with irq-on. I
guess we can change the raw_spin_lock() to raw_spin_lock_irqsave() in
current_save_and_set_rtlock_wait_state() to solve this.

Regards,
Boqun

> + raw_spin_lock_irqsave(&rtm->wait_lock, flags);
>
> - /* Block until all readers have left the critical section. */
> - for (; atomic_read(&rwb->readers);) {
> + /*
> + * Block until all readers have left the critical section.
> + *
> + * In the case of !readers, the above implies TSO ordering
> + * at the very least and hence provides ACQUIRE vs the earlier
> + * atomic_sub_return_relaxed().
> + */
> + while (readers) {
> /* Optimized out for rwlocks */
> if (rwbase_signal_pending_state(state, current)) {
> __set_current_state(TASK_RUNNING);
[...]