Skip to content

remove the vmas parameter from GUP APIs

Lorenzo Stoakes amd-gfx bpf dri-devel io-uring kvm linux-arm-kernel linux-fsdevel linux-iommu linux-media linux-mm linux-perf-users linux-rdma linux-s390 linux-security-module linux-sgx linuxppc-dev lkml netdev 6 versions – Applied

Status from this message: [merged mm-stable] mm-gup-remove-vmas-array-from-internal-gup-functions.patch removed from -mm tree

Get series with b4
b4 am [email protected]

Versions newest first · linked by author and title

  1. v6 Latest6 patches 0 repliesApplied 6/6
  2. v56 patches 20 repliesSuperseded
  3. v46 patches 28 repliesSuperseded
Earlier versions · 3
VersionPostedPatchesRepliesStatus
v37 patches9 repliesSuperseded
v27 patches6 repliesSuperseded
v17 patches28 repliesSuperseded

Changes in v6 from the cover letter

- Fixed up unused variable warning under !CONFIG_HAVE_IOREMAP_PROT reported by Anders Roxell.
- Fixed up use of uninitialised vma parameter on error/no page found reported by Dan Carpenter (caused by reuse of code with helper that sets vma under different circumstances).
- Fixed up the missed kernel/trace/trace_events_user.c caller.

Patches in v6 All 6 applied

#Subject and filesSizeRepliesApplied
1/6mm/gup: remove unused vmas parameter from get_user_pages()arch/x86/kernel/cpu/sgx/ioctl.c drivers/gpu/drm/radeon/radeon_ttm.c drivers/misc/sgi-gru/grufault.c +4 more+10 -150 repliesApplied
2/6mm/gup: remove unused vmas parameter from pin_user_pages_remote()drivers/iommu/iommufd/pages.c drivers/vfio/vfio_iommu_type1.c include/linux/mm.h +3 more+9 -110 repliesApplied
3/6mm/gup: remove vmas parameter from get_user_pages_remote()arch/arm64/kernel/mte.c arch/s390/kvm/interrupt.c fs/exec.c +7 more+64 -430 repliesApplied
4/6io_uring: rsrc: delegate VMA file-backed check to GUPio_uring/rsrc.c+6 -280 repliesApplied
5/6mm/gup: remove vmas parameter from pin_user_pages()arch/powerpc/mm/book3s64/iommu_api.c drivers/infiniband/hw/qib/qib_user_pages.c drivers/infiniband/hw/usnic/usnic_uiom.c +9 more+17 -220 repliesApplied
6/6mm/gup: remove vmas array from internal GUP functionsinclude/linux/hugetlb.h mm/gup.c mm/hugetlb.c+45 -720 repliesApplied

Reviews trailers per patch

All 6 patches have a Reviewed-by or Acked-by.

PatchDavid HildenbrandJason GunthorpeChristoph HellwigGreg Kroah-HartmanChristian KönigJarkko SakkinenSean ChristophersonCatalin MarinasJanosch FrankJens AxboeDennis DalessandroSakari Ailus
1/6A-b R-b R-b A-b A-b A-b A-b
2/6A-b R-b R-b
3/6A-b R-b R-b R-b
4/6R-b R-b R-b
5/6A-b R-b A-b A-b
6/6A-b R-b

Trailers given in replies are filled; those carried in the patches from earlier versions are outlined with dashes. Scroll the table sideways for all 12 reviewers.

Read the v6 thread

Cover letter [PATCH v6 0/6] remove the vmas parameter from GUP APIs

(pin_/get)_user_pages[_remote]() each provide an optional output parameter for an array of VMA objects associated with each page in the input range.

These provide the means for VMAs to be returned, as long as mm->mmap_lock is never released during the GUP operation (i.e. the internal flag FOLL_UNLOCKABLE is not specified).

In addition, these VMAs can only be accessed with the mmap_lock held and become invalidated the moment it is released.

The vast majority of invocations do not use this functionality and of those that do, all but one case retrieve a single VMA to perform checks upon.

It is not egregious in the single VMA cases to simply replace the operation with a vma_lookup(). In these cases we duplicate the (fast) lookup on a slow path already under the mmap_lock, abstracted to a new get_user_page_vma_remote() inline helper function which also performs error checking and reference count maintenance.

The special case is io_uring, where io_pin_pages() specifically needs to assert that the VMAs underlying the range do not result in broken long-term GUP file-backed mappings.

As GUP now internally asserts that FOLL_LONGTERM mappings are not file-backed in a broken fashion (i.e. requiring dirty tracking) - as implemented in "mm/gup: disallow FOLL_LONGTERM GUP-nonfast writing to file-backed mappings" - this logic is no longer required and so we can simply remove it altogether from io_uring.

Eliminating the vmas parameter eliminates an entire class of danging pointer errors that might have occured should the lock have been incorrectly released.

In addition, the API is simplified and now clearly expresses what it is intended for - applying the specified GUP flags and (if pinning) returning pinned pages.

This change additionally opens the door to further potential improvements in GUP and the possible marrying of disparate code paths.

I have run this series against gup_test with no issues.

This patch series is rebased on mm-unstable as of 12th May.

Thanks to Matthew Wilcox for suggesting this refactoring!

v6:
- Fixed up unused variable warning under !CONFIG_HAVE_IOREMAP_PROT reported by Anders Roxell.
- Fixed up use of uninitialised vma parameter on error/no page found reported by Dan Carpenter (caused by reuse of code with helper that sets vma under different circumstances).
- Fixed up the missed kernel/trace/trace_events_user.c caller.

v5:
- Remove the io_uring open-coded VMA file-backed check, as this is now explicitly disallowed by GUP.
- Updated the subsequent patch to eliminate the vmas parameter accordingly.
https://lore.kernel/org/linux-mm/[email protected]/

v4:
- Drop FOLL_SAME_FILE as the complexity costs exceed the benefit of having it for a single case.
- Update io_pin_pages() to perform VMA lookup directly.
- Add get_user_page_vma_remote() to perform the single page/VMA lookup with error checks performed correctly.
https://lore.kernel.org/linux-mm/[email protected]/

v3:
- Always explicitly handle !vma cases, feeding back an error to the user if appropriate, indicating the operation did not completely succeed if not and always with a warning since these conditions should be impossible.
https://lore.kernel.org/linux-mm/[email protected]/

v2:
- Only lookup the VMA if the pin succeeded (other than __access_remote_vm() which has different semantics)
- Be pedantically careful about ensuring that under no circumstances can we fail to unpin a page
https://lore.kernel.org/linux-mm/[email protected]/

v1:
https://lore.kernel.org/linux-mm/[email protected]/

Lorenzo Stoakes (6):
mm/gup: remove unused vmas parameter from get_user_pages() mm/gup: remove unused vmas parameter from pin_user_pages_remote() mm/gup: remove vmas parameter from get_user_pages_remote()
io_uring: rsrc: delegate VMA file-backed check to GUP mm/gup: remove vmas parameter from pin_user_pages() mm/gup: remove vmas array from internal GUP functions

 arch/arm64/kernel/mte.c                    |  17 ++--
 arch/powerpc/mm/book3s64/iommu_api.c       |   2 +-
 arch/s390/kvm/interrupt.c                  |   2 +-
 arch/x86/kernel/cpu/sgx/ioctl.c            |   2 +-
 drivers/gpu/drm/radeon/radeon_ttm.c        |   2 +-
 drivers/infiniband/hw/qib/qib_user_pages.c |   2 +-
 drivers/infiniband/hw/usnic/usnic_uiom.c   |   2 +-
 drivers/infiniband/sw/siw/siw_mem.c        |   2 +-
 drivers/iommu/iommufd/pages.c              |   4 +-
 drivers/media/v4l2-core/videobuf-dma-sg.c  |   2 +-
 drivers/misc/sgi-gru/grufault.c            |   2 +-
 drivers/vdpa/vdpa_user/vduse_dev.c         |   2 +-
 drivers/vfio/vfio_iommu_type1.c            |   2 +-
 drivers/vhost/vdpa.c                       |   2 +-
 fs/exec.c                                  |   2 +-
 include/linux/hugetlb.h                    |  10 +-
 include/linux/mm.h                         |  42 +++++++--
 io_uring/rsrc.c                            |  34 ++-----
 kernel/events/uprobes.c                    |  13 +--
 kernel/trace/trace_events_user.c           |   2 +-
 mm/gup.c                                   | 105 +++++++--------------
 mm/gup_test.c                              |  14 ++-
 mm/hugetlb.c                               |  24 ++---
 mm/memory.c                                |  20 ++--
 mm/process_vm_access.c                     |   2 +-
 mm/rmap.c                                  |   2 +-
 net/xdp/xdp_umem.c                         |   2 +-
 security/tomoyo/domain.c                   |   2 +-
 virt/kvm/async_pf.c                        |   3 +-
 virt/kvm/kvm_main.c                        |   2 +-
 30 files changed, 142 insertions(+), 182 deletions(-)

--
2.40.1