Re: [PATCH v4 08/14] mm/gup: grab head page refcount once for group of subpages

[Jason Gunthorpe <jgg@ziepe.ca>]

On Fri, Aug 27, 2021 at 03:58:13PM +0100, Joao Martins wrote:

>  #if defined(CONFIG_ARCH_HAS_PTE_DEVMAP) && defined(CONFIG_TRANSPARENT_HUGEPAGE)
>  static int __gup_device_huge(unsigned long pfn, unsigned long addr,
>  			     unsigned long end, unsigned int flags,
>  			     struct page **pages, int *nr)
>  {
> -	int nr_start = *nr;
> +	int refs, nr_start = *nr;
>  	struct dev_pagemap *pgmap = NULL;
>  	int ret = 1;
>  
>  	do {
> -		struct page *page = pfn_to_page(pfn);
> +		struct page *head, *page = pfn_to_page(pfn);
> +		unsigned long next = addr + PAGE_SIZE;
>  
>  		pgmap = get_dev_pagemap(pfn, pgmap);
>  		if (unlikely(!pgmap)) {
> @@ -2252,16 +2265,25 @@ static int __gup_device_huge(unsigned long pfn, unsigned long addr,
>  			ret = 0;
>  			break;
>  		}
> -		SetPageReferenced(page);
> -		pages[*nr] = page;
> -		if (unlikely(!try_grab_page(page, flags))) {
> -			undo_dev_pagemap(nr, nr_start, flags, pages);
> +
> +		head = compound_head(page);
> +		/* @end is assumed to be limited at most one compound page */
> +		if (PageHead(head))
> +			next = end;
> +		refs = record_subpages(page, addr, next, pages + *nr);
> +
> +		SetPageReferenced(head);
> +		if (unlikely(!try_grab_compound_head(head, refs, flags))) {
> +			if (PageHead(head))
> +				ClearPageReferenced(head);
> +			else
> +				undo_dev_pagemap(nr, nr_start, flags, pages);
>  			ret = 0;
>  			break;

Why is this special cased for devmap?

Shouldn't everything processing pud/pmds/etc use the same basic loop
that is similar in idea to the 'for_each_compound_head' scheme in
unpin_user_pages_dirty_lock()?

Doesn't that work for all the special page type cases here?

Jason