Re: [PATCH 1/3] selftests/mm: check a valid fd with negative value

[David Hildenbrand <david@redhat.com>]

On 17.07.25 03:18, Wei Yang wrote:
> On Wed, Jul 16, 2025 at 10:52:21AM +0200, David Hildenbrand wrote:
>> On 16.07.25 10:27, Wei Yang wrote:
>>> fd 0 is usually occupied by stdin, which is not expected to be returned
>>> by open(). But 0 is still a valid fd.
>>>
>>> Check the valid fd with negative value like other places.
>>>
>>> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>>> Cc: David Hildenbrand <david@redhat.com>
>>> Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
>>> Cc: Rik van Riel <riel@surriel.com>
>>> Cc: Liam R. Howlett <Liam.Howlett@oracle.com>
>>> Cc: Vlastimil Babka <vbabka@suse.cz>
>>> Cc: Harry Yoo <harry.yoo@oracle.com>
>>> ---
>>>    tools/testing/selftests/mm/ksm_functional_tests.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/tools/testing/selftests/mm/ksm_functional_tests.c b/tools/testing/selftests/mm/ksm_functional_tests.c
>>> index d8bd1911dfc0..6da5c3340e10 100644
>>> --- a/tools/testing/selftests/mm/ksm_functional_tests.c
>>> +++ b/tools/testing/selftests/mm/ksm_functional_tests.c
>>> @@ -81,7 +81,7 @@ static long get_my_ksm_zero_pages(void)
>>>    	ssize_t read_size;
>>>    	unsigned long my_ksm_zero_pages;
>>> -	if (!proc_self_ksm_stat_fd)
>>> +	if (proc_self_ksm_stat_fd < 0)
>>>    		return 0;
>>
>> Only test_unmerge_zero_pages() calls get_my_ksm_zero_pages(). And there, we
>> have
>>
>> if (proc_self_ksm_stat_fd < 0) {
>> 	ksft_test_result_skip("open(\"/proc/self/ksm_stat\") failed\n");
>> 	return;
>> }
>>
>> As the fd is opened after others, we should never get fd=0.
>>
> 
> You are right.
> 
>> So probably, this should just be handled as part of factoring it out, no need
>> for this change upfront AFAIKS.
>>
> 
> My intention of this change is, we plan to move get_my_ksm_zero_pages() into a
> standalone helper and will open and check the fd there. To make following
> change consistent, I split it out.
> 
> If you think it is not necessary, will drop this.

IMHO, just cleaning it up while factoring out is better.

-- 
Cheers,

David / dhildenb