linux-kernel - Re: [PATCH 5/6] iomap: drop unnecessary state

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: <ZqprvNM5itMbanuH@casper.infradead.org>
Date: Wed, 31 Jul 2024 17:52:12 +0100
From: Matthew Wilcox <willy@...radead.org>
To: Zhang Yi <yi.zhang@...weicloud.com>
Cc: linux-xfs@...r.kernel.org, linux-fsdevel@...r.kernel.org,
	linux-kernel@...r.kernel.org, djwong@...nel.org, hch@...radead.org,
	brauner@...nel.org, david@...morbit.com, jack@...e.cz,
	yi.zhang@...wei.com, chengzhihao1@...wei.com, yukuai3@...wei.com
Subject: Re: [PATCH 5/6] iomap: drop unnecessary state_lock when setting ifs
 uptodate bits

On Wed, Jul 31, 2024 at 05:13:04PM +0800, Zhang Yi wrote:
> Commit '1cea335d1db1 ("iomap: fix sub-page uptodate handling")' fix a
> race issue when submitting multiple read bios for a page spans more than
> one file system block by adding a spinlock(which names state_lock now)
> to make the page uptodate synchronous. However, the race condition only
> happened between the read I/O submitting and completeing threads, it's
> sufficient to use page lock to protect other paths, e.g. buffered write
> path. After large folio is supported, the spinlock could affect more
> about the buffered write performance, so drop it could reduce some
> unnecessary locking overhead.

This patch doesn't work.  If we get two read completions at the same
time for blocks belonging to the same folio, they will both write to
the uptodate array at the same time.