linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Lorenzo Stoakes <lstoakes@gmail.com>
To: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>
Cc: Matthew Wilcox <willy@infradead.org>,
	David Hildenbrand <david@redhat.com>,
	Jens Axboe <axboe@kernel.dk>,
	Lorenzo Stoakes <lstoakes@gmail.com>
Subject: [PATCH v4 0/6] remove the vmas parameter from GUP APIs
Date: Tue, 18 Apr 2023 16:48:58 +0100	[thread overview]
Message-ID: <cover.1681831798.git.lstoakes@gmail.com> (raw)

(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 have also to only be accessed under the mmap_lock,
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 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 all the VMAs possess the same vm->vm_file (possibly NULL) and
they are either anonymous or hugetlb pages.

We adjust this to perform its own VMA lookup, which in most cases should
consist of a single VMA, so the performance cost on an already slow path
should be minimal. By doing so, we avoid an allocation in any case.

In future it is sensible to simply restrict write pinning of file-backed
folios in which case we may be able to simply avoid this check altogether,
but for the time being we maintain it as-is.

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

In addition the API is simplified and now clearly expresses what it is 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 the gup_test and a simple io_uring program which exercises the
use of FOLL_SAME_PAGE with no issues.

This patch series is rebased on mm-unstable as of 17th April.

Thanks to Matthew Wilcox for suggesting this refactoring!

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.

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/cover.1681558407.git.lstoakes@gmail.com/

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/cover.1681547405.git.lstoakes@gmail.com/

v1:
https://lore.kernel.org/linux-mm/cover.1681508038.git.lstoakes@gmail.com/

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: avoid use of vmas parameter in pin_user_pages()
  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                            |  53 ++++++-----
 kernel/events/uprobes.c                    |  13 +--
 mm/gup.c                                   | 105 +++++++--------------
 mm/gup_test.c                              |  14 ++-
 mm/hugetlb.c                               |  24 ++---
 mm/memory.c                                |  14 +--
 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 +-
 29 files changed, 161 insertions(+), 174 deletions(-)

--
2.40.0


             reply	other threads:[~2023-04-18 15:49 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-18 15:48 Lorenzo Stoakes [this message]
2023-04-18 15:49 ` [PATCH v4 1/6] mm/gup: remove unused vmas parameter from get_user_pages() Lorenzo Stoakes
2023-04-19  8:40   ` Christian König
2023-04-23 15:12   ` Jarkko Sakkinen
2023-04-18 15:49 ` [PATCH v4 2/6] mm/gup: remove unused vmas parameter from pin_user_pages_remote() Lorenzo Stoakes
2023-04-18 15:49 ` [PATCH v4 3/6] mm/gup: remove vmas parameter from get_user_pages_remote() Lorenzo Stoakes
2023-04-19 12:05   ` Janosch Frank
2023-04-18 15:49 ` [PATCH v4 4/6] io_uring: rsrc: avoid use of vmas parameter in pin_user_pages() Lorenzo Stoakes
2023-04-18 15:55   ` David Hildenbrand
2023-04-18 15:56     ` David Hildenbrand
2023-04-18 16:25     ` Lorenzo Stoakes
2023-04-19 16:35   ` Jens Axboe
2023-04-19 16:59     ` Jens Axboe
2023-04-19 17:23       ` Lorenzo Stoakes
2023-04-19 17:35         ` Jens Axboe
2023-04-19 17:47           ` Lorenzo Stoakes
2023-04-19 17:51             ` Jens Axboe
2023-04-19 18:18               ` Lorenzo Stoakes
2023-04-19 18:22                 ` Jason Gunthorpe
2023-04-19 18:50                   ` Lorenzo Stoakes
2023-04-19 18:23                 ` Matthew Wilcox
2023-04-19 18:24                   ` Jason Gunthorpe
2023-04-19 18:35                     ` Matthew Wilcox
2023-04-19 18:45                       ` Lorenzo Stoakes
2023-04-19 23:22                         ` Jason Gunthorpe
2023-04-20 13:57                           ` Lorenzo Stoakes
2023-04-20 13:36                       ` Pavel Begunkov
2023-04-20 14:19                         ` Lorenzo Stoakes
2023-04-20 15:31                           ` Jason Gunthorpe
2023-04-19 20:15                     ` Jens Axboe
2023-04-19 20:18                       ` Jens Axboe
2023-04-20 13:37                       ` Pavel Begunkov
2023-04-19 17:07     ` Lorenzo Stoakes
2023-04-18 15:49 ` [PATCH v4 5/6] mm/gup: remove vmas parameter from pin_user_pages() Lorenzo Stoakes
2023-04-18 15:49 ` [PATCH v4 6/6] mm/gup: remove vmas array from internal GUP functions Lorenzo Stoakes

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cover.1681831798.git.lstoakes@gmail.com \
    --to=lstoakes@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=axboe@kernel.dk \
    --cc=david@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).