lkml.org 
[lkml]   [2022]   [Jul]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH] fs: Replace kmap{,_atomic}() with kmap_local_page()
Date
On giovedì 30 giugno 2022 19:38:08 CEST Eric W. Biederman wrote:
> "Fabio M. De Francesco" <fmdefrancesco@gmail.com> 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@intel.com>
> > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> > ---
> > 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);
>




\
 
 \ /
  Last update: 2022-07-01 12:11    [W:0.094 / U:0.124 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site