Re: [PATCH 0/2] locking: Provide {rwlock|spin}_is_contended on PREEMPT_RT
From: Sebastian Andrzej Siewior
Date: Tue Sep 07 2021 - 06:09:50 EST
On 2021-09-06 16:30:02 [+0200], To linux-kernel@xxxxxxxxxxxxxxx wrote:
> The lkp bot reported that rt_rwlock_is_contended() is not used so I
> looked closer.
>
> #1 wires up rwlock_is_contended() with rt_rwlock_is_contended() as it
> was probably intended. As noted in the patch description, a writer
> will always see true based on the current implementation. This could be
> solved by looking at the waiters of the rtmutex underneath as done in #2
> for spin_lock (which is missing). The helper is not exported and would
> be also needed for rwsem_is_contended() because it is using the same
> rw_base_is_contended() implementation.
>
> Maybe it is not worth the trouble given that on PREEMPT_RT the rwlock/
> spinlock is preemptible so it might be just best to return false and
> wait for the scheduler to do its magic.
I looked at callers and would argue that we could simply return false
here for all is_conded checks (as we do). We don't spin on RT but
schedule out on contention and even if there are waiters, as long as we
are the task with the highest priority, then the lock won't be handed
over to the next task in line. Therefore I suggest to remove the unused
rt_rwlock_is_contended().
The rwsem_is_contended() makes sense since it is only used by readers.
Also it is a sleep-able lock so it has the same semantic as !RT.
However. shrink_slab() will drop the lock _and_ could move on which is
good. The two mmap_lock_is_contended() user will drop the read lock and
acquire it again which will always succeed as long as there is another
reader.
Unlike the !RT implementation of rwsem which would not allow that.
Sebastian