On Thu, Jul 21, 2022 at 03:13:00PM -0600, Jonathan Corbet wrote:
> "Fabio M. De Francesco" <fmdefrancesco@xxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxx>
> > Cc: Matthew Wilcox (Oracle) <willy@xxxxxxxxxxxxx>
> > Cc: Mike Rapoport <rppt@xxxxxxxxxxxxx>
> > Cc: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
> > Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> > Suggested-by: Ira Weiny <ira.weiny@xxxxxxxxx>
> > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@xxxxxxxxx>
> > ---
> > 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.
This is a good point. Perhaps this is better worded as:
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 may call a plain page_address()
for pages which are known to not come from ZONE_HIGHMEM.
However, it is always safe to use kmap_local_page()/kunmap_local() and
access through those calls will be as efficient as page_address() on
most architectures.
> But it
> would also run afoul of other semantics for kmap*(), such as PKS, should
> that ever be merged:
>
> https://lwn.net/Articles/894531/
PKS is yet to be merged. As of now, there is no good reason to force users to
use the kmap_local_page() if the page zone is known.
I believe that beyond PKS there will come a time when we need to change the
page_address() callers but currently this documentation is correct and does
allow callers to optimize for the corner case of a HIGHMEM system if they
desire.
As a reference, the use of kmap* vs page_address() was discussed recently in a
couple of places[1][2] and I can't fault the logic there at this time.
Thanks for the review,
Ira
[1] https://lore.kernel.org/linux-btrfs/20220621131521.GW20633@xxxxxxxxxxxxx/
[2] https://lore.kernel.org/lkml/CANn89iK6g+4Fy2VMV7=feUAOUDHu-J38be+oU76yp+zGH6xCJQ@xxxxxxxxxxxxxx/
>
> Thanks,
>
> jon
