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]

Date:   Mon, 16 Mar 2020 11:34:54 +0100
From:   Sebastian Andrzej Siewior <bigeasy@...utronix.de>
To:     Randy Dunlap <rdunlap@...radead.org>
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 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.

> > +   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?

…

Sebastian