Re: [PATCH 5/7] gup: Use folios for gup_devmap

[David Hildenbrand <david@redhat.com>]

On 24.04.24 21:19, Matthew Wilcox (Oracle) wrote:

Nit: s/gup_devmap/gup_fast_devmap/

> Use try_grab_folio() instead of try_grab_page() so we get the folio
> back that we calculated, and then use folio_set_referenced() instead
> of SetPageReferenced().  Correspondingly, use gup_put_folio() to
> put any unneeded references.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>   mm/gup.c | 17 ++++++++---------
>   1 file changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/mm/gup.c b/mm/gup.c
> index 49376f756936..e4cc12b8e985 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -2877,13 +2877,10 @@ static void __maybe_unused gup_fast_undo_dev_pagemap(int *nr, int nr_start,
>   		unsigned int flags, struct page **pages)
>   {
>   	while ((*nr) - nr_start) {
> -		struct page *page = pages[--(*nr)];
> +		struct folio *folio = page_folio(pages[--(*nr)]);
>   
> -		ClearPageReferenced(page);

I stumbled over that likely unwarranted ClearPageReferenced() recently 
as well: what if the page was already referenced before we called 
SetPageReferenced?

> -		if (flags & FOLL_PIN)
> -			unpin_user_page(page);
> -		else
> -			put_page(page);
> +		folio_clear_referenced(folio);
> +		gup_put_folio(folio, 1, flags);

For !FOLL_PIN, we wouldn't have done the
	if (!put_devmap_managed_page_refs(&folio->page, refs))
		folio_put_refs(folio, refs);

Magic in gup_put_folio()

... was that a BUG?


This devmap crap is so confusing.

-- 
Cheers,

David / dhildenb