Re: [PATCH v10 3/3] ksm: add mremap selftests for ksm_rmap_walk

[David Hildenbrand (Arm)]

On 6/29/26 16:43, xu.xin16@xxxxxxxxxx wrote:
>> I think there are various improvements and simplifications we can perform. In particular,
>>  I don't think we need the errors messages or use data-> members.
>>
>> What about the following simplification, to move this over the finishing line? (untested)
> 
> Agreed. Your version is cleaner and tests successfully.
> 
>>
>> There is the low chance of page compaction migrating the page while we check for it. Not sure 
>> if we should handle it (but it would involve retrying on PFN mismatch).
> 
> It is really possible. To improve robustness, how about double-checking in range_maps_pfn() and
> rename range_maps_pfn as range_maps_the_same_pfn() which caculate the fisrt PFN itself without
> the input PFN as follows:
> 
> static bool range_maps_the_same_pfn(int pagemap_fd, void *region, int nr_pages)
> {
>         int i;
>         int second_times = 0;
>         unsigned long first_pfn;
> 
> again:
>         first_pfn = pagemap_get_pfn(pagemap_fd, region);
>         for (i = 0; i < nr_pages; i++) {
>                 if (pagemap_get_pfn(pagemap_fd, region + i * getpagesize()) != first_pfn) {
>                         if (second_times)
>                                 return false;
>                         else {
>                                 /*
>                                  * In case of low chance the low chance of page compaction
>                                  * migrating the page while we check for pfn.
>                                  */
>                                 second_times++;
>                                 goto again;
>                         }
>                 }
>         }
> 
>         return true;
> }

Something like that could work. Best to retry more often (10 times?) and call
the variable "retries". Also, call the label "retry"

Simply check after the retry label

if (retries > 10)
	return false;

-- 
Cheers,

David