Re: [PATCH 5/5] selftests: prctl: introduce tests for disabling THPs except for madvise

[Usama Arif <usamaarif642@gmail.com>]

>> +	res = test_mmap_thp(COLLAPSE, pmdsize);
>> +	ASSERT_EQ(res, 1);
> 
> Seems res is not being used other than saving the return value for assertions.
> Why don't you do the assertion at once, e.g., ASSERT_EQ(test_mmap_thp(...), 1)?
> No strong opinion, but I think that could make code shorter and easier to read.
> 

Yeah this is on purpose, I found it easier to evaluate what the output should be
if I wrote code like this. As compiler will likely just optimize it out (although
we dont really care about performance :) ), although others might find without res
better.
I think you had mentioned it in the earlier thread as well. I will remove it.>> +}
>> +
>> +TEST_F(prctl_thp_disable_except_madvise, nofork)
>> +{
>> +	int res = 0;
>> +
>> +	res = prctl(PR_SET_THP_DISABLE, 1, PR_THP_DISABLE_EXCEPT_ADVISED, NULL, NULL);
>> +	ASSERT_EQ(res, 0);
> 
> Again, I think 'res' can be removed.
> 
>> +	prctl_thp_disable_except_madvise(_metadata, self->pmdsize);
>> +}
>> +
>> +TEST_F(prctl_thp_disable_except_madvise, fork)
>> +{
>> +	int res = 0, ret = 0;
>> +	pid_t pid;
>> +
>> +	res = prctl(PR_SET_THP_DISABLE, 1, PR_THP_DISABLE_EXCEPT_ADVISED, NULL, NULL);
>> +	ASSERT_EQ(res, 0);
> 
> Ditto.
> 
>> +
>> +	/* Make sure prctl changes are carried across fork */
>> +	pid = fork();
>> +	ASSERT_GE(pid, 0);
>> +
>> +	if (!pid)
>> +		prctl_thp_disable_except_madvise(_metadata, self->pmdsize);
>> +
>> +	wait(&ret);
>> +	if (WIFEXITED(ret))
>> +		ret = WEXITSTATUS(ret);
>> +	else
>> +		ret = -EINVAL;
>> +	ASSERT_EQ(ret, 0);
>> +}
>> +
>>  FIXTURE(prctl_thp_disable_completely)
>>  {
>>  	struct thp_settings settings;
>> -- 
>> 2.47.3
> 
> 
> Thanks,
> SJ