linux-kernel - Re: [RFC PATCH] locking/rwsem: Add upgrade

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: <YUuMv6fA+TpqH3wb@hirez.programming.kicks-ass.net>
Date:   Wed, 22 Sep 2021 22:06:23 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Waiman Long <longman@...hat.com>
Cc:     Ingo Molnar <mingo@...hat.com>, Will Deacon <will.deacon@....com>,
        Boqun Feng <boqun.feng@...il.com>,
        linux-kernel@...r.kernel.org, Davidlohr Bueso <dave@...olabs.net>
Subject: Re: [RFC PATCH] locking/rwsem: Add upgrade_read()

On Wed, Sep 22, 2021 at 03:36:57PM -0400, Waiman Long wrote:
> Currently there are about 12 instances in the kernel where an up_read()
> is immediately followed by a down_write() of the same lock. For example,
> 
>   drivers/tty/n_tty.c:		up_read(&tty->termios_rwsem);
>   drivers/tty/n_tty.c-		down_write(&tty->termios_rwsem);

And TTY is a high performance issue, that requires hacks like this?

> Since we have already provided a downgrade_write() function, we may as
> well provide an upgrade_read() function to make the code easier to read
> and the intention clearer.
> 
> If the current task is the only reader, the upgrade can be done by a
> single atomic operation. If not, the upgrade will have to be done by a
> separate up_read() call followed by a down_write(). In the former case,
> the handoff bit is not considered and the waiter will have to wait a
> bit longer to acquire the lock.
> 
> The new upgrade_read() function returns a value of 0 for safe upgrade
> where rwsem protected data won't change. Otherwise a value of 1 is
> returned to indicate unsafe upgrade where rwsem protected data may
> change during the upgrade process.

Yuck...

Is there any workload where this is a massive win? I'm thinking that
either the lock is contended and you get the unsafe option which is the
same as today, or the lock isn't contended and you would've gotten
fast-paths and you barely safe anything anyway.

Also, -ENODATA