fs: Call kmap_local_page() in copy_string_kernel()
Subsystems (6)
PATCH 1/1 fs: Call kmap_local_page() in copy_string_kernel()
The use of kmap_atomic() is being deprecated in favor of kmap_local_page().
With kmap_local_page(), the mappings are per thread, CPU local, not globally visible and can take page faults. Furthermore, the mappings can be acquired from any context (including interrupts).
Therefore, use kmap_local_page() in copy_string_kernel() instead of kmap_atomic().
Tested with xfstests on a QEMU + KVM 32-bits VM booting a kernel with HIGHMEM64GB enabled.
---
I sent a first patch to fs/exec.c for converting kmap() and kmap_atomic() to kmap_local_page():
https://lore.kernel.org/lkml/[email protected]/
Some days ago, Ira Weiny, while he was reviewing that patch, made me notice that I had overlooked a second kmap_atomic() in the same file (thanks):
https://lore.kernel.org/lkml/YsiQptk19txHrG4c@iweiny-desk3/
I've been asked to send this as an additional change. This is why there will not be any second version of that previous patch.
fs/exec.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
fs/exec.c +2 -2
@@ -639,11 +639,11 @@ int copy_string_kernel(const char *arg, struct linux_binprm *bprm) page =get_arg_page (bprm, pos,1 );if (!page)return -E2BIG ; - kaddr =kmap_atomic (page); + kaddr =kmap_local_page (page);flush_arg_page (bprm, pos &PAGE_MASK , page);memcpy (kaddr +offset_in_page (pos), arg, bytes_to_copy);flush_dcache_page (page); -kunmap_atomic (kaddr); +kunmap_local (kaddr);put_arg_page (page); }
Signature · 1 line
2.36.1
Quoted 35 lines · Fabio M. De Francesco
The use of
kmap_atomic()is being deprecated in favor ofkmap_local_page().With
kmap_local_page(), the mappings are per thread, CPU local, not globally visible and can take page faults. Furthermore, the mappings can be acquired from any context (including interrupts).Therefore, use
kmap_local_page()incopy_string_kernel()instead ofkmap_atomic().Tested with xfstests on a QEMU + KVM 32-bits VM booting a kernel with HIGHMEM64GB enabled.
Suggested-by Ira Weiny <[email protected]>Signed-off-by Fabio M. De Francesco <[email protected]>---
I sent a first patch to
fs/exec.cfor convertingkmap()andkmap_atomic()tokmap_local_page():
https://lore.kernel.org/lkml/[email protected]/Some days ago, Ira Weiny, while he was reviewing that patch, made me notice that I had overlooked a second
kmap_atomic()in the same file (thanks):
https://lore.kernel.org/lkml/YsiQptk19txHrG4c@iweiny-desk3/I've been asked to send this as an additional change. This is why there will not be any second version of that previous patch.
fs/exec.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)fs/exec.c +0
-0
fs/exec.c int copy_string_kernel(const char *arg, struct linux_binprm *bprm) · line 643
Quoted 6 lines
fs/exec.c +1
-1@@ -639,11 +639,11 @@ int copy_string_kernel(const char *arg, struct linux_binprm *bprm) page =get_arg_page (bprm, pos,1 );if (!page)return -E2BIG ; - kaddr =kmap_atomic (page); + kaddr =kmap_local_page (page);flush_arg_page (bprm, pos &PAGE_MASK , page);
I really question why we can't use memcpy_to_page() here and move the flush_arg_page() prior to the mapping?
flush_arg_page() only calls flush_cache_page() which does not need the mapping to work correctly AFAICT.
Ira
Quoted 10 lines
fs/exec.c +1
-1memcpy (kaddr +offset_in_page (pos), arg, bytes_to_copy);flush_dcache_page (page); -kunmap_atomic (kaddr); +kunmap_local (kaddr);put_arg_page (page); }Signature · 1 line
2.36.1
Quoted 2 lines · On venerdì 22 luglio 2022 02:14:20 CEST Ira Weiny wrote:
Quoted 1 line · Fabio M. De Francesco
The use of
kmap_atomic()is being deprecated in favor of
kmap_local_page().
Quoted 3 lines
Quoted 3 lines
With
kmap_local_page(), the mappings are per thread, CPU local, not globally visible and can take page faults. Furthermore, the mappings
can be
Quoted 5 lines
Quoted 5 lines
acquired from any context (including interrupts).
Therefore, use
kmap_local_page()incopy_string_kernel()instead ofkmap_atomic().
Quoted 8 lines
Quoted 8 lines
Tested with xfstests on a QEMU + KVM 32-bits VM booting a kernel with HIGHMEM64GB enabled.
Suggested-by Ira Weiny <[email protected]>Signed-off-by Fabio M. De Francesco <[email protected]>---
I sent a first patch to
fs/exec.cfor convertingkmap()and
kmap_atomic()
Quoted 4 lines
Quoted 4 lines
to
kmap_local_page():
https://lore.kernel.org/lkml/[email protected]/Some days ago, Ira Weiny, while he was reviewing that patch, made me
notice
Quoted 4 lines
Quoted 4 lines
that I had overlooked a second
kmap_atomic()in the same file (thanks):
https://lore.kernel.org/lkml/YsiQptk19txHrG4c@iweiny-desk3/I've been asked to send this as an additional change. This is why there
will
Quoted 10 lines
fs/exec.c int copy_string_kernel(const char *arg, struct
Quoted 10 lines
not be any second version of that previous patch.
fs/exec.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)fs/exec.c +0
-0@@ -639,11 +639,11 @@ int copy_string_kernel(const char *arg, struct
linux_binprm *bprm)
Quoted 10 lines
fs/exec.c int copy_string_kernel(const char *arg, struct linux_binprm *bprm) · line 643
Quoted 6 lines
fs/exec.c +1
-1page =get_arg_page (bprm, pos,1 );if (!page)return -E2BIG ; - kaddr =kmap_atomic (page); + kaddr =kmap_local_page (page);flush_arg_page (bprm, pos &PAGE_MASK , page);I really question why we can't use
memcpy_to_page()here and move theflush_arg_page()prior to the mapping?
Quoted 2 lines
flush_arg_page()only callsflush_cache_page()which does not need the mapping to work correctly AFAICT.
You're right here. I'm sorry for being so lazy and not checking that flush_arg_page() does not need to be called while the task holds the local mapping :-(
In v2 I'll move flush_arg_page() one line above memcpy_to_page().
Thanks for your comment,
Fabio
Quoted 4 lines
Ira
Quoted 1 line
fs/exec.c +0
-0memcpy(kaddr + offset_in_page(pos), arg,
bytes_to_copy);
Quoted 10 lines
Quoted 9 lines
fs/exec.c +1
-1flush_dcache_page (page); -kunmap_atomic (kaddr); +kunmap_local (kaddr);put_arg_page (page); }Signature · 1 line
2.36.1