The Highmem's interface is evolving and the current documentation does not
reflect the intended uses of each of the calls. Furthermore, after a
recent series of reworks, the differences of the calls can still be
confusing and may lead to the expanded use of calls which are deprecated.
This series is the second round of changes towards an enhanced
documentation of the Highmem's interface; at this stage the patches are
only focused to kmap_local_page().
In addition it also contains some minor clean ups.
Cc: Andrew Morton <[email protected]>
Cc: Matthew Wilcox (Oracle) <[email protected]>
Cc: Mike Rapoport <[email protected]>
Cc: Sebastian Andrzej Siewior <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Suggested-by: Ira Weiny <[email protected]>
Signed-off-by: Fabio M. De Francesco <[email protected]>
v1->v2: According to a comment from Jonathan Corbet and some
modifications suggested by Ira Weiny, change a couple of phrases in 3/7.
1,2,4-7/7 have no changes since v1.
Fabio M. De Francesco (7):
highmem: Remove unneeded spaces in kmap_local_page() kdocs
highmem: Specify that kmap_local_page() is callable from interrupts
Documentation/mm: Don't kmap*() pages which can't come from HIGHMEM
Documentation/mm: Avoid invalid use of addresses from
kmap_local_page()
Documentation/mm: Prefer kmap_local_page() and avoid kmap()
highmem: Delete a sentence from kmap_local_page() kdocs
Documentation/mm: Add details about kmap_local_page() and preemption
Documentation/vm/highmem.rst | 31 +++++++++++++++++++++++++++----
include/linux/highmem.h | 7 +++----
2 files changed, 30 insertions(+), 8 deletions(-)
--
2.37.1
What happens if a thread is preempted after mapping pages with
kmap_local_page() was questioned recently.[1]
Commit f3ba3c710ac5 ("mm/highmem: Provide kmap_local*") from Thomas
Gleixner explains clearly that on context switch, the maps of an
outgoing task are removed and the map of the incoming task are restored
and that kmap_local_page() can be invoked from both preemptible and
atomic contexts.[2]
Therefore, for the purpose to make it clearer that users can call
kmap_local_page() from contexts that allow preemption, rework a couple
of sentences and add further information in highmem.rst.
[1] https://lore.kernel.org/lkml/5303077.Sb9uPGUboI@opensuse/
[2] https://lore.kernel.org/all/[email protected]/
Cc: Andrew Morton <[email protected]>
Cc: Matthew Wilcox (Oracle) <[email protected]>
Cc: Mike Rapoport <[email protected]>
Cc: Sebastian Andrzej Siewior <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Suggested-by: Ira Weiny <[email protected]>
Signed-off-by: Fabio M. De Francesco <[email protected]>
---
Documentation/vm/highmem.rst | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/Documentation/vm/highmem.rst b/Documentation/vm/highmem.rst
index e045a4b7b3da..0f731d9196b0 100644
--- a/Documentation/vm/highmem.rst
+++ b/Documentation/vm/highmem.rst
@@ -60,14 +60,19 @@ list shows them in order of preference of use.
This function should be preferred, where feasible, over all the others.
These mappings are thread-local and CPU-local, meaning that the mapping
- can only be accessed from within this thread and the thread is bound the
- CPU while the mapping is active. Even if the thread is preempted (since
- preemption is never disabled by the function) the CPU can not be
- unplugged from the system via CPU-hotplug until the mapping is disposed.
+ can only be accessed from within this thread and the thread is bound to the
+ CPU while the mapping is active. Although preemption is never disabled by
+ this function, the CPU can not be unplugged from the system via
+ CPU-hotplug until the mapping is disposed.
It's valid to take pagefaults in a local kmap region, unless the context
in which the local mapping is acquired does not allow it for other reasons.
+ As said, pagefaults and preemption are never disabled. There is no need to
+ disable preemption because, when context switches to a different task, the
+ maps of the outgoing task are saved and those of the incoming one are
+ restored.
+
kmap_local_page() always returns a valid virtual address and it is assumed
that kunmap_local() will never fail.
--
2.37.1
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 which are clearly known which
can't come from ZONE_HIGHMEM.
Therefore, add a paragraph to highmem.rst, to explain better that a
plain page_address() may be used for getting the address of pages
which cannot come from ZONE_HIGHMEM, although it is always safe to use
kmap_local_page() / kunmap_local() also on those pages.
Cc: Andrew Morton <[email protected]>
Cc: Matthew Wilcox (Oracle) <[email protected]>
Cc: Mike Rapoport <[email protected]>
Cc: Sebastian Andrzej Siewior <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Suggested-by: Ira Weiny <[email protected]>
Signed-off-by: Fabio M. De Francesco <[email protected]>
---
Documentation/vm/highmem.rst | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/vm/highmem.rst b/Documentation/vm/highmem.rst
index c9887f241c6c..34d7097d3ce8 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 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().
+
Nesting kmap_local_page() and kmap_atomic() mappings is allowed to a certain
extent (up to KMAP_TYPE_NR) but their invocations have to be strictly ordered
because the map implementation is stack based. See kmap_local_page() kdocs
--
2.37.1
The reasoning for converting kmap() to kmap_local_page() was questioned
recently.[1]
There are two main problems with kmap(): (1) It comes with an overhead as
mapping space is restricted and protected by a global lock for
synchronization and (2) kmap() also requires global TLB invalidation when
its pool wraps and it might block when the mapping space is fully utilized
until a slot becomes available.
Warn users to avoid the use of kmap() and instead use kmap_local_page(),
by designing their code to map pages in the same context the mapping will
be used.
[1] https://lore.kernel.org/lkml/1891319.taCxCBeP46@opensuse/
Cc: Andrew Morton <[email protected]>
Cc: Matthew Wilcox (Oracle) <[email protected]>
Cc: Mike Rapoport <[email protected]>
Cc: Sebastian Andrzej Siewior <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Suggested-by: Ira Weiny <[email protected]>
Signed-off-by: Fabio M. De Francesco <[email protected]>
---
Documentation/vm/highmem.rst | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/Documentation/vm/highmem.rst b/Documentation/vm/highmem.rst
index 71dc09563ff8..e045a4b7b3da 100644
--- a/Documentation/vm/highmem.rst
+++ b/Documentation/vm/highmem.rst
@@ -84,6 +84,11 @@ list shows them in order of preference of use.
be absolutely sure to keep the use of the return address local to the
thread which mapped it.
+ Most code can be designed to use thread local mappings. User should
+ therefore try to design their code to avoid the use of kmap() by mapping
+ pages in the same thread the address will be used and prefer
+ kmap_local_page().
+
Nesting kmap_local_page() and kmap_atomic() mappings is allowed to a certain
extent (up to KMAP_TYPE_NR) but their invocations have to be strictly ordered
because the map implementation is stack based. See kmap_local_page() kdocs
--
2.37.1
In a recent thread about converting kmap() to kmap_local_page(), the
safety of calling kmap_local_page() was questioned.[1]
"any context" should probably be enough detail for users who want to
know whether or not kmap_local_page() can be called from interrupts.
However, Linux still has kmap_atomic() which might make users think they
must use the latter in interrupts.
Add "including interrupts" for better clarity.
[1] https://lore.kernel.org/lkml/3187836.aeNJFYEL58@opensuse/
Cc: Andrew Morton <[email protected]>
Cc: Matthew Wilcox (Oracle) <[email protected]>
Cc: Mike Rapoport <[email protected]>
Cc: Sebastian Andrzej Siewior <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Suggested-by: Ira Weiny <[email protected]>
Signed-off-by: Fabio M. De Francesco <[email protected]>
---
include/linux/highmem.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/highmem.h b/include/linux/highmem.h
index accd286a6af5..0ba031ad29c2 100644
--- a/include/linux/highmem.h
+++ b/include/linux/highmem.h
@@ -64,7 +64,7 @@ static inline void kmap_flush_unused(void);
*
* Returns: The virtual address of the mapping
*
- * Can be invoked from any context.
+ * Can be invoked from any context, including interrupts.
*
* Requires careful handling when nesting multiple mappings because the map
* management is stack based. The unmap has to be in the reverse order of
--
2.37.1
kmap_local_page() should always be preferred in place of kmap() and
kmap_atomic(). "Only use when really necessary." is not consistent with
the Documentation/mm/highmem.rst and these kdocs it embeds.
Therefore, delete the above-mentioned sentence from kdocs.
Cc: Andrew Morton <[email protected]>
Cc: Matthew Wilcox (Oracle) <[email protected]>
Cc: Mike Rapoport <[email protected]>
Cc: Sebastian Andrzej Siewior <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Suggested-by: Ira Weiny <[email protected]>
Reviewed-by: Ira Weiny <[email protected]>
Signed-off-by: Fabio M. De Francesco <[email protected]>
---
include/linux/highmem.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/include/linux/highmem.h b/include/linux/highmem.h
index 0ba031ad29c2..63f25dfc6317 100644
--- a/include/linux/highmem.h
+++ b/include/linux/highmem.h
@@ -86,8 +86,7 @@ static inline void kmap_flush_unused(void);
* temporarily mapped.
*
* While it is significantly faster than kmap() for the higmem case it
- * comes with restrictions about the pointer validity. Only use when really
- * necessary.
+ * comes with restrictions about the pointer validity.
*
* On HIGHMEM enabled systems mapping a highmem page has the side effect of
* disabling migration in order to keep the virtual address stable across
--
2.37.1
Users of kmap_local_page() must be absolutely sure to not hand kernel
virtual address obtained calling kmap_local_page() on highmem pages to
other contexts because those pointers are thread local, therefore, they
are no longer valid across different contexts.
Extend the documentation of kmap_local_page() to warn users about the
above-mentioned potential invalid use of pointers returned by
kmap_local_page().
Cc: Andrew Morton <[email protected]>
Cc: Matthew Wilcox (Oracle) <[email protected]>
Cc: Mike Rapoport <[email protected]>
Cc: Sebastian Andrzej Siewior <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Suggested-by: Ira Weiny <[email protected]>
Signed-off-by: Fabio M. De Francesco <[email protected]>
---
Documentation/vm/highmem.rst | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/Documentation/vm/highmem.rst b/Documentation/vm/highmem.rst
index 34d7097d3ce8..71dc09563ff8 100644
--- a/Documentation/vm/highmem.rst
+++ b/Documentation/vm/highmem.rst
@@ -77,6 +77,13 @@ list shows them in order of preference of use.
for pages which are known to not come from ZONE_HIGHMEM. However, it is
always safe to use kmap_local_page() / kunmap_local().
+ While it is significantly faster than kmap(), for the higmem case it
+ comes with restrictions about the pointers validity. Contrary to kmap()
+ mappings, the local mappings are only valid in the context of the caller
+ and cannot be handed to other contexts. This implies that users must
+ be absolutely sure to keep the use of the return address local to the
+ thread which mapped it.
+
Nesting kmap_local_page() and kmap_atomic() mappings is allowed to a certain
extent (up to KMAP_TYPE_NR) but their invocations have to be strictly ordered
because the map implementation is stack based. See kmap_local_page() kdocs
--
2.37.1