Hello Vivek and Andrew,
On Sun, 23 Jun 2024 23:36:11 -0700 Vivek Kasireddy <vivek.kasire...@intel.com>
wrote:
> For drivers that would like to longterm-pin the folios associated
> with a memfd, the memfd_pin_folios() API provides an option to
> not only pin the folios via FOLL_PIN but also to check and migrate
> them if they reside in movable zone or CMA block. This API
> currently works with memfds but it should work with any files
> that belong to either shmemfs or hugetlbfs. Files belonging to
> other filesystems are rejected for now.
>
> The folios need to be located first before pinning them via FOLL_PIN.
> If they are found in the page cache, they can be immediately pinned.
> Otherwise, they need to be allocated using the filesystem specific
> APIs and then pinned.
>
> Cc: David Hildenbrand <da...@redhat.com>
> Cc: Matthew Wilcox (Oracle) <wi...@infradead.org>
> Cc: Christoph Hellwig <h...@infradead.org>
> Cc: Daniel Vetter <daniel.vet...@ffwll.ch>
> Cc: Hugh Dickins <hu...@google.com>
> Cc: Peter Xu <pet...@redhat.com>
> Cc: Gerd Hoffmann <kra...@redhat.com>
> Cc: Dongwon Kim <dongwon....@intel.com>
> Cc: Junxiao Chang <junxiao.ch...@intel.com>
> Suggested-by: Jason Gunthorpe <j...@nvidia.com>
> Reviewed-by: Jason Gunthorpe <j...@nvidia.com> (v2)
> Reviewed-by: David Hildenbrand <da...@redhat.com> (v3)
> Reviewed-by: Christoph Hellwig <h...@lst.de> (v6)
> Acked-by: Dave Airlie <airl...@redhat.com>
> Acked-by: Gerd Hoffmann <kra...@redhat.com>
> Signed-off-by: Vivek Kasireddy <vivek.kasire...@intel.com>
> ---
> include/linux/memfd.h | 5 ++
> include/linux/mm.h | 3 +
> mm/gup.c | 137 ++++++++++++++++++++++++++++++++++++++++++
> mm/memfd.c | 45 ++++++++++++++
> 4 files changed, 190 insertions(+)
>
[...]
> diff --git a/mm/gup.c b/mm/gup.c
> index a88e19c78730..94160abbf499 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
[...]
> @@ -3747,3 +3749,138 @@ long pin_user_pages_unlocked(unsigned long start,
> unsigned long nr_pages,
> &locked, gup_flags);
> }
> EXPORT_SYMBOL(pin_user_pages_unlocked);
> +
> +/**
> + * memfd_pin_folios() - pin folios associated with a memfd
[...]
> + for (i = 0; i < nr_found; i++) {
> + /*
> + * As there can be multiple entries for a
> + * given folio in the batch returned by
> + * filemap_get_folios_contig(), the below
> + * check is to ensure that we pin and return a
> + * unique set of folios between start and end.
> + */
> + if (next_idx &&
> + next_idx != folio_index(fbatch.folios[i]))
> + continue;
> +
> + folio = try_grab_folio(&fbatch.folios[i]->page,
> + 1, FOLL_PIN);
> + if (!folio) {
> + folio_batch_release(&fbatch);
> + ret = -EINVAL;
> + goto err;
> + }
I found this patch is applied on mm-unstable as commit 7618d1ff59ef ("mm/gup:
introduce memfd_pin_folios() for pinning memfd folios"). Somehow, however, the
commit has changd the above try_grab_folio() call to try_grab_folio_fast()
call.
As a result, building kernel without CONFIG_MMU fais as below:
CC mm/gup.o
mm/gup.c: In function 'memfd_pin_folios':
mm/gup.c:3862:41: error: implicit declaration of function
'try_grab_folio_fast'; did you mean 'try_grab_folio'?
[-Werror=implicit-function-declaration]
3862 | folio =
try_grab_folio_fast(&fbatch.folios[i]->page,
| ^~~~~~~~~~~~~~~~~~~
| try_grab_folio
mm/gup.c:3862:39: warning: assignment to 'struct folio *' from 'int' makes
pointer from integer without a cast [-Wint-conversion]
3862 | folio =
try_grab_folio_fast(&fbatch.folios[i]->page,
| ^
But simply changing the call back to try_grab_folio() causes another failure:
CC mm/gup.o
mm/gup.c: In function 'memfd_pin_folios':
mm/gup.c:3862:56: error: passing argument 1 of 'try_grab_folio' from
incompatible pointer type [-Werror=incompatible-pointer-types]
3862 | folio =
try_grab_folio(&fbatch.folios[i]->page,
|
^~~~~~~~~~~~~~~~~~~~~~~
| |
| struct page *
mm/gup.c:141:47: note: expected 'struct folio *' but argument is of type
'struct page *'
141 | int __must_check try_grab_folio(struct folio *folio, int refs,
| ~~~~~~~~~~~~~~^~~~~
mm/gup.c:3862:39: warning: assignment to 'struct folio *' from 'int' makes
pointer from integer without a cast [-Wint-conversion]
3862 | folio =
try_grab_folio(&fbatch.folios[i]->page,
| ^
Maybe the change has made to fix conflict with another mm-unstable commit
02a2d55767d1 ("mm: gup: stop abusing try_grab_folio"), but forgot the
CONFIG_MMU unset case?
I confirmed the failure disappears after further cleanup like below:
diff --git a/mm/gup.c b/mm/gup.c
index 46a266ed84f7..9f4902425070 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -3859,9 +3859,9 @@ long memfd_pin_folios(struct file *memfd, loff_t start,
loff_t end,
next_idx != folio_index(fbatch.folios[i]))
continue;
- folio =
try_grab_folio_fast(&fbatch.folios[i]->page,
- 1, FOLL_PIN);
- if (!folio) {
+ folio = page_folio(&fbatch.folios[i]->page);
+
+ if (try_grab_folio(folio, 1, FOLL_PIN)) {
folio_batch_release(&fbatch);
ret = -EINVAL;
goto err;
I didn't look deep into the patch, so unsure if that's a valid fix, though.
May I ask your thoughts?
Thanks,
SJ
[...]
