On 9/15/25 18:10, David Hildenbrand wrote:
>>>> +
>>>
>>> Wrong indentation.
>>
>> checkpatch.pl did not complain, what I see is
>>
>> + VM_WARN_ON(!is_pmd_migration_entry(pmd) &&
>> + !is_pmd_device_private_entry(pmd));
>>
>
> David complains :)
>
>> It looks different in your reply (is your email converting tabs to space?
>> did you want me to align the conditions?
>
> Indeed, looks wrong, not sure why. Likely I messed it up.
>
Ack
>>
>> + VM_WARN_ON(!is_pmd_migration_entry(pmd) &&
>> + !is_pmd_device_private_entry(pmd));
>>
>>
>
> Exactly.
>
>>
>>>
>>>> + if (is_migration_entry(entry) &&
>>>> + !is_readable_migration_entry(entry)) {
>>>
>>> Dito.
>>>
>>
>> Same as above :)
>>
>>
>>> Wonder if we want to be more explicit.
>>>
>>> if (is_readable_migration_entry(enrty) ||
>>> is_readable_exclusive_migration_entry)) {
>>>
>>
>> !is_readable_migration_entry => writable entry or read exclusive, did you
>> mean is_writable_migration_entry()
>> above?
>
> Yes, sorry, my brain was dizzy after all the review lately.
>
> if (is_writable_migration_entry(enrty) ||
> is_readable_exclusive_migration_entry))
>
> [...]
>
Ack
>>>
>>> Couldn't we do here
>>>
>>> if (!pmd_present(pmdval))
>>> goto nomap;
>>>
>>> To replace the original pmd_none() .. check.
>>>
>>> A page table must always be present IIRC.
>>>
>>
>> I am not sure about the pmd_none(), a page table may not be present, I've
>> not audited
>> the callers. But I think we can do
>
> IIRC page tables must always have the present bit set. So we can just
> simplify to the single pmd_present() check.
>
Not sure about that one, are you happy if we follow it up later with a separate
fix
Balbir
