>> + 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
