linux-kernel - Re: [PATCH 0/2] locking: Provide {rwlock|spin}_is_contended on PREEMPT

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210907100944.7qu3frjuuty3oi3d@linutronix.de>
Date:   Tue, 7 Sep 2021 12:09:44 +0200
From:   Sebastian Andrzej Siewior <bigeasy@...utronix.de>
To:     linux-kernel@...r.kernel.org
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>, Will Deacon <will@...nel.org>,
        Waiman Long <longman@...hat.com>,
        Boqun Feng <boqun.feng@...il.com>,
        Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH 0/2] locking: Provide {rwlock|spin}_is_contended on
 PREEMPT_RT

On 2021-09-06 16:30:02 [+0200], To linux-kernel@...r.kernel.org 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