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:     Bagas Sanjaya <bagasdotme@gmail.com>
Cc:     Jonathan Corbet <corbet@lwn.net>,
        Andrew Morton <akpm@linux-foundation.org>,
        Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
        Ira Weiny <ira.weiny@intel.com>,
        Mike Rapoport <rppt@kernel.org>, linux-doc@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-mm@kvack.org,
        Peter Zijlstra <a.p.zijlstra@chello.nl>,
        Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [PATCH v2] mm/highmem: Add notes about conversions from kmap{,_atomic}()
Date:   Thu, 08 Dec 2022 21:05:56 +0100
Message-ID: <21697972.EfDdHjke4D@suse>
In-Reply-To: <Y5FmjdBNEO8aP/Ls@debian.me>
References: <20221207225308.8290-1-fmdefrancesco@gmail.com> <Y5FmjdBNEO8aP/Ls@debian.me>
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset="iso-8859-1"
Precedence: bulk

On gioved=EC 8 dicembre 2022 05:22:37 CET Bagas Sanjaya wrote:
> On Wed, Dec 07, 2022 at 11:53:08PM +0100, Fabio M. De Francesco wrote:
> > diff --git a/Documentation/mm/highmem.rst b/Documentation/mm/highmem.rst
> > index 0f731d9196b0..59d1078f53df 100644
> > --- a/Documentation/mm/highmem.rst
> > +++ b/Documentation/mm/highmem.rst
> > @@ -57,7 +57,8 @@ list shows them in order of preference of use.
> >=20
> >    It can be invoked from any context (including interrupts) but the
> >    mappings
> >    can only be used in the context which acquired them.
> >=20
> > -  This function should be preferred, where feasible, over all the othe=
rs.
> > +  This function should always be used. kmap_atomic() and kmap() have b=
een
> > +  deprecated.
>=20
> "... always be used, whereas ..."
>=20
> > +  NOTE: Conversions to kmap_local_page() must take care to follow the
> > mapping +  restrictions imposed on kmap_local_page(). Furthermore, the=
=20
code
> > between +  calls to kmap_atomic() and kunmap_atomic() may implicitly=20
depend
> > on the side +  effects of atomic mappings, i.e. disabling page faults or
> > preemption, or both. +  In that case, explicit calls to=20
pagefault_disable()
> > or preempt_disable() or +  both must be made in conjunction with the us=
e=20
of
> > kmap_local_page(). +
> > <snipped>...
> > +  NOTE: Conversions to kmap_local_page() must take care to follow the
> > mapping +  restrictions imposed on kmap_local_page(). In particular, it=
 is
> > necessary to +  make sure that the kernel virtual memory pointer is only
> > valid in the thread +  that obtained it.
> > +
>=20
> What about using note block to signify conversion notes above?
>=20
> ---- >8 ----
>=20
> diff --git a/Documentation/mm/highmem.rst b/Documentation/mm/highmem.rst
> index 59d1078f53df57..ef53eb580d4cda 100644
> --- a/Documentation/mm/highmem.rst
> +++ b/Documentation/mm/highmem.rst
> @@ -103,12 +103,14 @@ list shows them in order of preference of use.
>=20
>  * kmap_atomic(). This function has been deprecated; use kmap_local_page(=
).
>=20
> -  NOTE: Conversions to kmap_local_page() must take care to follow the=20
mapping
> -  restrictions imposed on kmap_local_page(). Furthermore, the code betwe=
en=20
=2D
>  calls to kmap_atomic() and kunmap_atomic() may implicitly depend on the=
=20
side
> -  effects of atomic mappings, i.e. disabling page faults or preemption, =
or
> both. -  In that case, explicit calls to pagefault_disable() or
> preempt_disable() or -  both must be made in conjunction with the use of
> kmap_local_page(). +  .. note::
> +     Conversions to kmap_local_page() must take care to follow the mappi=
ng
> +     restrictions imposed on kmap_local_page(). Furthermore, the code=20
between
> +     calls to kmap_atomic() and kunmap_atomic() may implicitly depend on=
=20
the
> +     side effects of atomic mappings, i.e. disabling page faults or
> preemption, +     or both. In that case, explicit calls to
> pagefault_disable() or +     preempt_disable() or both must be made in
> conjunction with the use of +     kmap_local_page().
>=20
>    [Legacy documentation]
>=20
> @@ -129,10 +131,11 @@ list shows them in order of preference of use.
>=20
>  * kmap(). This function has been deprecated; use kmap_local_page().
>=20
> -  NOTE: Conversions to kmap_local_page() must take care to follow the=20
mapping
> -  restrictions imposed on kmap_local_page(). In particular, it is necess=
ary
> to -  make sure that the kernel virtual memory pointer is only valid in t=
he
> thread -  that obtained it.
> +  .. note::
> +     Conversions to kmap_local_page() must take care to follow the mappi=
ng
> +     restrictions imposed on kmap_local_page(). In particular, it is
> necessary +     to make sure that the kernel virtual memory pointer is on=
ly
> valid in the +     thread that obtained it.
>=20
>    [Legacy documentation]
>=20
> Thanks.
>=20
> --
> An old man doll... just what I always wanted! - Clara

You provided valid suggestions, thanks!
However, immediately after submitting this v2 patch, Andrew added it to the=
 -
mm mm-unstable branch.
I'll do the changes that you suggested in a subsequent patch which will bui=
ld=20
on this.

Again thanks,

=46abio