2024-06-11 18:29:28

by Martin Oliveira

[permalink] [raw]
Subject: [PATCH v2 2/4] mm/gup: handle ZONE_DEVICE pages in folio_fast_pin_allowed()

folio_fast_pin_allowed() does not support ZONE_DEVICE pages because
currently it is impossible for that type of page to be used with
FOLL_LONGTERM. When this changes in a subsequent patch, this path will
attempt to read the mapping of a ZONE_DEVICE page which is not valid.

Instead, allow ZONE_DEVICE pages explicitly seeing they shouldn't pose
any problem with the fast path.

Co-developed-by: Logan Gunthorpe <[email protected]>
Signed-off-by: Logan Gunthorpe <[email protected]>
Signed-off-by: Martin Oliveira <[email protected]>
---
mm/gup.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/mm/gup.c b/mm/gup.c
index ca0f5cedce9b2..00d0a77112f4f 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -2847,6 +2847,10 @@ static bool gup_fast_folio_allowed(struct folio *folio, unsigned int flags)
if (folio_test_hugetlb(folio))
return true;

+ /* It makes no sense to access the mapping of ZONE_DEVICE pages */
+ if (folio_is_zone_device(folio))
+ return true;
+
/*
* GUP-fast disables IRQs. When IRQS are disabled, RCU grace periods
* cannot proceed, which means no actions performed under RCU can
--
2.43.0



2024-06-15 02:40:29

by John Hubbard

[permalink] [raw]
Subject: Re: [PATCH v2 2/4] mm/gup: handle ZONE_DEVICE pages in folio_fast_pin_allowed()

On 6/11/24 11:27 AM, Martin Oliveira wrote:
> folio_fast_pin_allowed() does not support ZONE_DEVICE pages because

s/folio_fast_pin_allowed/gup_fast_folio_allowed/ ?

> currently it is impossible for that type of page to be used with
> FOLL_LONGTERM. When this changes in a subsequent patch, this path will
> attempt to read the mapping of a ZONE_DEVICE page which is not valid.
>
> Instead, allow ZONE_DEVICE pages explicitly seeing they shouldn't pose
> any problem with the fast path.
>
> Co-developed-by: Logan Gunthorpe <[email protected]>
> Signed-off-by: Logan Gunthorpe <[email protected]>
> Signed-off-by: Martin Oliveira <[email protected]>
> ---
> mm/gup.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/mm/gup.c b/mm/gup.c
> index ca0f5cedce9b2..00d0a77112f4f 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -2847,6 +2847,10 @@ static bool gup_fast_folio_allowed(struct folio *folio, unsigned int flags)
> if (folio_test_hugetlb(folio))
> return true;
>
> + /* It makes no sense to access the mapping of ZONE_DEVICE pages */

This comment is very difficult, because it states that one cannot
do something, right before explicitly enable something else. And the
reader is given little help on connecting the two.

And there are several subtypes of ZONE_DEVICE. Is it really true that
none of them can be mapped to user space? For p2p BAR1 mappings, those
actually go to user space, yes? Confused, need help. :)

> + if (folio_is_zone_device(folio))
> + return true;
> +
> /*
> * GUP-fast disables IRQs. When IRQS are disabled, RCU grace periods
> * cannot proceed, which means no actions performed under RCU can

thanks,
--
John Hubbard
NVIDIA