linux-kernel - Re: [PATCH 1/9] Documentation: Add lock ordering and nesting documentation

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <de6fdc5d-690a-1130-c911-caccbb0b1a8f@infradead.org>
Date:   Mon, 16 Mar 2020 08:13:38 -0700
From:   Randy Dunlap <rdunlap@...radead.org>
To:     Sebastian Andrzej Siewior <bigeasy@...utronix.de>
Cc:     linux-kernel@...r.kernel.org,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...nel.org>, Will Deacon <will@...nel.org>,
        "Paul E . McKenney" <paulmck@...nel.org>,
        Joel Fernandes <joel@...lfernandes.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH 1/9] Documentation: Add lock ordering and nesting
 documentation

On 3/16/20 3:34 AM, Sebastian Andrzej Siewior wrote:
> On 2020-03-14 15:57:24 [-0700], Randy Dunlap wrote:
>> Hi,
> Hi Randy,
> 
>> A few comments for your consideration:
> 
> I merged all of you comments but two:
> 
>> On 3/13/20 10:46 AM, Sebastian Andrzej Siewior wrote:
> …
>>> +rwlock_t and PREEMPT_RT
>>> +-----------------------
>>> +
>>> +On a PREEMPT_RT enabled kernel rwlock_t is mapped to a separate
>>> +implementation based on rt_mutex which changes the semantics:
>>> +
>>> + - Same changes as for spinlock_t
>>> +
>>> + - The implementation is not fair and can cause writer starvation under
>>> +   certain circumstances. The reason for this is that a writer cannot
>>> +   inherit its priority to multiple readers. Readers which are blocked
>>
>>       ^^^^^^^ I think this is backwards. Maybe more like so:
>>                                                          a writer cannot
>>       bequeath or grant or bestow or pass down    ...    its priority to
> 
> So the term "inherit" is the problem. The protocol is officially called
> PI which is short for Priority Inheritance. Other documentation,
> RT-mutex for instance, is also using this term when it is referring to
> altering the priority of a task. For that reason I prefer to keep using
> this term.

OK, I get it.

>>> +   on a writer fully support the priority inheritance protocol.
> …
>>> +raw_spinlock_t
>>> +--------------
>>> +
>>> +As raw_spinlock_t locking disables preemption and eventually interrupts the
>>> +code inside the critical region has to be careful to avoid calls into code
>>
>> Can I buy a comma in there somewhere, please?
>> I don't get it as is.
> 
> What about
> 
> | As raw_spinlock_t locking disables preemption, and eventually interrupts, the
> | code inside the critical region has to be careful to avoid calls into code
> 
> any better?

Yes.

thanks.
-- 
~Randy