Re: [PATCH] fs/proc/task_mmu: do not warn on seeing non-migration pmd entry

["David Hildenbrand (Arm)" <david@kernel.org>]

On 6/1/26 06:56, Dev Jain wrote:
> 
> 
> 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: stable@vger.kernel.org
>>> Signed-off-by: Dev Jain <dev.jain@arm.com>
>>> ---
>>> 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.

Right, what I noted below.

Right now it's all a bit hacked on top of initial migration entry support.

>>
>> 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/20260427100553.2754667-4-usama.arif@linux.dev/
> 
> I think in addition to that, Usama can also remove the thp_migration_supported() check.
Yes, that one should go away.

-- 
Cheers,

David