linux-ext4 - Re: [GIT PULL] Hole puch vs page cache filling races fixes for 5.14-rc1

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>] [day] [month] [year] [list]

Message-ID: <20210706201714.GD17149@quack2.suse.cz>
Date:   Tue, 6 Jul 2021 22:17:14 +0200
From:   Jan Kara <jack@...e.cz>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Jan Kara <jack@...e.cz>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        "Darrick J. Wong" <djwong@...nel.org>, linux-ext4@...r.kernel.org,
        linux-mm@...ck.org, Christoph Hellwig <hch@...radead.org>
Subject: Re: [GIT PULL] Hole puch vs page cache filling races fixes for
 5.14-rc1

On Thu 01-07-21 11:04:28, Linus Torvalds wrote:
> On Thu, Jul 1, 2021 at 9:19 AM Jan Kara <jack@...e.cz> wrote:
> >
> > That being said I don't expect the optimization to matter too much
> > because in do_read_fault() we first call do_fault_around() which will
> > exactly map pages that are already in cache and uptodate
> 
> Yeah, I think that ends up saving the situation.
> 
> > So do you think the optimization is still worth it despite
> > do_fault_around()?
> 
> I suspect it doesn't matter that much for performance as you say due
> to any filesystem that cares about performance having the "map_pages"
> function pointing to filemap_map_pages, but I reacted to it just from
> looking at the patch, and it just seems conceptually wrong. Taking the
> lock in a situation where it's not actually needed will just cause
> problems later when somebody decides that the lock protects something
> else entirely.

OK, I did some experiments and indeed in my totally unscientific boot and
compile tests I've seen ~90% of page faults to not get to filemap_fault()
(they were either anon memory or satisfied with filemap_map_pages()). From
the remaining 10% which got to filemap_fault() about 95% already had a page
in the page cache (so the optimization would help) - usually because we
were doing a write fault. So the optimization seems worth it.  I've added
attached patch on top of the series which implements the optimization you
suggested. I've also pushed out the complete series to

git://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs.git hole_punch_fixes

so that people can see the whole picture. Review is welcome!

								Honza
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR

View attachment "0001-mm-Acquire-invalidate_lock-in-filemap_fault-only-whe.patch" of type "text/x-patch" (3694 bytes)