linux-kernel - Re: [RFC 0/2] rwsem: introduce 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: <20241016085345.46956-1-lizhe.67@bytedance.com>
Date: Wed, 16 Oct 2024 16:53:45 +0800
From: lizhe.67@...edance.com
To: peterz@...radead.org
Cc: akpm@...ux-foundation.org,
	boqun.feng@...il.com,
	linux-kernel@...r.kernel.org,
	linux-mm@...ck.org,
	lizhe.67@...edance.com,
	longman@...hat.com,
	mingo@...hat.com,
	will@...nel.org
Subject: Re: [RFC 0/2] rwsem: introduce upgrade_read interface

On Wed, 16 Oct 2024 10:09:55 +0200, peterz@...radead.org wrote:

> On Wed, Oct 16, 2024 at 12:35:58PM +0800, lizhe.67@...edance.com wrote:
> > From: Li Zhe <lizhe.67@...edance.com>
> > 
> > In the current kernel rwsem implementation, there is an interface to
> > downgrade write lock to read lock, but there is no interface to upgrade
> > a read lock to write lock. This means that in order to acquire write
> > lock while holding read lock, we have to release the read lock first and
> > then acquire the write lock, which will introduce some troubles in
> > concurrent programming. This patch set provides the 'upgrade_read' interface
> > to solve this problem. This interface can change a read lock to a write
> > lock.
> 
> upgrade-read is fundamentally prone to deadlocks. Imagine two concurrent
> invocations, each waiting for all readers to go away before proceeding
> to upgrade to a writer.
>
> Any solution to fixing that will end up being semantically similar to
> dropping the read lock and acquiring a write lock -- there will not be a
> single continuous critical section.

According to the implementation of this patch, one of the invocation will
get '-EBUSY' in this case. If -EBUSY is obtained and the invocation thread
continues to retry instead of dropping the read lock and acquiring a write lock,
it may cause problems. Of course, this patchset only try it's best to achieve a
single continuous critical section as much as possible, and there is no guarantee.

> As such, this interface makes no sense.

This interface is just trying to reduce the overhead caused by the additional
checks, which is caused by non-continuous critical sections, as much as possible.
Rather than eliminating it in all scenarios. So would it be better to change the
error code to something else? So that the caller will not retry this interface?