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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <59099cb0-638b-4ca7-b20e-f407a2d27217@gmail.com>
Date: Tue, 29 Jul 2025 23:17:52 +0100
From: Usama Arif <usamaarif642@...il.com>
To: SeongJae Park <sj@...nel.org>
Cc: Andrew Morton <akpm@...ux-foundation.org>, david@...hat.com,
 linux-mm@...ck.org, linux-fsdevel@...r.kernel.org, corbet@....net,
 rppt@...nel.org, surenb@...gle.com, mhocko@...e.com, hannes@...xchg.org,
 baohua@...nel.org, shakeel.butt@...ux.dev, riel@...riel.com, ziy@...dia.com,
 laoar.shao@...il.com, dev.jain@....com, baolin.wang@...ux.alibaba.com,
 npache@...hat.com, lorenzo.stoakes@...cle.com, Liam.Howlett@...cle.com,
 ryan.roberts@....com, vbabka@...e.cz, jannh@...gle.com,
 Arnd Bergmann <arnd@...db.de>, linux-kernel@...r.kernel.org,
 linux-doc@...r.kernel.org, kernel-team@...a.com
Subject: Re: [PATCH 5/5] selftests: prctl: introduce tests for disabling THPs
 except for madvise


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