Re: [PATCH 3/7] Documentation/mm: Don't kmap*() pages which can't come from HIGHMEM

[Jonathan Corbet <corbet@lwn.net>]

"Fabio M. De Francesco" <fmdefrancesco@gmail.com> writes:

> 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 assumed
>    that kunmap_local() will never fail.
>  
> +  On CONFIG_HIGHMEM=n kernels and for low memory pages this returns 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, depending
> +  on the GFP_* flags, cannot come from ZONE_HIGHMEM.
> +

Is this good advice?  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:

  https://lwn.net/Articles/894531/

Thanks,

jon