Re: [PATCH v2 2/4] arm64: hugetlb: Fix huge_ptep_get_and_clear() for non-present ptes

[Will Deacon <will@kernel.org>]

On Mon, Feb 24, 2025 at 12:11:19PM +0000, Ryan Roberts wrote:
> On 21/02/2025 15:31, Will Deacon wrote:
> > On Mon, Feb 17, 2025 at 02:04:15PM +0000, Ryan Roberts wrote:
> >> +	pte = __ptep_get_and_clear(mm, addr, ptep);
> >> +	present = pte_present(pte);
> >> +	while (--ncontig) {
> >> +		ptep++;
> >> +		addr += pgsize;
> >> +		tmp_pte = __ptep_get_and_clear(mm, addr, ptep);
> >> +		if (present) {
> >> +			if (pte_dirty(tmp_pte))
> >> +				pte = pte_mkdirty(pte);
> >> +			if (pte_young(tmp_pte))
> >> +				pte = pte_mkyoung(pte);
> >> +		}
> >>  	}
> > 
> > nit: With the loop now structured like this, we really can't handle
> > num_contig_ptes() returning 0 if it gets an unknown size. Granted, that
> > really shouldn't happen, but perhaps it would be better to add a 'default'
> > case with a WARN() to num_contig_ptes() and then add an early return here?
> 
> Looking at other users of num_contig_ptes() it looks like huge_ptep_get()
> already assumes at least 1 pte (it calls __ptep_get() before calling
> num_contig_ptes()) and set_huge_pte_at() assumes 1 pte for the "present and
> non-contig" case. So num_contig_ptes() returning 0 is already not really
> consumed consistently.
> 
> How about we change the default num_contig_ptes() return value to 1 and add a
> warning if size is invalid:

Fine by me!

I assume you'll fold that in and send a new version, along with the typo
fixes?

Cheers,

Will