mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Matthew Wilcox <willy@infradead.org>
To: "Fabio M. De Francesco" <fmdefrancesco@gmail.com>
Cc: Song Liu <song@kernel.org>, Takashi Iwai <tiwai@suse.de>,
	Adam Manzanares <a.manzanares@samsung.com>,
	Davidlohr Bueso <dave@stgolabs.net>,
	Luis Chamberlain <mcgrof@kernel.org>,
	Ira Weiny <ira.weiny@intel.com>,
	linux-modules@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] module: Replace kmap() with kmap_local_page()
Date: Tue, 19 Jul 2022 21:23:49 +0100	[thread overview]
Message-ID: <YtcS1QNcIrTt0DN1@casper.infradead.org> (raw)
In-Reply-To: <5303077.Sb9uPGUboI@opensuse>

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.

  reply	other threads:[~2022-07-19 20:24 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-18  0:26 Fabio M. De Francesco
2022-07-18 22:19 ` Luis Chamberlain
2022-07-19  9:19   ` Fabio M. De Francesco
2022-07-19 20:23     ` Matthew Wilcox [this message]
2022-07-19 21:35       ` Luis Chamberlain

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YtcS1QNcIrTt0DN1@casper.infradead.org \
    --to=willy@infradead.org \
    --cc=a.manzanares@samsung.com \
    --cc=dave@stgolabs.net \
    --cc=fmdefrancesco@gmail.com \
    --cc=ira.weiny@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-modules@vger.kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=song@kernel.org \
    --cc=tiwai@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®