Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   "Fabio M. De Francesco" <fmdefrancesco@gmail.com>
To:     Ira Weiny <ira.weiny@intel.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Catalin Marinas <catalin.marinas@arm.com>,
        "Matthew Wilcox (Oracle)" <willy@infradead.org>,
        Will Deacon <will@kernel.org>,
        Peter Collingbourne <pcc@google.com>,
        Vlastimil Babka <vbabka@suse.cz>,
        Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
        linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org,
        Jonathan Corbet <corbet@lwn.net>
Cc:     Mike Rapoport <rppt@linux.ibm.com>,
        Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [PATCH 3/7] Documentation/mm: Don't kmap*() pages which can't come from HIGHMEM
Date:   Thu, 28 Jul 2022 16:52:45 +0200
Message-ID: <7426546.EvYhyI6sBW@opensuse>
In-Reply-To: <87czdykw4j.fsf@meer.lwn.net>
References: <20220721210206.13774-1-fmdefrancesco@gmail.com> <20220721210206.13774-4-fmdefrancesco@gmail.com> <87czdykw4j.fsf@meer.lwn.net>
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset="utf-8"
Precedence: bulk

On gioved=C3=AC 21 luglio 2022 23:13:00 CEST Jonathan Corbet wrote:
> "Fabio M. De Francesco" <fmdefrancesco@gmail.com> writes:
>=20
> > There is no need to kmap*() pages which are guaranteed to come from
> > ZONE_NORMAL (or lower). Linux has currently several call sites of
> > kmap{,_atomic,_local_page}() on pages allocated, for instance, with
> > alloc_page(GFP_NOFS) and other similar allocations.
> >
> > Therefore, add a paragraph to highmem.rst, to explain better that a
> > plain page_address() should be used for getting the address of pages
> > which cannot come from ZONE_HIGHMEM.
> >
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: Matthew Wilcox (Oracle) <willy@infradead.org>
> > Cc: Mike Rapoport <rppt@linux.ibm.com>
> > Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Suggested-by: Ira Weiny <ira.weiny@intel.com>
> > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> > ---
> >  Documentation/vm/highmem.rst | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/Documentation/vm/highmem.rst b/Documentation/vm/
highmem.rst
> > index c9887f241c6c..f266354c82ab 100644
> > --- a/Documentation/vm/highmem.rst
> > +++ b/Documentation/vm/highmem.rst
> > @@ -71,6 +71,12 @@ list shows them in order of preference of use.
> >    kmap_local_page() always returns a valid virtual address and it is=20
assumed
> >    that kunmap_local() will never fail.
> > =20
> > +  On CONFIG_HIGHMEM=3Dn kernels and for low memory pages this returns=
=20
the
> > +  virtual address of the direct mapping. Only real highmem pages are
> > +  temporarily mapped. Therefore, users should instead call a plain
> > +  page_address() for getting the address of memory pages which,=20
depending
> > +  on the GFP_* flags, cannot come from ZONE_HIGHMEM.
> > +
>=20
> Is this good advice?

Well... yes and no :-)=20

However yours is a legit objection.=20

I'm taking most of the suggestion from Ira (from an email in this same=20
thread) and send the v2 of this series.

My intention was to avoid things like those I encountered when converting=20
fs/btrfs:

page =3D alloc_page(GFP_NOFS);
kaddr =3D kmap(page);

Why one should kmap*() pages allocated one or two lines above with=20
GFP_NOFS?=20

=46urthermore, since nesting kmap_local_page() / kunmap_local() is last in=
=20
first out (LIFO), I had several problems with several un-mappings until=20
David Sterba made me notice that GFP_NOFS is not OR'ed with __GFP_HIGHMEM=20
and suggested to use plain page_address() instead of those unnecessary=20
mappings.

However, you are right about the fact that, with most of other allocations,=
=20
it is not so clear where and how pages are being allocated.

Thanks,

=46abio=20

> First, it requires developers to worry about
> whether their pages might be in highmem, which is kind of like worrying
> about having coins in your pocket in case you need a payphone.  But it
> would also run afoul of other semantics for kmap*(), such as PKS, should
> that ever be merged:
>=20
>   https://lwn.net/Articles/894531/
>
> Thanks,
>=20
> jon
>=20