Subject: [PATCH v3 03/10] mm/gup: fail get_user_pages for LONGTERM dev coherent type

Avoid long term pinning for Coherent device type pages. This could
interfere with their own device memory manager. For now, we are just
returning error for PIN_LONGTERM Coherent device type pages. Eventually,
these type of pages will get migrated to system memory, once the device
migration pages support is added.

Signed-off-by: Alex Sierra <[email protected]>
---
mm/gup.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/mm/gup.c b/mm/gup.c
index 886d6148d3d0..9c8a075d862d 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -1720,6 +1720,12 @@ static long check_and_migrate_movable_pages(unsigned long nr_pages,
* If we get a movable page, since we are going to be pinning
* these entries, try to move them out if possible.
*/
+ if (is_device_page(head)) {
+ WARN_ON_ONCE(is_device_private_page(head));
+ ret = -EFAULT;
+ goto unpin_pages;
+ }
+
if (!is_pinnable_page(head)) {
if (PageHuge(head)) {
if (!isolate_huge_page(head, &movable_page_list))
@@ -1750,6 +1756,7 @@ static long check_and_migrate_movable_pages(unsigned long nr_pages,
if (list_empty(&movable_page_list) && !isolation_error_count)
return nr_pages;

+unpin_pages:
if (gup_flags & FOLL_PIN) {
unpin_user_pages(pages, nr_pages);
} else {
--
2.32.0



2022-01-21 21:59:48

by Joao Martins

[permalink] [raw]
Subject: Re: [PATCH v3 03/10] mm/gup: fail get_user_pages for LONGTERM dev coherent type

On 1/10/22 22:31, Alex Sierra wrote:
> Avoid long term pinning for Coherent device type pages. This could
> interfere with their own device memory manager. For now, we are just
> returning error for PIN_LONGTERM Coherent device type pages. Eventually,
> these type of pages will get migrated to system memory, once the device
> migration pages support is added.
>
> Signed-off-by: Alex Sierra <[email protected]>
> ---
> mm/gup.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/mm/gup.c b/mm/gup.c
> index 886d6148d3d0..9c8a075d862d 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -1720,6 +1720,12 @@ static long check_and_migrate_movable_pages(unsigned long nr_pages,
> * If we get a movable page, since we are going to be pinning
> * these entries, try to move them out if possible.
> */
> + if (is_device_page(head)) {
> + WARN_ON_ONCE(is_device_private_page(head));
> + ret = -EFAULT;
> + goto unpin_pages;
> + }
> +

Wouldn't be more efficient for you failing earlier instead of after all the pages are pinned?

Filesystem DAX suffers from a somewhat similar issue[0] -- albeit it's more related to
blocking FOLL_LONGTERM in gup-fast while gup-slow can still do it. Coherent devmap appears
to want to block it in all gup.

On another thread Jason was suggesting about having different pgmap::flags to capture
these special cases[1] instead of selecting what different pgmap types can do in various
different places.

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

2022-01-21 22:07:10

by Alistair Popple

[permalink] [raw]
Subject: Re: [PATCH v3 03/10] mm/gup: fail get_user_pages for LONGTERM dev coherent type

On Thursday, 20 January 2022 11:36:21 PM AEDT Joao Martins wrote:
> On 1/10/22 22:31, Alex Sierra wrote:
> > Avoid long term pinning for Coherent device type pages. This could
> > interfere with their own device memory manager. For now, we are just
> > returning error for PIN_LONGTERM Coherent device type pages. Eventually,
> > these type of pages will get migrated to system memory, once the device
> > migration pages support is added.
> >
> > Signed-off-by: Alex Sierra <[email protected]>
> > ---
> > mm/gup.c | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/mm/gup.c b/mm/gup.c
> > index 886d6148d3d0..9c8a075d862d 100644
> > --- a/mm/gup.c
> > +++ b/mm/gup.c
> > @@ -1720,6 +1720,12 @@ static long check_and_migrate_movable_pages(unsigned long nr_pages,
> > * If we get a movable page, since we are going to be pinning
> > * these entries, try to move them out if possible.
> > */
> > + if (is_device_page(head)) {
> > + WARN_ON_ONCE(is_device_private_page(head));
> > + ret = -EFAULT;
> > + goto unpin_pages;
> > + }
> > +
>
> Wouldn't be more efficient for you failing earlier instead of after all the pages are pinned?

Rather than failing I think the plan is to migrate the device coherent pages
like we do for ZONE_MOVABLE, so leaving this here is a good place holder until
that is done. Currently we are missing some functionality required to do that
but I am hoping to post a series fixing that soon.

> Filesystem DAX suffers from a somewhat similar issue[0] -- albeit it's more related to
> blocking FOLL_LONGTERM in gup-fast while gup-slow can still do it. Coherent devmap appears
> to want to block it in all gup.
>
> On another thread Jason was suggesting about having different pgmap::flags to capture
> these special cases[1] instead of selecting what different pgmap types can do in various
> different places.
>
> [0] https://lore.kernel.org/linux-mm/[email protected]/
> [1] https://lore.kernel.org/linux-mm/[email protected]/
>