linux-kernel - Re: [PATCH] module: Replace kmap() with kmap_local

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

Message-ID: <Ytcjhw6tckKe7V/U@bombadil.infradead.org>
Date:   Tue, 19 Jul 2022 14:35:03 -0700
From:   Luis Chamberlain <mcgrof@...nel.org>
To:     Matthew Wilcox <willy@...radead.org>
Cc:     "Fabio M. De Francesco" <fmdefrancesco@...il.com>,
        Song Liu <song@...nel.org>, Takashi Iwai <tiwai@...e.de>,
        Adam Manzanares <a.manzanares@...sung.com>,
        Davidlohr Bueso <dave@...olabs.net>,
        Ira Weiny <ira.weiny@...el.com>, linux-modules@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] module: Replace kmap() with kmap_local_page()

On Tue, Jul 19, 2022 at 09:23:49PM +0100, Matthew Wilcox wrote:
> On Tue, Jul 19, 2022 at 11:19:24AM +0200, Fabio M. De Francesco wrote:
> > On martedì 19 luglio 2022 00:19:50 CEST Luis Chamberlain wrote:
> > > > Therefore, replace kmap() with kmap_local_page().
> > > 
> > > While this churn is going on everywhere I was wondering why not
> > > go ahead and adopt kmap_local_folio() instead?
> > 
> > I'm sorry but, due to my lack of knowledge and experience, I'm not sure to 
> > understand how kmap_local_folio() could help here. My fault. I'm going to 
> > make some research and ask for help from more experienced developers. 
> 
> I haven't made this suggestion to Fabio before for a few reasons.
> 
> First, it makes his work harder.  He not only has to understand the
> implications of the kmap semantic changes but also the implications of
> the folio change.
> 
> Then, I'm not sure that I necessarily have enough infrastructure in place
> for doing a folio conversion everywhere that he's doing a kmap/kmap_atomic
> to kmap_local_page conversion.
> 
> What makes it particularly tricky is that you can only kmap a single
> page out of a folio at a time; there's no ability to kmap the entire
> folio, no matter how large it is.  I've looked at doing the conversion
> for ext2 directories, and it's _tricky_.  There's only one 'checked'
> flag for the entire folio, but ext2_check_page() needs to take a mapped
> page.  So now we have to make a judgement call about whether to support
> caching ext2 directories with large folios or whether to restrict them
> to single-page folios.
> 
> So yes, there's probably a second patch coming for maintainers to look
> at that will convert the kmap_local_page() to kmap_local_folio().
> However, I think it's actually less of a burden for maintainers if
> these two different conversions happen separately because there are very
> different considerations to review.  Also, there's no equivalent to kmap()
> or kmap_atomic() for folios (deliberately), so the more conversions to
> kmap_local_page() Fabio gets done, the easier it will be for a later
> folio conversion.

Makes sense, thanks for the feedback. I'll wrestle with ensuring the first
step to kmap_local_page() doens't break things where I see them
suggested first.

  Luis