On 25/02/2025 22:18, Will Deacon wrote:
> 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?
Yep, I'll aim to post this today. I have a few review comments for s390 to add
in too.
>
> Cheers,
>
> Will
