* [PATCH 0/2] mm/gup: introduce pin_user_pages_locked(), use it in frame_vector.c
@ 2020-05-27 22:32 John Hubbard
2020-05-27 22:32 ` [PATCH 1/2] mm/gup: introduce pin_user_pages_locked() John Hubbard
2020-05-27 22:32 ` [PATCH 2/2] mm/gup: frame_vector: convert get_user_pages() --> pin_user_pages() John Hubbard
0 siblings, 2 replies; 10+ messages in thread
From: John Hubbard @ 2020-05-27 22:32 UTC (permalink / raw)
To: Andrew Morton; +Cc: LKML, linux-mm, John Hubbard
Hi,
This applies on top of the pin_user_pages_unlocked() patch that I sent
earlier [1].
This adds yet one more pin_user_pages*() variant, and uses that to
convert mm/frame_vector.c.
With this, along with maybe 20 or 30 other recent patches in various
trees, we are close to having the relevant gup call sites
converted--with the notable exception of the bio/block layer.
[1] https://lore.kernel.org/r/20200518012157.1178336-2-jhubbard@nvidia.com
John Hubbard (2):
mm/gup: introduce pin_user_pages_locked()
mm/gup: frame_vector: convert get_user_pages() --> pin_user_pages()
include/linux/mm.h | 2 ++
mm/frame_vector.c | 7 +++----
mm/gup.c | 30 ++++++++++++++++++++++++++++++
3 files changed, 35 insertions(+), 4 deletions(-)
base-commit: 9cb1fd0efd195590b828b9b865421ad345a4a145
--
2.26.2
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] mm/gup: introduce pin_user_pages_locked()
2020-05-27 22:32 [PATCH 0/2] mm/gup: introduce pin_user_pages_locked(), use it in frame_vector.c John Hubbard
@ 2020-05-27 22:32 ` John Hubbard
2020-05-28 8:43 ` David Hildenbrand
` (2 more replies)
2020-05-27 22:32 ` [PATCH 2/2] mm/gup: frame_vector: convert get_user_pages() --> pin_user_pages() John Hubbard
1 sibling, 3 replies; 10+ messages in thread
From: John Hubbard @ 2020-05-27 22:32 UTC (permalink / raw)
To: Andrew Morton; +Cc: LKML, linux-mm, John Hubbard
Introduce pin_user_pages_locked(), which is nearly identical to
get_user_pages_locked() except that it sets FOLL_PIN and rejects
FOLL_GET.
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
include/linux/mm.h | 2 ++
mm/gup.c | 30 ++++++++++++++++++++++++++++++
2 files changed, 32 insertions(+)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 98be7289d7e9..d16951087c93 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1700,6 +1700,8 @@ long pin_user_pages(unsigned long start, unsigned long nr_pages,
struct vm_area_struct **vmas);
long get_user_pages_locked(unsigned long start, unsigned long nr_pages,
unsigned int gup_flags, struct page **pages, int *locked);
+long pin_user_pages_locked(unsigned long start, unsigned long nr_pages,
+ unsigned int gup_flags, struct page **pages, int *locked);
long get_user_pages_unlocked(unsigned long start, unsigned long nr_pages,
struct page **pages, unsigned int gup_flags);
long pin_user_pages_unlocked(unsigned long start, unsigned long nr_pages,
diff --git a/mm/gup.c b/mm/gup.c
index 2f0a0b497c23..17418a949067 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -2992,3 +2992,33 @@ long pin_user_pages_unlocked(unsigned long start, unsigned long nr_pages,
return get_user_pages_unlocked(start, nr_pages, pages, gup_flags);
}
EXPORT_SYMBOL(pin_user_pages_unlocked);
+
+/*
+ * pin_user_pages_locked() is the FOLL_PIN variant of get_user_pages_locked().
+ * Behavior is the same, except that this one sets FOLL_PIN and rejects
+ * FOLL_GET.
+ */
+long pin_user_pages_locked(unsigned long start, unsigned long nr_pages,
+ unsigned int gup_flags, struct page **pages,
+ int *locked)
+{
+ /*
+ * FIXME: Current FOLL_LONGTERM behavior is incompatible with
+ * FAULT_FLAG_ALLOW_RETRY because of the FS DAX check requirement on
+ * vmas. As there are no users of this flag in this call we simply
+ * disallow this option for now.
+ */
+ if (WARN_ON_ONCE(gup_flags & FOLL_LONGTERM))
+ return -EINVAL;
+
+ /* FOLL_GET and FOLL_PIN are mutually exclusive. */
+ if (WARN_ON_ONCE(gup_flags & FOLL_GET))
+ return -EINVAL;
+
+ gup_flags |= FOLL_PIN;
+ return __get_user_pages_locked(current, current->mm, start, nr_pages,
+ pages, NULL, locked,
+ gup_flags | FOLL_TOUCH);
+}
+EXPORT_SYMBOL(pin_user_pages_locked);
+
--
2.26.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] mm/gup: frame_vector: convert get_user_pages() --> pin_user_pages()
2020-05-27 22:32 [PATCH 0/2] mm/gup: introduce pin_user_pages_locked(), use it in frame_vector.c John Hubbard
2020-05-27 22:32 ` [PATCH 1/2] mm/gup: introduce pin_user_pages_locked() John Hubbard
@ 2020-05-27 22:32 ` John Hubbard
2020-05-28 8:46 ` David Hildenbrand
1 sibling, 1 reply; 10+ messages in thread
From: John Hubbard @ 2020-05-27 22:32 UTC (permalink / raw)
To: Andrew Morton; +Cc: LKML, linux-mm, John Hubbard
This code was using get_user_pages*(), and all of the callers so far
were in a "Case 2" scenario (DMA/RDMA), using the categorization
from [1]. That means that it's time to convert the get_user_pages*() +
put_page() calls to pin_user_pages*() + unpin_user_pages() calls.
There is some helpful background in [2]: basically, this is a small
part of fixing a long-standing disconnect between pinning pages, and
file systems' use of those pages.
[1] Documentation/core-api/pin_user_pages.rst
[2] "Explicit pinning of user-space pages":
https://lwn.net/Articles/807108/
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
mm/frame_vector.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/mm/frame_vector.c b/mm/frame_vector.c
index c431ca81dad5..4107dbca0056 100644
--- a/mm/frame_vector.c
+++ b/mm/frame_vector.c
@@ -72,7 +72,7 @@ int get_vaddr_frames(unsigned long start, unsigned int nr_frames,
if (!(vma->vm_flags & (VM_IO | VM_PFNMAP))) {
vec->got_ref = true;
vec->is_pfns = false;
- ret = get_user_pages_locked(start, nr_frames,
+ ret = pin_user_pages_locked(start, nr_frames,
gup_flags, (struct page **)(vec->ptrs), &locked);
goto out;
}
@@ -122,7 +122,6 @@ EXPORT_SYMBOL(get_vaddr_frames);
*/
void put_vaddr_frames(struct frame_vector *vec)
{
- int i;
struct page **pages;
if (!vec->got_ref)
@@ -135,8 +134,8 @@ void put_vaddr_frames(struct frame_vector *vec)
*/
if (WARN_ON(IS_ERR(pages)))
goto out;
- for (i = 0; i < vec->nr_frames; i++)
- put_page(pages[i]);
+
+ unpin_user_pages(pages, vec->nr_frames);
vec->got_ref = false;
out:
vec->nr_frames = 0;
--
2.26.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] mm/gup: introduce pin_user_pages_locked()
2020-05-27 22:32 ` [PATCH 1/2] mm/gup: introduce pin_user_pages_locked() John Hubbard
@ 2020-05-28 8:43 ` David Hildenbrand
2020-05-31 7:04 ` Souptick Joarder
2020-05-31 13:03 ` Pankaj Gupta
2 siblings, 0 replies; 10+ messages in thread
From: David Hildenbrand @ 2020-05-28 8:43 UTC (permalink / raw)
To: John Hubbard, Andrew Morton; +Cc: LKML, linux-mm
On 28.05.20 00:32, John Hubbard wrote:
> Introduce pin_user_pages_locked(), which is nearly identical to
> get_user_pages_locked() except that it sets FOLL_PIN and rejects
> FOLL_GET.
>
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> ---
> include/linux/mm.h | 2 ++
> mm/gup.c | 30 ++++++++++++++++++++++++++++++
> 2 files changed, 32 insertions(+)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 98be7289d7e9..d16951087c93 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1700,6 +1700,8 @@ long pin_user_pages(unsigned long start, unsigned long nr_pages,
> struct vm_area_struct **vmas);
> long get_user_pages_locked(unsigned long start, unsigned long nr_pages,
> unsigned int gup_flags, struct page **pages, int *locked);
> +long pin_user_pages_locked(unsigned long start, unsigned long nr_pages,
> + unsigned int gup_flags, struct page **pages, int *locked);
> long get_user_pages_unlocked(unsigned long start, unsigned long nr_pages,
> struct page **pages, unsigned int gup_flags);
> long pin_user_pages_unlocked(unsigned long start, unsigned long nr_pages,
> diff --git a/mm/gup.c b/mm/gup.c
> index 2f0a0b497c23..17418a949067 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -2992,3 +2992,33 @@ long pin_user_pages_unlocked(unsigned long start, unsigned long nr_pages,
> return get_user_pages_unlocked(start, nr_pages, pages, gup_flags);
> }
> EXPORT_SYMBOL(pin_user_pages_unlocked);
> +
> +/*
> + * pin_user_pages_locked() is the FOLL_PIN variant of get_user_pages_locked().
> + * Behavior is the same, except that this one sets FOLL_PIN and rejects
> + * FOLL_GET.
> + */
> +long pin_user_pages_locked(unsigned long start, unsigned long nr_pages,
> + unsigned int gup_flags, struct page **pages,
> + int *locked)
> +{
> + /*
> + * FIXME: Current FOLL_LONGTERM behavior is incompatible with
> + * FAULT_FLAG_ALLOW_RETRY because of the FS DAX check requirement on
> + * vmas. As there are no users of this flag in this call we simply
> + * disallow this option for now.
> + */
> + if (WARN_ON_ONCE(gup_flags & FOLL_LONGTERM))
> + return -EINVAL;
> +
> + /* FOLL_GET and FOLL_PIN are mutually exclusive. */
> + if (WARN_ON_ONCE(gup_flags & FOLL_GET))
> + return -EINVAL;
> +
> + gup_flags |= FOLL_PIN;
> + return __get_user_pages_locked(current, current->mm, start, nr_pages,
> + pages, NULL, locked,
> + gup_flags | FOLL_TOUCH);
> +}
> +EXPORT_SYMBOL(pin_user_pages_locked);
> +
>
LGTM
Reviewed-by: David Hildenbrand <david@redhat.com>
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] mm/gup: frame_vector: convert get_user_pages() --> pin_user_pages()
2020-05-27 22:32 ` [PATCH 2/2] mm/gup: frame_vector: convert get_user_pages() --> pin_user_pages() John Hubbard
@ 2020-05-28 8:46 ` David Hildenbrand
0 siblings, 0 replies; 10+ messages in thread
From: David Hildenbrand @ 2020-05-28 8:46 UTC (permalink / raw)
To: John Hubbard, Andrew Morton; +Cc: LKML, linux-mm
On 28.05.20 00:32, John Hubbard wrote:
> This code was using get_user_pages*(), and all of the callers so far
> were in a "Case 2" scenario (DMA/RDMA), using the categorization
> from [1]. That means that it's time to convert the get_user_pages*() +
> put_page() calls to pin_user_pages*() + unpin_user_pages() calls.
>
> There is some helpful background in [2]: basically, this is a small
> part of fixing a long-standing disconnect between pinning pages, and
> file systems' use of those pages.
>
> [1] Documentation/core-api/pin_user_pages.rst
>
> [2] "Explicit pinning of user-space pages":
> https://lwn.net/Articles/807108/
>
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> ---
> mm/frame_vector.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/mm/frame_vector.c b/mm/frame_vector.c
> index c431ca81dad5..4107dbca0056 100644
> --- a/mm/frame_vector.c
> +++ b/mm/frame_vector.c
> @@ -72,7 +72,7 @@ int get_vaddr_frames(unsigned long start, unsigned int nr_frames,
> if (!(vma->vm_flags & (VM_IO | VM_PFNMAP))) {
> vec->got_ref = true;
> vec->is_pfns = false;
> - ret = get_user_pages_locked(start, nr_frames,
> + ret = pin_user_pages_locked(start, nr_frames,
> gup_flags, (struct page **)(vec->ptrs), &locked);
> goto out;
> }
> @@ -122,7 +122,6 @@ EXPORT_SYMBOL(get_vaddr_frames);
> */
> void put_vaddr_frames(struct frame_vector *vec)
> {
> - int i;
> struct page **pages;
>
> if (!vec->got_ref)
> @@ -135,8 +134,8 @@ void put_vaddr_frames(struct frame_vector *vec)
> */
> if (WARN_ON(IS_ERR(pages)))
> goto out;
> - for (i = 0; i < vec->nr_frames; i++)
> - put_page(pages[i]);
> +
> + unpin_user_pages(pages, vec->nr_frames);
> vec->got_ref = false;
> out:
> vec->nr_frames = 0;
>
Not familiar with this code, but from what I can tell this LGTM.
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] mm/gup: introduce pin_user_pages_locked()
2020-05-27 22:32 ` [PATCH 1/2] mm/gup: introduce pin_user_pages_locked() John Hubbard
2020-05-28 8:43 ` David Hildenbrand
@ 2020-05-31 7:04 ` Souptick Joarder
2020-05-31 7:13 ` Souptick Joarder
2020-05-31 19:52 ` John Hubbard
2020-05-31 13:03 ` Pankaj Gupta
2 siblings, 2 replies; 10+ messages in thread
From: Souptick Joarder @ 2020-05-31 7:04 UTC (permalink / raw)
To: John Hubbard; +Cc: Andrew Morton, LKML, Linux-MM
On Thu, May 28, 2020 at 4:02 AM John Hubbard <jhubbard@nvidia.com> wrote:
>
> Introduce pin_user_pages_locked(), which is nearly identical to
> get_user_pages_locked() except that it sets FOLL_PIN and rejects
> FOLL_GET.
>
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> ---
> include/linux/mm.h | 2 ++
> mm/gup.c | 30 ++++++++++++++++++++++++++++++
> 2 files changed, 32 insertions(+)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 98be7289d7e9..d16951087c93 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1700,6 +1700,8 @@ long pin_user_pages(unsigned long start, unsigned long nr_pages,
> struct vm_area_struct **vmas);
> long get_user_pages_locked(unsigned long start, unsigned long nr_pages,
> unsigned int gup_flags, struct page **pages, int *locked);
> +long pin_user_pages_locked(unsigned long start, unsigned long nr_pages,
> + unsigned int gup_flags, struct page **pages, int *locked);
> long get_user_pages_unlocked(unsigned long start, unsigned long nr_pages,
> struct page **pages, unsigned int gup_flags);
> long pin_user_pages_unlocked(unsigned long start, unsigned long nr_pages,
> diff --git a/mm/gup.c b/mm/gup.c
> index 2f0a0b497c23..17418a949067 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -2992,3 +2992,33 @@ long pin_user_pages_unlocked(unsigned long start, unsigned long nr_pages,
> return get_user_pages_unlocked(start, nr_pages, pages, gup_flags);
> }
> EXPORT_SYMBOL(pin_user_pages_unlocked);
> +
> +/*
> + * pin_user_pages_locked() is the FOLL_PIN variant of get_user_pages_locked().
> + * Behavior is the same, except that this one sets FOLL_PIN and rejects
> + * FOLL_GET.
> + */
> +long pin_user_pages_locked(unsigned long start, unsigned long nr_pages,
> + unsigned int gup_flags, struct page **pages,
> + int *locked)
> +{
> + /*
> + * FIXME: Current FOLL_LONGTERM behavior is incompatible with
> + * FAULT_FLAG_ALLOW_RETRY because of the FS DAX check requirement on
> + * vmas. As there are no users of this flag in this call we simply
> + * disallow this option for now.
> + */
> + if (WARN_ON_ONCE(gup_flags & FOLL_LONGTERM))
> + return -EINVAL;
> +
> + /* FOLL_GET and FOLL_PIN are mutually exclusive. */
> + if (WARN_ON_ONCE(gup_flags & FOLL_GET))
> + return -EINVAL;
> +
> + gup_flags |= FOLL_PIN;
Right now get_user_pages_locked() doesn't have similar check for FOLL_PIN
and also not setting FOLL_GET internally irrespective of gup_flags
passed by user.
Do we need to add the same in get_user_pages_locked() ?
> + return __get_user_pages_locked(current, current->mm, start, nr_pages,
> + pages, NULL, locked,
> + gup_flags | FOLL_TOUCH);
> +}
> +EXPORT_SYMBOL(pin_user_pages_locked);
> +
> --
> 2.26.2
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] mm/gup: introduce pin_user_pages_locked()
2020-05-31 7:04 ` Souptick Joarder
@ 2020-05-31 7:13 ` Souptick Joarder
2020-05-31 19:56 ` John Hubbard
2020-05-31 19:52 ` John Hubbard
1 sibling, 1 reply; 10+ messages in thread
From: Souptick Joarder @ 2020-05-31 7:13 UTC (permalink / raw)
To: John Hubbard; +Cc: Andrew Morton, LKML, Linux-MM
On Sun, May 31, 2020 at 12:34 PM Souptick Joarder <jrdr.linux@gmail.com> wrote:
>
> On Thu, May 28, 2020 at 4:02 AM John Hubbard <jhubbard@nvidia.com> wrote:
> >
> > Introduce pin_user_pages_locked(), which is nearly identical to
> > get_user_pages_locked() except that it sets FOLL_PIN and rejects
> > FOLL_GET.
Forget to ask, is it fine to add this new pin_user_pages_locked() in
Documentation/core-api/pin_user_pages.rst ?
> >
> > Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> > ---
> > include/linux/mm.h | 2 ++
> > mm/gup.c | 30 ++++++++++++++++++++++++++++++
> > 2 files changed, 32 insertions(+)
> >
> > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > index 98be7289d7e9..d16951087c93 100644
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
> > @@ -1700,6 +1700,8 @@ long pin_user_pages(unsigned long start, unsigned long nr_pages,
> > struct vm_area_struct **vmas);
> > long get_user_pages_locked(unsigned long start, unsigned long nr_pages,
> > unsigned int gup_flags, struct page **pages, int *locked);
> > +long pin_user_pages_locked(unsigned long start, unsigned long nr_pages,
> > + unsigned int gup_flags, struct page **pages, int *locked);
> > long get_user_pages_unlocked(unsigned long start, unsigned long nr_pages,
> > struct page **pages, unsigned int gup_flags);
> > long pin_user_pages_unlocked(unsigned long start, unsigned long nr_pages,
> > diff --git a/mm/gup.c b/mm/gup.c
> > index 2f0a0b497c23..17418a949067 100644
> > --- a/mm/gup.c
> > +++ b/mm/gup.c
> > @@ -2992,3 +2992,33 @@ long pin_user_pages_unlocked(unsigned long start, unsigned long nr_pages,
> > return get_user_pages_unlocked(start, nr_pages, pages, gup_flags);
> > }
> > EXPORT_SYMBOL(pin_user_pages_unlocked);
> > +
> > +/*
> > + * pin_user_pages_locked() is the FOLL_PIN variant of get_user_pages_locked().
> > + * Behavior is the same, except that this one sets FOLL_PIN and rejects
> > + * FOLL_GET.
> > + */
> > +long pin_user_pages_locked(unsigned long start, unsigned long nr_pages,
> > + unsigned int gup_flags, struct page **pages,
> > + int *locked)
> > +{
> > + /*
> > + * FIXME: Current FOLL_LONGTERM behavior is incompatible with
> > + * FAULT_FLAG_ALLOW_RETRY because of the FS DAX check requirement on
> > + * vmas. As there are no users of this flag in this call we simply
> > + * disallow this option for now.
> > + */
> > + if (WARN_ON_ONCE(gup_flags & FOLL_LONGTERM))
> > + return -EINVAL;
> > +
> > + /* FOLL_GET and FOLL_PIN are mutually exclusive. */
> > + if (WARN_ON_ONCE(gup_flags & FOLL_GET))
> > + return -EINVAL;
> > +
> > + gup_flags |= FOLL_PIN;
>
> Right now get_user_pages_locked() doesn't have similar check for FOLL_PIN
> and also not setting FOLL_GET internally irrespective of gup_flags
> passed by user.
> Do we need to add the same in get_user_pages_locked() ?
>
> > + return __get_user_pages_locked(current, current->mm, start, nr_pages,
> > + pages, NULL, locked,
> > + gup_flags | FOLL_TOUCH);
> > +}
> > +EXPORT_SYMBOL(pin_user_pages_locked);
> > +
> > --
> > 2.26.2
> >
> >
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] mm/gup: introduce pin_user_pages_locked()
2020-05-27 22:32 ` [PATCH 1/2] mm/gup: introduce pin_user_pages_locked() John Hubbard
2020-05-28 8:43 ` David Hildenbrand
2020-05-31 7:04 ` Souptick Joarder
@ 2020-05-31 13:03 ` Pankaj Gupta
2 siblings, 0 replies; 10+ messages in thread
From: Pankaj Gupta @ 2020-05-31 13:03 UTC (permalink / raw)
To: John Hubbard; +Cc: Andrew Morton, LKML, Linux MM
Acked-by: Pankaj Gupta <pankaj.gupta.linux@gmail.com>
On Thu, 28 May 2020 at 00:32, John Hubbard <jhubbard@nvidia.com> wrote:
>
> Introduce pin_user_pages_locked(), which is nearly identical to
> get_user_pages_locked() except that it sets FOLL_PIN and rejects
> FOLL_GET.
>
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> ---
> include/linux/mm.h | 2 ++
> mm/gup.c | 30 ++++++++++++++++++++++++++++++
> 2 files changed, 32 insertions(+)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 98be7289d7e9..d16951087c93 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1700,6 +1700,8 @@ long pin_user_pages(unsigned long start, unsigned long nr_pages,
> struct vm_area_struct **vmas);
> long get_user_pages_locked(unsigned long start, unsigned long nr_pages,
> unsigned int gup_flags, struct page **pages, int *locked);
> +long pin_user_pages_locked(unsigned long start, unsigned long nr_pages,
> + unsigned int gup_flags, struct page **pages, int *locked);
> long get_user_pages_unlocked(unsigned long start, unsigned long nr_pages,
> struct page **pages, unsigned int gup_flags);
> long pin_user_pages_unlocked(unsigned long start, unsigned long nr_pages,
> diff --git a/mm/gup.c b/mm/gup.c
> index 2f0a0b497c23..17418a949067 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -2992,3 +2992,33 @@ long pin_user_pages_unlocked(unsigned long start, unsigned long nr_pages,
> return get_user_pages_unlocked(start, nr_pages, pages, gup_flags);
> }
> EXPORT_SYMBOL(pin_user_pages_unlocked);
> +
> +/*
> + * pin_user_pages_locked() is the FOLL_PIN variant of get_user_pages_locked().
> + * Behavior is the same, except that this one sets FOLL_PIN and rejects
> + * FOLL_GET.
> + */
> +long pin_user_pages_locked(unsigned long start, unsigned long nr_pages,
> + unsigned int gup_flags, struct page **pages,
> + int *locked)
> +{
> + /*
> + * FIXME: Current FOLL_LONGTERM behavior is incompatible with
> + * FAULT_FLAG_ALLOW_RETRY because of the FS DAX check requirement on
> + * vmas. As there are no users of this flag in this call we simply
> + * disallow this option for now.
> + */
> + if (WARN_ON_ONCE(gup_flags & FOLL_LONGTERM))
> + return -EINVAL;
> +
> + /* FOLL_GET and FOLL_PIN are mutually exclusive. */
> + if (WARN_ON_ONCE(gup_flags & FOLL_GET))
> + return -EINVAL;
> +
> + gup_flags |= FOLL_PIN;
> + return __get_user_pages_locked(current, current->mm, start, nr_pages,
> + pages, NULL, locked,
> + gup_flags | FOLL_TOUCH);
> +}
> +EXPORT_SYMBOL(pin_user_pages_locked);
> +
> --
> 2.26.2
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] mm/gup: introduce pin_user_pages_locked()
2020-05-31 7:04 ` Souptick Joarder
2020-05-31 7:13 ` Souptick Joarder
@ 2020-05-31 19:52 ` John Hubbard
1 sibling, 0 replies; 10+ messages in thread
From: John Hubbard @ 2020-05-31 19:52 UTC (permalink / raw)
To: Souptick Joarder; +Cc: Andrew Morton, LKML, Linux-MM
On 2020-05-31 00:04, Souptick Joarder wrote:
...
>> +/*
>> + * pin_user_pages_locked() is the FOLL_PIN variant of get_user_pages_locked().
>> + * Behavior is the same, except that this one sets FOLL_PIN and rejects
>> + * FOLL_GET.
>> + */
>> +long pin_user_pages_locked(unsigned long start, unsigned long nr_pages,
>> + unsigned int gup_flags, struct page **pages,
>> + int *locked)
>> +{
>> + /*
>> + * FIXME: Current FOLL_LONGTERM behavior is incompatible with
>> + * FAULT_FLAG_ALLOW_RETRY because of the FS DAX check requirement on
>> + * vmas. As there are no users of this flag in this call we simply
>> + * disallow this option for now.
>> + */
>> + if (WARN_ON_ONCE(gup_flags & FOLL_LONGTERM))
>> + return -EINVAL;
>> +
>> + /* FOLL_GET and FOLL_PIN are mutually exclusive. */
>> + if (WARN_ON_ONCE(gup_flags & FOLL_GET))
>> + return -EINVAL;
>> +
>> + gup_flags |= FOLL_PIN;
>
> Right now get_user_pages_locked() doesn't have similar check for FOLL_PIN
Yes, that should be added...
> and also not setting FOLL_GET internally irrespective of gup_flags
> passed by user.
> Do we need to add the same in get_user_pages_locked() ?
...and no, that should not.
Yes, it's prudent to assert that FOLL_PIN is *not* set, at all the
get_user_pages*() API calls, thanks for spotting that. I'll add that to
this patch and send out a v2.
The same check should also be added to get_user_pages_unlocked(). I'll send
out a correction (I think just a v3 of that patchset) to add that.
The setting of FOLL_GET, on the other hand, is something best left as-is
so far. Some call sites set FOLL_GET, some want it *not* set, and some
expect that FOLL_GET is implied, and at the moment, the delicate balance is
correct. :)
thanks,
--
John Hubbard
NVIDIA
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] mm/gup: introduce pin_user_pages_locked()
2020-05-31 7:13 ` Souptick Joarder
@ 2020-05-31 19:56 ` John Hubbard
0 siblings, 0 replies; 10+ messages in thread
From: John Hubbard @ 2020-05-31 19:56 UTC (permalink / raw)
To: Souptick Joarder; +Cc: Andrew Morton, LKML, Linux-MM
On 2020-05-31 00:13, Souptick Joarder wrote:
> On Sun, May 31, 2020 at 12:34 PM Souptick Joarder <jrdr.linux@gmail.com> wrote:
>>
>> On Thu, May 28, 2020 at 4:02 AM John Hubbard <jhubbard@nvidia.com> wrote:
>>>
>>> Introduce pin_user_pages_locked(), which is nearly identical to
>>> get_user_pages_locked() except that it sets FOLL_PIN and rejects
>>> FOLL_GET.
>
> Forget to ask, is it fine to add this new pin_user_pages_locked() in
> Documentation/core-api/pin_user_pages.rst ?
>
I wasn't planning on maintaining a rigorous list of all of the pin_user_page*()
API calls, in pin_user_pages.rst. Each call is documented in gup.c, for that.
The .rst is there to help explain things at a slighly higher abstraction level.
thanks,
--
John Hubbard
NVIDIA
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-05-31 19:56 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-27 22:32 [PATCH 0/2] mm/gup: introduce pin_user_pages_locked(), use it in frame_vector.c John Hubbard
2020-05-27 22:32 ` [PATCH 1/2] mm/gup: introduce pin_user_pages_locked() John Hubbard
2020-05-28 8:43 ` David Hildenbrand
2020-05-31 7:04 ` Souptick Joarder
2020-05-31 7:13 ` Souptick Joarder
2020-05-31 19:56 ` John Hubbard
2020-05-31 19:52 ` John Hubbard
2020-05-31 13:03 ` Pankaj Gupta
2020-05-27 22:32 ` [PATCH 2/2] mm/gup: frame_vector: convert get_user_pages() --> pin_user_pages() John Hubbard
2020-05-28 8:46 ` David Hildenbrand
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).