Re: [PATCH 4/5] mm/gup.c: Reorganize try_get_folio()

[Matthew Wilcox <willy@xxxxxxxxxxxxx>]

On Tue, Jun 13, 2023 at 01:18:26PM -0700, Vishal Moola (Oracle) wrote:
>  struct folio *try_grab_folio(struct page *page, int refs, unsigned int flags)
>  {
> +	struct folio *folio;

checkpatch will whinge about there not being a blank line here, and in
this case, I think it's correct.

>  	if (unlikely(!(flags & FOLL_PCI_P2PDMA) && is_pci_p2pdma_page(page)))
>  		return NULL;
>  
> +	folio = try_get_folio(page, refs);
> +
>  	if (flags & FOLL_GET)
> -		return try_get_folio(page, refs);
> -	else if (flags & FOLL_PIN) {
> -		struct folio *folio;
> +		return folio;
>  
> -		/*
> -		 * Can't do FOLL_LONGTERM + FOLL_PIN gup fast path if not in a
> -		 * right zone, so fail and let the caller fall back to the slow
> -		 * path.
> -		 */
> -		if (unlikely((flags & FOLL_LONGTERM) &&
> -			     !is_longterm_pinnable_page(page)))
> -			return NULL;
> +	if (unlikely(!(flags & FOLL_PIN))) {
> +		WARN_ON_ONCE(1);
> +		return NULL;

Don't we need to folio_put_refs() in this case?  Or rather, I think the

	if (WARN_ON_ONCE(flags & (FOLL_PIN|FOLL_GET) == 0) {

test should be first.

> +	/*
> +	 * CAUTION: Don't use compound_head() on the page before this
> +	 * point, the result won't be stable.
> +	 */

I think we can lose the comment at this point?

> +	if (!folio)
> +		return NULL;