On giovedì 30 giugno 2022 19:38:08 CEST Eric W. Biederman wrote: > "Fabio M. De Francesco" <fmdefrancesco@xxxxxxxxx> writes: > > > The use of kmap() and kmap_atomic() are being deprecated in favor of > > kmap_local_page(). > > > > With kmap_local_page(), the mappings are per thread, CPU local and not > > globally visible. Furthermore, the mappings can be acquired from any > > context (including interrupts). > > > > Therefore, use kmap_local_page() in exec.c because these mappings are per > > thread, CPU local, and not globally visible. > > > > Tested with xfstests on a QEMU + KVM 32-bits VM booting a kernel with > > HIGHMEM64GB enabled. > > Can someone please refresh my memory on what is going on. > > I remember there were limitations that kmap_atomic had that are hard to > meet so something I think it was kmap_local was invented and created > to be the kmap_atomic replacement. Please read highmem.rst. I've updated that document weeks ago: https://docs.kernel.org/vm/highmem.html?highlight=highmem Currently it contains many more information I can ever place here in order to answer your questions. Believe me, this is not by any means a way to elude your questions. I'm pretty sure that by reading that document you'll have a clear vision on what is going on :-) > > What are the requirements on kmap_local? In copy_strings > kmap is called in contexts that can sleep in page faults No problems with kmap_local_page() with regard to page faults (again, please read the above-mentioned document). >From that document... "It’s valid to take pagefaults in a local kmap region []". "Each call of kmap_atomic() in the kernel creates a non-preemptible section and disable pagefaults. This could be a source of unwanted latency. Therefore users should prefer kmap_local_page() instead of kmap_atomic().". > so any > nearly any requirement except a thread local use is invalidated. > > As you have described kmap_local above it does not sound like kmap_local > is safe in this context, Sorry, probably I should add that taking page faults is allowed. Would you prefer I send a v2 and add this information? Thanks, Fabio > but that could just be a problem in description > that my poor memory does is not recalling the necessary details to > correct. > > Eric > > > Suggested-by: Ira Weiny <ira.weiny@xxxxxxxxx> > > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@xxxxxxxxx> > > --- > > fs/exec.c | 14 +++++++------- > > 1 file changed, 7 insertions(+), 7 deletions(-) > > > > diff --git a/fs/exec.c b/fs/exec.c > > index 0989fb8472a1..4a2129c0d422 100644 > > --- a/fs/exec.c > > +++ b/fs/exec.c > > @@ -583,11 +583,11 @@ static int copy_strings(int argc, struct user_arg_ptr argv, > > > > if (kmapped_page) { > > flush_dcache_page(kmapped_page); > > - kunmap(kmapped_page); > > + kunmap_local(kaddr); > > put_arg_page(kmapped_page); > > } > > kmapped_page = page; > > - kaddr = kmap(kmapped_page); > > + kaddr = kmap_local_page(kmapped_page); > > kpos = pos & PAGE_MASK; > > flush_arg_page(bprm, kpos, kmapped_page); > > } > > @@ -601,7 +601,7 @@ static int copy_strings(int argc, struct user_arg_ptr argv, > > out: > > if (kmapped_page) { > > flush_dcache_page(kmapped_page); > > - kunmap(kmapped_page); > > + kunmap_local(kaddr); > > put_arg_page(kmapped_page); > > } > > return ret; > > @@ -883,11 +883,11 @@ int transfer_args_to_stack(struct linux_binprm *bprm, > > > > for (index = MAX_ARG_PAGES - 1; index >= stop; index--) { > > unsigned int offset = index == stop ? bprm->p & ~PAGE_MASK : 0; > > - char *src = kmap(bprm->page[index]) + offset; > > + char *src = kmap_local_page(bprm->page[index]) + offset; > > sp -= PAGE_SIZE - offset; > > if (copy_to_user((void *) sp, src, PAGE_SIZE - offset) != 0) > > ret = -EFAULT; > > - kunmap(bprm->page[index]); > > + kunmap_local(src); > > if (ret) > > goto out; > > } > > @@ -1680,13 +1680,13 @@ int remove_arg_zero(struct linux_binprm *bprm) > > ret = -EFAULT; > > goto out; > > } > > - kaddr = kmap_atomic(page); > > + kaddr = kmap_local_page(page); > > > > for (; offset < PAGE_SIZE && kaddr[offset]; > > offset++, bprm->p++) > > ; > > > > - kunmap_atomic(kaddr); > > + kunmap_local(kaddr); > > put_arg_page(page); > > } while (offset == PAGE_SIZE); >
