On venerdì 19 agosto 2022 20:29:31 CEST Matthew Wilcox wrote:
> On Fri, Aug 19, 2022 at 09:50:37AM +0200, Fabio M. De Francesco wrote:
> > On venerdì 19 agosto 2022 00:40:10 CEST Eric Biggers wrote:
> > > From: Eric Biggers <ebiggers@xxxxxxxxxx>
> > >
> > > Convert the use of kmap() to its recommended replacement
> > > kmap_local_page(). This avoids the overhead of doing a non-local
> > > mapping, which is unnecessary in this case.
> > >
> > > Signed-off-by: Eric Biggers <ebiggers@xxxxxxxxxx>
> > > ---
> > >
> > > fs/verity/read_metadata.c | 6 +++---
> > > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > It looks good to me...
> >
> > > - virt = kmap(page);
> > > + virt = kmap_local_page(page);
> > >
> > > if (copy_to_user(buf, virt + offs_in_page, bytes_to_copy))
> >
> > {
> >
> > > - kunmap(page);
> > > + kunmap_local(virt);
> > >
> > > put_page(page);
> > > err = -EFAULT;
> > > break;
> > >
> > > }
> > >
> > > - kunmap(page);
> > > + kunmap_local(virt);
>
> Is this a common pattern? eg do we want something like:
>
> static inline int copy_user_page(void __user *dst, struct page *page,
> size_t offset, size_t len)
> {
> char *src = kmap_local_page(page) + offset;
> int err = 0;
>
> VM_BUG_ON(offset + len > PAGE_SIZE);
> if (copy_to_user(dst, src, len))
> err = -EFAULT;
>
> kunmap_local(src);
> return err;
> }
>
> in highmem.h?
Not sure that it is much common...
Can the following command provide any insight?
opensuse:/usr/src/git/kernels/linux> grep -rn copy_to_user -B7 . --exclude-
dir\=Documentation | grep kmap
./drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c-2306- ptr =
kmap_local_page(p);
./drivers/gpu/drm/i915/i915_gem.c-215- vaddr = kmap_local_page(page);
./drivers/gpu/drm/radeon/radeon_ttm.c-872- ptr =
kmap(page);
./drivers/vfio/pci/mlx5/main.c-182- from_buff =
kmap_local_page(page);
./arch/parisc/kernel/cache.c-580- kto = kmap_local_page(to);
./mm/memory.c-5474- maddr = kmap(page);
./fs/verity/read_metadata.c-56- virt = kmap(page);
./fs/aio.c-1252- ev = kmap_local_page(page);
./fs/exec.c-883- char *src = kmap_local_page(bprm->page[index])
+ offset;
If this command is good to provide any hint, it suggests that having
copy_to_user() in highmem.h is not probably worth.
Thanks,
Fabio
