linux-ext4 - Re: Kernel Benchmarking

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 17 Sep 2020 20:27:07 +0100
From:   Matthew Wilcox <willy@...radead.org>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Michael Larabel <Michael@...haellarabel.com>,
        Matthieu Baerts <matthieu.baerts@...sares.net>,
        Amir Goldstein <amir73il@...il.com>,
        Ted Ts'o <tytso@...gle.com>,
        Andreas Dilger <adilger.kernel@...ger.ca>,
        Ext4 Developers List <linux-ext4@...r.kernel.org>,
        Jan Kara <jack@...e.cz>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>
Subject: Re: Kernel Benchmarking

On Thu, Sep 17, 2020 at 12:00:06PM -0700, Linus Torvalds wrote:
> On Thu, Sep 17, 2020 at 11:50 AM Matthew Wilcox <willy@...radead.org> wrote:
> >
> > Ahh.  Here's a race this doesn't close:
> >
> > int truncate_inode_page(struct address_space *mapping, struct page *page)
> 
> I think this one currently depends on the page lock, doesn't it?
> 
> And I think the point would be to get rid of that dependency, and just
> make the rule be that it's done with the i_mmap_rwsem held for
> writing.

Ah, I see what you mean.  Hold the i_mmap_rwsem for write across,
basically, the entirety of truncate_inode_pages_range().  I don't see
a problem with lock scope; according to rmap.c, i_mmap_rwsem is near
the top of the hierarchy, just under lock_page.  We do wait for I/O to
complete (both reads and writes), but I don't know a reason for that to
be a problem.

We might want to take the page lock anyway to prevent truncate() from
racing with a read() that decides to start new I/O to this page, which
would involve adjusting the locking hierarchy (although to a way in which
hugetlb and the regular VM are back in sync).  My brain is starting to
hurt from thinking about ways that not taking the page lock in truncate
might go wrong.