Linux-Sgx Archive mirror
 help / color / mirror / Atom feed
From: "Jarkko Sakkinen" <jarkko@kernel.org>
To: "Lorenzo Stoakes" <lstoakes@gmail.com>, <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>, <x86@kernel.org>,
	<linux-sgx@vger.kernel.org>, <amd-gfx@lists.freedesktop.org>,
	<dri-devel@lists.freedesktop.org>, <kvm@vger.kernel.org>,
	"Thomas Gleixner" <tglx@linutronix.de>,
	"Ingo Molnar" <mingo@redhat.com>,
	"Borislav Petkov" <bp@alien8.de>,
	"H . Peter Anvin" <hpa@zytor.com>,
	"Xinhui Pan" <Xinhui.Pan@amd.com>,
	"David Airlie" <airlied@gmail.com>,
	"Daniel Vetter" <daniel@ffwll.ch>,
	"Dimitri Sivanich" <dimitri.sivanich@hpe.com>,
	"Arnd Bergmann" <arnd@arndb.de>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Jason Gunthorpe" <jgg@nvidia.com>
Subject: Re: [PATCH v4 1/6] mm/gup: remove unused vmas parameter from get_user_pages()
Date: Sun, 23 Apr 2023 18:12:54 +0300	[thread overview]
Message-ID: <CS48DTLH7UEG.1PX2N6DVS1UDR@suppilovahvero> (raw)
In-Reply-To: <cd05b41d6d15ee9ff94273bc116ed3db3f5125bf.1681831798.git.lstoakes@gmail.com>

On Tue Apr 18, 2023 at 6:49 PM EEST, Lorenzo Stoakes wrote:
> No invocation of get_user_pages() uses the vmas parameter, so remove
> it.
>
> The GUP API is confusing and caveated. Recent changes have done much to
> improve that, however there is more we can do. Exporting vmas is a prime
> target as the caller has to be extremely careful to preclude their use
> after the mmap_lock has expired or otherwise be left with dangling
> pointers.
>
> Removing the vmas parameter focuses the GUP functions upon their primary
> purpose - pinning (and outputting) pages as well as performing the actions
> implied by the input flags.
>
> This is part of a patch series aiming to remove the vmas parameter
> altogether.
>
> Suggested-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Acked-by: David Hildenbrand <david@redhat.com>
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>
> ---
>  arch/x86/kernel/cpu/sgx/ioctl.c     | 2 +-
>  drivers/gpu/drm/radeon/radeon_ttm.c | 2 +-
>  drivers/misc/sgi-gru/grufault.c     | 2 +-
>  include/linux/mm.h                  | 3 +--
>  mm/gup.c                            | 9 +++------
>  mm/gup_test.c                       | 5 ++---
>  virt/kvm/kvm_main.c                 | 2 +-
>  7 files changed, 10 insertions(+), 15 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c
> index 21ca0a831b70..5d390df21440 100644
> --- a/arch/x86/kernel/cpu/sgx/ioctl.c
> +++ b/arch/x86/kernel/cpu/sgx/ioctl.c
> @@ -214,7 +214,7 @@ static int __sgx_encl_add_page(struct sgx_encl *encl,
>  	if (!(vma->vm_flags & VM_MAYEXEC))
>  		return -EACCES;
>  
> -	ret = get_user_pages(src, 1, 0, &src_page, NULL);
> +	ret = get_user_pages(src, 1, 0, &src_page);
>  	if (ret < 1)
>  		return -EFAULT;
>  
> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
> index 1e8e287e113c..0597540f0dde 100644
> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> @@ -362,7 +362,7 @@ static int radeon_ttm_tt_pin_userptr(struct ttm_device *bdev, struct ttm_tt *ttm
>  		struct page **pages = ttm->pages + pinned;
>  
>  		r = get_user_pages(userptr, num_pages, write ? FOLL_WRITE : 0,
> -				   pages, NULL);
> +				   pages);
>  		if (r < 0)
>  			goto release_pages;
>  
> diff --git a/drivers/misc/sgi-gru/grufault.c b/drivers/misc/sgi-gru/grufault.c
> index b836936e9747..378cf02a2aa1 100644
> --- a/drivers/misc/sgi-gru/grufault.c
> +++ b/drivers/misc/sgi-gru/grufault.c
> @@ -185,7 +185,7 @@ static int non_atomic_pte_lookup(struct vm_area_struct *vma,
>  #else
>  	*pageshift = PAGE_SHIFT;
>  #endif
> -	if (get_user_pages(vaddr, 1, write ? FOLL_WRITE : 0, &page, NULL) <= 0)
> +	if (get_user_pages(vaddr, 1, write ? FOLL_WRITE : 0, &page) <= 0)
>  		return -EFAULT;
>  	*paddr = page_to_phys(page);
>  	put_page(page);
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 37554b08bb28..b14cc4972d0b 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2380,8 +2380,7 @@ long pin_user_pages_remote(struct mm_struct *mm,
>  			   unsigned int gup_flags, struct page **pages,
>  			   struct vm_area_struct **vmas, int *locked);
>  long get_user_pages(unsigned long start, unsigned long nr_pages,
> -			    unsigned int gup_flags, struct page **pages,
> -			    struct vm_area_struct **vmas);
> +		    unsigned int gup_flags, struct page **pages);
>  long pin_user_pages(unsigned long start, unsigned long nr_pages,
>  		    unsigned int gup_flags, struct page **pages,
>  		    struct vm_area_struct **vmas);
> diff --git a/mm/gup.c b/mm/gup.c
> index 1f72a717232b..7e454d6b157e 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -2251,8 +2251,6 @@ long get_user_pages_remote(struct mm_struct *mm,
>   * @pages:      array that receives pointers to the pages pinned.
>   *              Should be at least nr_pages long. Or NULL, if caller
>   *              only intends to ensure the pages are faulted in.
> - * @vmas:       array of pointers to vmas corresponding to each page.
> - *              Or NULL if the caller does not require them.
>   *
>   * This is the same as get_user_pages_remote(), just with a less-flexible
>   * calling convention where we assume that the mm being operated on belongs to
> @@ -2260,16 +2258,15 @@ long get_user_pages_remote(struct mm_struct *mm,
>   * obviously don't pass FOLL_REMOTE in here.
>   */
>  long get_user_pages(unsigned long start, unsigned long nr_pages,
> -		unsigned int gup_flags, struct page **pages,
> -		struct vm_area_struct **vmas)
> +		    unsigned int gup_flags, struct page **pages)
>  {
>  	int locked = 1;
>  
> -	if (!is_valid_gup_args(pages, vmas, NULL, &gup_flags, FOLL_TOUCH))
> +	if (!is_valid_gup_args(pages, NULL, NULL, &gup_flags, FOLL_TOUCH))
>  		return -EINVAL;
>  
>  	return __get_user_pages_locked(current->mm, start, nr_pages, pages,
> -				       vmas, &locked, gup_flags);
> +				       NULL, &locked, gup_flags);
>  }
>  EXPORT_SYMBOL(get_user_pages);
>  
> diff --git a/mm/gup_test.c b/mm/gup_test.c
> index 8ae7307a1bb6..9ba8ea23f84e 100644
> --- a/mm/gup_test.c
> +++ b/mm/gup_test.c
> @@ -139,8 +139,7 @@ static int __gup_test_ioctl(unsigned int cmd,
>  						 pages + i);
>  			break;
>  		case GUP_BASIC_TEST:
> -			nr = get_user_pages(addr, nr, gup->gup_flags, pages + i,
> -					    NULL);
> +			nr = get_user_pages(addr, nr, gup->gup_flags, pages + i);
>  			break;
>  		case PIN_FAST_BENCHMARK:
>  			nr = pin_user_pages_fast(addr, nr, gup->gup_flags,
> @@ -161,7 +160,7 @@ static int __gup_test_ioctl(unsigned int cmd,
>  						    pages + i, NULL);
>  			else
>  				nr = get_user_pages(addr, nr, gup->gup_flags,
> -						    pages + i, NULL);
> +						    pages + i);
>  			break;
>  		default:
>  			ret = -EINVAL;
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index d255964ec331..7f31e0a4adb5 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -2474,7 +2474,7 @@ static inline int check_user_page_hwpoison(unsigned long addr)
>  {
>  	int rc, flags = FOLL_HWPOISON | FOLL_WRITE;
>  
> -	rc = get_user_pages(addr, 1, flags, NULL, NULL);
> +	rc = get_user_pages(addr, 1, flags, NULL);
>  	return rc == -EHWPOISON;
>  }
>  
> -- 
> 2.40.0


Acked-by: Jarkko Sakkinen <jarkko@kernel.org>

BR, Jarkko

      parent reply	other threads:[~2023-04-23 15:13 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <cover.1681831798.git.lstoakes@gmail.com>
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 [this message]

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=CS48DTLH7UEG.1PX2N6DVS1UDR@suppilovahvero \
    --to=jarkko@kernel.org \
    --cc=Xinhui.Pan@amd.com \
    --cc=airlied@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=arnd@arndb.de \
    --cc=bp@alien8.de \
    --cc=daniel@ffwll.ch \
    --cc=david@redhat.com \
    --cc=dimitri.sivanich@hpe.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=hpa@zytor.com \
    --cc=jgg@nvidia.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-sgx@vger.kernel.org \
    --cc=lstoakes@gmail.com \
    --cc=mingo@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=willy@infradead.org \
    --cc=x86@kernel.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 read-only IMAP folder(s) and NNTP newsgroup(s).