linux-kernel - Re: [PATCH 4/5] mm/gup.c: Reorganize try_get

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 13 Jun 2023 21:29:27 +0100
From:   Matthew Wilcox <willy@...radead.org>
To:     "Vishal Moola (Oracle)" <vishal.moola@...il.com>
Cc:     akpm@...ux-foundation.org, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 4/5] mm/gup.c: Reorganize try_get_folio()

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;