On 01/06/26 12:41 am, David Hildenbrand (Arm) wrote:
> On 5/29/26 13:17, Dev Jain wrote:
>> pagemap_pmd_range_thp() warns if a non-present PMD is not a migration
>> entry. This became false once device-private entries at the PMD level were
>> added.
>>
>> One can hit the warning by patching hmm-tests.c with the following:
>>
>> diff --git a/tools/testing/selftests/mm/hmm-tests.c
>> b/tools/testing/selftests/mm/hmm-tests.c
>> index e1c8a679a4cf3..7f0a3384f3c5f 100644
>> --- a/tools/testing/selftests/mm/hmm-tests.c
>> +++ b/tools/testing/selftests/mm/hmm-tests.c
>> @@ -209,6 +209,37 @@ static int hmm_dmirror_cmd(int fd,
>> return 0;
>> }
>>
>> +static int hmm_read_self_pagemap(void *addr, unsigned long npages,
>> + unsigned long page_size)
>> +{
>> + const size_t entry_size = sizeof(uint64_t);
>> + const off_t offset = ((uintptr_t)addr / page_size) * entry_size;
>> + uint64_t *entries;
>> + ssize_t nread;
>> + int fd;
>> +
>> + entries = malloc(npages * entry_size);
>> + if (!entries)
>> + return -ENOMEM;
>> +
>> + fd = open("/proc/self/pagemap", O_RDONLY);
>> + if (fd < 0) {
>> + free(entries);
>> + return -errno;
>> + }
>> +
>> + nread = pread(fd, entries, npages * entry_size, offset);
>> + close(fd);
>> + free(entries);
>> +
>> + if (nread < 0)
>> + return -errno;
>> + if ((size_t)nread != npages * entry_size)
>> + return -EIO;
>> +
>> + return 0;
>> +}
>> +
>> static void hmm_buffer_free(struct hmm_buffer *buffer)
>> {
>> if (buffer == NULL)
>> @@ -2314,6 +2345,10 @@ TEST_F(hmm, migrate_anon_huge_fault)
>> ASSERT_EQ(ret, 0);
>> ASSERT_EQ(buffer->cpages, npages);
>>
>> + /* Exercise pagemap on a PMD device-private entry. */
>> + ret = hmm_read_self_pagemap(buffer->ptr, npages, self->page_size);
>> + ASSERT_EQ(ret, 0);
>> +
>> /* Check what the device read. */
>> for (i = 0, ptr = buffer->mirror; i < size / sizeof(*ptr); ++i)
>> ASSERT_EQ(ptr[i], i);
>>
>>
>
>
>> Therefore, remove the stale migration-only assertion.
>>
>> Fixes: a30b48bf1b24 ("mm/migrate_device: implement THP migration of zone
>> device pages")
>> Cc: [email protected]
>> Signed-off-by: Dev Jain <[email protected]>
>> ---
>> Applies on mm-unstable (404fb4f38e8f).
>>
>> fs/proc/task_mmu.c | 1 -
>> 1 file changed, 1 deletion(-)
>>
>> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
>> index 1e3a15bf46f4e..58938e62154d9 100644
>> --- a/fs/proc/task_mmu.c
>> +++ b/fs/proc/task_mmu.c
>> @@ -2129,7 +2129,6 @@ static int pagemap_pmd_range_thp(pmd_t *pmdp, unsigned
>> long addr,
>> flags |= PM_SOFT_DIRTY;
>> if (pmd_swp_uffd_wp(pmd))
>> flags |= PM_UFFD_WP;
>> - VM_WARN_ON_ONCE(!pmd_is_migration_entry(pmd));
>> page = softleaf_to_page(entry);
>> }
>>
>
> The whole thp_migration_supported() guard is a bit shaky, right?
I think if you remove this, then you will trigger a WARN_ON in
softleaf_to_page(),
for the case of !CONFIG_ARCH_ENABLE_THP_MIGRATION.
>
> I guess device-private entries currently imply thp_migration_supported(), but
> that thp_migration_supported() check is really questionable and should likely
> just go away (else if -> else).
>
> Staring at pte_to_pagemap_entry(), likely we'd also want
>
> if (softleaf_has_pfn(entry))
> page = softleaf_to_page(entry);
>
> to prepare for PMD swap entries.
Correct, and this is done in
https://lore.kernel.org/all/[email protected]/
I think in addition to that, Usama can also remove the
thp_migration_supported() check.
>
>
> Anyhow, both are unrelated (can you send patches to clean it up?)
>
> Acked-by: David Hildenbrand (Arm) <[email protected]>
>
