* [PATCH v3 1/5] selftests/mm: hugetlb_reparenting_test: do not unmount
2024-01-25 15:46 [PATCH v3 0/5] selftests/mm: Improve run_vmtests.sh Muhammad Usama Anjum
@ 2024-01-25 15:46 ` Muhammad Usama Anjum
2024-01-25 15:46 ` [PATCH v3 2/5] selftests/mm: run_vmtests: remove sudo and conform to tap Muhammad Usama Anjum
` (3 subsequent siblings)
4 siblings, 0 replies; 14+ messages in thread
From: Muhammad Usama Anjum @ 2024-01-25 15:46 UTC (permalink / raw)
To: Andrew Morton, Shuah Khan
Cc: Muhammad Usama Anjum, kernel, Ryan Roberts, linux-mm,
linux-kselftest, linux-kernel
Do not unmount the cgroup if it wasn't mounted by the test. The earlier
patch had fixed this for charge_reserved_hugetlb, but not for this test.
I'm adding fixes tag to that earlier patch.
Fixes: 209376ed2a84 ("selftests/vm: make charge_reserved_hugetlb.sh work with existing cgroup setting")
Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
tools/testing/selftests/mm/hugetlb_reparenting_test.sh | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/mm/hugetlb_reparenting_test.sh b/tools/testing/selftests/mm/hugetlb_reparenting_test.sh
index 14d26075c8635..615c4d766c909 100755
--- a/tools/testing/selftests/mm/hugetlb_reparenting_test.sh
+++ b/tools/testing/selftests/mm/hugetlb_reparenting_test.sh
@@ -248,5 +248,7 @@ cleanup
echo ALL PASS
-umount $CGROUP_ROOT
-rm -rf $CGROUP_ROOT
+if [[ $do_umount ]]; then
+ umount $CGROUP_ROOT
+ rm -rf $CGROUP_ROOT
+fi
--
2.42.0
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH v3 2/5] selftests/mm: run_vmtests: remove sudo and conform to tap
2024-01-25 15:46 [PATCH v3 0/5] selftests/mm: Improve run_vmtests.sh Muhammad Usama Anjum
2024-01-25 15:46 ` [PATCH v3 1/5] selftests/mm: hugetlb_reparenting_test: do not unmount Muhammad Usama Anjum
@ 2024-01-25 15:46 ` Muhammad Usama Anjum
2024-02-01 12:04 ` Ryan Roberts
2024-01-25 15:46 ` [PATCH v3 3/5] selftests/mm: save and restore nr_hugepages value Muhammad Usama Anjum
` (2 subsequent siblings)
4 siblings, 1 reply; 14+ messages in thread
From: Muhammad Usama Anjum @ 2024-01-25 15:46 UTC (permalink / raw)
To: Andrew Morton, Shuah Khan
Cc: Muhammad Usama Anjum, kernel, Ryan Roberts, linux-mm,
linux-kselftest, linux-kernel
Remove sudo as some test running environments may not have sudo
available. Instead skip the test if root privileges aren't available in
the test.
Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
Changes since v1:
- Added this patch in v2
We are allocating 2*RLIMIT_MEMLOCK.rlim_max memory and mmap() isn't
failing. This seems like true bug in the kernel. Even the root user
shouldn't be able to allocate more memory than allowed MEMLOCKed memory.
Any ideas?
---
tools/testing/selftests/mm/on-fault-limit.c | 36 ++++++++++-----------
tools/testing/selftests/mm/run_vmtests.sh | 2 +-
2 files changed, 18 insertions(+), 20 deletions(-)
diff --git a/tools/testing/selftests/mm/on-fault-limit.c b/tools/testing/selftests/mm/on-fault-limit.c
index b5888d613f34e..0ea98ffab3589 100644
--- a/tools/testing/selftests/mm/on-fault-limit.c
+++ b/tools/testing/selftests/mm/on-fault-limit.c
@@ -5,40 +5,38 @@
#include <string.h>
#include <sys/time.h>
#include <sys/resource.h>
+#include "../kselftest.h"
-static int test_limit(void)
+static void test_limit(void)
{
- int ret = 1;
struct rlimit lims;
void *map;
- if (getrlimit(RLIMIT_MEMLOCK, &lims)) {
- perror("getrlimit");
- return ret;
- }
+ if (getrlimit(RLIMIT_MEMLOCK, &lims))
+ ksft_exit_fail_msg("getrlimit: %s\n", strerror(errno));
- if (mlockall(MCL_ONFAULT | MCL_FUTURE)) {
- perror("mlockall");
- return ret;
- }
+ if (mlockall(MCL_ONFAULT | MCL_FUTURE))
+ ksft_exit_fail_msg("mlockall: %s\n", strerror(errno));
map = mmap(NULL, 2 * lims.rlim_max, PROT_READ | PROT_WRITE,
MAP_PRIVATE | MAP_ANONYMOUS | MAP_POPULATE, -1, 0);
+
+ ksft_test_result(map == MAP_FAILED, "Failed mmap\n");
+
if (map != MAP_FAILED)
- printf("mmap should have failed, but didn't\n");
- else {
- ret = 0;
munmap(map, 2 * lims.rlim_max);
- }
-
munlockall();
- return ret;
}
int main(int argc, char **argv)
{
- int ret = 0;
+ ksft_print_header();
+ ksft_set_plan(1);
+
+ if (getuid())
+ ksft_test_result_skip("Require root privileges to run\n");
+ else
+ test_limit();
- ret += test_limit();
- return ret;
+ ksft_finished();
}
diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh
index 246d53a5d7f28..e373d592dbf5c 100755
--- a/tools/testing/selftests/mm/run_vmtests.sh
+++ b/tools/testing/selftests/mm/run_vmtests.sh
@@ -291,7 +291,7 @@ echo "$nr_hugepgs" > /proc/sys/vm/nr_hugepages
CATEGORY="compaction" run_test ./compaction_test
-CATEGORY="mlock" run_test sudo -u nobody ./on-fault-limit
+CATEGORY="mlock" run_test ./on-fault-limit
CATEGORY="mmap" run_test ./map_populate
--
2.42.0
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v3 2/5] selftests/mm: run_vmtests: remove sudo and conform to tap
2024-01-25 15:46 ` [PATCH v3 2/5] selftests/mm: run_vmtests: remove sudo and conform to tap Muhammad Usama Anjum
@ 2024-02-01 12:04 ` Ryan Roberts
2024-02-01 12:24 ` Muhammad Usama Anjum
0 siblings, 1 reply; 14+ messages in thread
From: Ryan Roberts @ 2024-02-01 12:04 UTC (permalink / raw)
To: Muhammad Usama Anjum, Andrew Morton, Shuah Khan
Cc: kernel, linux-mm, linux-kselftest, linux-kernel
On 25/01/2024 15:46, Muhammad Usama Anjum wrote:
> Remove sudo as some test running environments may not have sudo
> available. Instead skip the test if root privileges aren't available in
> the test.
>
> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> ---
> Changes since v1:
> - Added this patch in v2
>
> We are allocating 2*RLIMIT_MEMLOCK.rlim_max memory and mmap() isn't
> failing. This seems like true bug in the kernel. Even the root user
> shouldn't be able to allocate more memory than allowed MEMLOCKed memory.
> Any ideas?
> ---
> tools/testing/selftests/mm/on-fault-limit.c | 36 ++++++++++-----------
> tools/testing/selftests/mm/run_vmtests.sh | 2 +-
> 2 files changed, 18 insertions(+), 20 deletions(-)
>
> diff --git a/tools/testing/selftests/mm/on-fault-limit.c b/tools/testing/selftests/mm/on-fault-limit.c
> index b5888d613f34e..0ea98ffab3589 100644
> --- a/tools/testing/selftests/mm/on-fault-limit.c
> +++ b/tools/testing/selftests/mm/on-fault-limit.c
> @@ -5,40 +5,38 @@
> #include <string.h>
> #include <sys/time.h>
> #include <sys/resource.h>
> +#include "../kselftest.h"
>
> -static int test_limit(void)
> +static void test_limit(void)
> {
> - int ret = 1;
> struct rlimit lims;
> void *map;
>
> - if (getrlimit(RLIMIT_MEMLOCK, &lims)) {
> - perror("getrlimit");
> - return ret;
> - }
> + if (getrlimit(RLIMIT_MEMLOCK, &lims))
> + ksft_exit_fail_msg("getrlimit: %s\n", strerror(errno));
>
> - if (mlockall(MCL_ONFAULT | MCL_FUTURE)) {
> - perror("mlockall");
> - return ret;
> - }
> + if (mlockall(MCL_ONFAULT | MCL_FUTURE))
> + ksft_exit_fail_msg("mlockall: %s\n", strerror(errno));
>
> map = mmap(NULL, 2 * lims.rlim_max, PROT_READ | PROT_WRITE,
> MAP_PRIVATE | MAP_ANONYMOUS | MAP_POPULATE, -1, 0);
> +
> + ksft_test_result(map == MAP_FAILED, "Failed mmap\n");
> +
> if (map != MAP_FAILED)
> - printf("mmap should have failed, but didn't\n");
> - else {
> - ret = 0;
> munmap(map, 2 * lims.rlim_max);
> - }
> -
> munlockall();
> - return ret;
> }
>
> int main(int argc, char **argv)
> {
> - int ret = 0;
> + ksft_print_header();
> + ksft_set_plan(1);
> +
> + if (getuid())
> + ksft_test_result_skip("Require root privileges to run\n");
> + else
> + test_limit();
>
> - ret += test_limit();
> - return ret;
> + ksft_finished();
> }
> diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh
> index 246d53a5d7f28..e373d592dbf5c 100755
> --- a/tools/testing/selftests/mm/run_vmtests.sh
> +++ b/tools/testing/selftests/mm/run_vmtests.sh
> @@ -291,7 +291,7 @@ echo "$nr_hugepgs" > /proc/sys/vm/nr_hugepages
>
> CATEGORY="compaction" run_test ./compaction_test
>
> -CATEGORY="mlock" run_test sudo -u nobody ./on-fault-limit
> +CATEGORY="mlock" run_test ./on-fault-limit
I think changing this is going to give unintended results. run_vmtests.sh must
already be running as root. "sudo -u nobody" is deprivieging the test to run as
nobody. The rlimit is not enforced for root so this test must run unprivileged
to work. See man page for getrlimit():
Since Linux 2.6.9, no limits are placed on the amount of memory that a
privileged process may lock, and this limit instead governs the amount of
memory that an unprivileged process may lock
So I think the correct fix is actually to install sudo on your CI.
>
> CATEGORY="mmap" run_test ./map_populate
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v3 2/5] selftests/mm: run_vmtests: remove sudo and conform to tap
2024-02-01 12:04 ` Ryan Roberts
@ 2024-02-01 12:24 ` Muhammad Usama Anjum
2024-02-01 12:45 ` Ryan Roberts
0 siblings, 1 reply; 14+ messages in thread
From: Muhammad Usama Anjum @ 2024-02-01 12:24 UTC (permalink / raw)
To: Ryan Roberts, Andrew Morton, Shuah Khan
Cc: Muhammad Usama Anjum, kernel, linux-mm, linux-kselftest, linux-kernel
On 2/1/24 5:04 PM, Ryan Roberts wrote:
> On 25/01/2024 15:46, Muhammad Usama Anjum wrote:
>> Remove sudo as some test running environments may not have sudo
>> available. Instead skip the test if root privileges aren't available in
>> the test.
>>
>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>> ---
>> Changes since v1:
>> - Added this patch in v2
>>
>> We are allocating 2*RLIMIT_MEMLOCK.rlim_max memory and mmap() isn't
>> failing. This seems like true bug in the kernel. Even the root user
>> shouldn't be able to allocate more memory than allowed MEMLOCKed memory.
>> Any ideas?
>> ---
>> tools/testing/selftests/mm/on-fault-limit.c | 36 ++++++++++-----------
>> tools/testing/selftests/mm/run_vmtests.sh | 2 +-
>> 2 files changed, 18 insertions(+), 20 deletions(-)
>>
>> diff --git a/tools/testing/selftests/mm/on-fault-limit.c b/tools/testing/selftests/mm/on-fault-limit.c
>> index b5888d613f34e..0ea98ffab3589 100644
>> --- a/tools/testing/selftests/mm/on-fault-limit.c
>> +++ b/tools/testing/selftests/mm/on-fault-limit.c
>> @@ -5,40 +5,38 @@
>> #include <string.h>
>> #include <sys/time.h>
>> #include <sys/resource.h>
>> +#include "../kselftest.h"
>>
>> -static int test_limit(void)
>> +static void test_limit(void)
>> {
>> - int ret = 1;
>> struct rlimit lims;
>> void *map;
>>
>> - if (getrlimit(RLIMIT_MEMLOCK, &lims)) {
>> - perror("getrlimit");
>> - return ret;
>> - }
>> + if (getrlimit(RLIMIT_MEMLOCK, &lims))
>> + ksft_exit_fail_msg("getrlimit: %s\n", strerror(errno));
>>
>> - if (mlockall(MCL_ONFAULT | MCL_FUTURE)) {
>> - perror("mlockall");
>> - return ret;
>> - }
>> + if (mlockall(MCL_ONFAULT | MCL_FUTURE))
>> + ksft_exit_fail_msg("mlockall: %s\n", strerror(errno));
>>
>> map = mmap(NULL, 2 * lims.rlim_max, PROT_READ | PROT_WRITE,
>> MAP_PRIVATE | MAP_ANONYMOUS | MAP_POPULATE, -1, 0);
>> +
>> + ksft_test_result(map == MAP_FAILED, "Failed mmap\n");
>> +
>> if (map != MAP_FAILED)
>> - printf("mmap should have failed, but didn't\n");
>> - else {
>> - ret = 0;
>> munmap(map, 2 * lims.rlim_max);
>> - }
>> -
>> munlockall();
>> - return ret;
>> }
>>
>> int main(int argc, char **argv)
>> {
>> - int ret = 0;
>> + ksft_print_header();
>> + ksft_set_plan(1);
>> +
>> + if (getuid())
>> + ksft_test_result_skip("Require root privileges to run\n");
I'd sent a patch to fix this behavior today. This test should run without
root privileges.
https://lore.kernel.org/all/20240201071307.592317-1-usama.anjum@collabora.com
>> + else
>> + test_limit();
>>
>> - ret += test_limit();
>> - return ret;
>> + ksft_finished();
>> }
>> diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh
>> index 246d53a5d7f28..e373d592dbf5c 100755
>> --- a/tools/testing/selftests/mm/run_vmtests.sh
>> +++ b/tools/testing/selftests/mm/run_vmtests.sh
>> @@ -291,7 +291,7 @@ echo "$nr_hugepgs" > /proc/sys/vm/nr_hugepages
>>
>> CATEGORY="compaction" run_test ./compaction_test
>>
>> -CATEGORY="mlock" run_test sudo -u nobody ./on-fault-limit
>> +CATEGORY="mlock" run_test ./on-fault-limit
>
> I think changing this is going to give unintended results. run_vmtests.sh must
> already be running as root. "sudo -u nobody" is deprivieging the test to run as
> nobody. The rlimit is not enforced for root so this test must run unprivileged
> to work. See man page for getrlimit():
>
> Since Linux 2.6.9, no limits are placed on the amount of memory that a
> privileged process may lock, and this limit instead governs the amount of
> memory that an unprivileged process may lock
>
> So I think the correct fix is actually to install sudo on your CI.
run_vmtests.sh is invoked without sudo with following:
make -C tools/testing/selftests/mm run_tests
Installing sudo in rootfs wouldn't be trivial enough on the CI side.
Alternatively, we can check if sudo is present before executing this test
to avoid error that sudo isn't found.
>
>>
>> CATEGORY="mmap" run_test ./map_populate
>>
>
>
--
BR,
Muhammad Usama Anjum
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v3 2/5] selftests/mm: run_vmtests: remove sudo and conform to tap
2024-02-01 12:24 ` Muhammad Usama Anjum
@ 2024-02-01 12:45 ` Ryan Roberts
0 siblings, 0 replies; 14+ messages in thread
From: Ryan Roberts @ 2024-02-01 12:45 UTC (permalink / raw)
To: Muhammad Usama Anjum, Andrew Morton, Shuah Khan
Cc: kernel, linux-mm, linux-kselftest, linux-kernel
On 01/02/2024 12:24, Muhammad Usama Anjum wrote:
> On 2/1/24 5:04 PM, Ryan Roberts wrote:
>> On 25/01/2024 15:46, Muhammad Usama Anjum wrote:
>>> Remove sudo as some test running environments may not have sudo
>>> available. Instead skip the test if root privileges aren't available in
>>> the test.
>>>
>>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>>> ---
>>> Changes since v1:
>>> - Added this patch in v2
>>>
>>> We are allocating 2*RLIMIT_MEMLOCK.rlim_max memory and mmap() isn't
>>> failing. This seems like true bug in the kernel. Even the root user
>>> shouldn't be able to allocate more memory than allowed MEMLOCKed memory.
>>> Any ideas?
>>> ---
>>> tools/testing/selftests/mm/on-fault-limit.c | 36 ++++++++++-----------
>>> tools/testing/selftests/mm/run_vmtests.sh | 2 +-
>>> 2 files changed, 18 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/tools/testing/selftests/mm/on-fault-limit.c b/tools/testing/selftests/mm/on-fault-limit.c
>>> index b5888d613f34e..0ea98ffab3589 100644
>>> --- a/tools/testing/selftests/mm/on-fault-limit.c
>>> +++ b/tools/testing/selftests/mm/on-fault-limit.c
>>> @@ -5,40 +5,38 @@
>>> #include <string.h>
>>> #include <sys/time.h>
>>> #include <sys/resource.h>
>>> +#include "../kselftest.h"
>>>
>>> -static int test_limit(void)
>>> +static void test_limit(void)
>>> {
>>> - int ret = 1;
>>> struct rlimit lims;
>>> void *map;
>>>
>>> - if (getrlimit(RLIMIT_MEMLOCK, &lims)) {
>>> - perror("getrlimit");
>>> - return ret;
>>> - }
>>> + if (getrlimit(RLIMIT_MEMLOCK, &lims))
>>> + ksft_exit_fail_msg("getrlimit: %s\n", strerror(errno));
>>>
>>> - if (mlockall(MCL_ONFAULT | MCL_FUTURE)) {
>>> - perror("mlockall");
>>> - return ret;
>>> - }
>>> + if (mlockall(MCL_ONFAULT | MCL_FUTURE))
>>> + ksft_exit_fail_msg("mlockall: %s\n", strerror(errno));
>>>
>>> map = mmap(NULL, 2 * lims.rlim_max, PROT_READ | PROT_WRITE,
>>> MAP_PRIVATE | MAP_ANONYMOUS | MAP_POPULATE, -1, 0);
>>> +
>>> + ksft_test_result(map == MAP_FAILED, "Failed mmap\n");
>>> +
>>> if (map != MAP_FAILED)
>>> - printf("mmap should have failed, but didn't\n");
>>> - else {
>>> - ret = 0;
>>> munmap(map, 2 * lims.rlim_max);
>>> - }
>>> -
>>> munlockall();
>>> - return ret;
>>> }
>>>
>>> int main(int argc, char **argv)
>>> {
>>> - int ret = 0;
>>> + ksft_print_header();
>>> + ksft_set_plan(1);
>>> +
>>> + if (getuid())
>>> + ksft_test_result_skip("Require root privileges to run\n");
> I'd sent a patch to fix this behavior today. This test should run without
> root privileges.
> https://lore.kernel.org/all/20240201071307.592317-1-usama.anjum@collabora.com
>
>>> + else
>>> + test_limit();
>>>
>>> - ret += test_limit();
>>> - return ret;
>>> + ksft_finished();
>>> }
>>> diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh
>>> index 246d53a5d7f28..e373d592dbf5c 100755
>>> --- a/tools/testing/selftests/mm/run_vmtests.sh
>>> +++ b/tools/testing/selftests/mm/run_vmtests.sh
>>> @@ -291,7 +291,7 @@ echo "$nr_hugepgs" > /proc/sys/vm/nr_hugepages
>>>
>>> CATEGORY="compaction" run_test ./compaction_test
>>>
>>> -CATEGORY="mlock" run_test sudo -u nobody ./on-fault-limit
>>> +CATEGORY="mlock" run_test ./on-fault-limit
>>
>> I think changing this is going to give unintended results. run_vmtests.sh must
>> already be running as root. "sudo -u nobody" is deprivieging the test to run as
>> nobody. The rlimit is not enforced for root so this test must run unprivileged
>> to work. See man page for getrlimit():
>>
>> Since Linux 2.6.9, no limits are placed on the amount of memory that a
>> privileged process may lock, and this limit instead governs the amount of
>> memory that an unprivileged process may lock
>>
>> So I think the correct fix is actually to install sudo on your CI.
> run_vmtests.sh is invoked without sudo with following:
> make -C tools/testing/selftests/mm run_tests
Unfortunately, I live in a world where my build machine isn't always the same as
the target machine, so I'm not too familiar with this method of invocation.
Regardless, the vast majority of the tests in run_vmtests.sh (as well as the
configuration code in the script itself) require root. So invoking
run_vmtests.sh as anything other than root is a BadIdea (TM). And when
run_vmtests.sh is running as root, then you need the "sudo -u nobody" to
deprivilege this particular test.
>
> Installing sudo in rootfs wouldn't be trivial enough on the CI side.
> Alternatively, we can check if sudo is present before executing this test
> to avoid error that sudo isn't found.
Yeah, that's probably the easiest solution; just skip it if the requirements are
not met.
>
>>
>>>
>>> CATEGORY="mmap" run_test ./map_populate
>>>
>>
>>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 3/5] selftests/mm: save and restore nr_hugepages value
2024-01-25 15:46 [PATCH v3 0/5] selftests/mm: Improve run_vmtests.sh Muhammad Usama Anjum
2024-01-25 15:46 ` [PATCH v3 1/5] selftests/mm: hugetlb_reparenting_test: do not unmount Muhammad Usama Anjum
2024-01-25 15:46 ` [PATCH v3 2/5] selftests/mm: run_vmtests: remove sudo and conform to tap Muhammad Usama Anjum
@ 2024-01-25 15:46 ` Muhammad Usama Anjum
2024-01-25 15:46 ` [PATCH v3 4/5] selftests/mm: protection_keys: save/restore nr_hugepages settings Muhammad Usama Anjum
2024-01-25 15:46 ` [PATCH v3 5/5] selftests/mm: run_vmtests.sh: add missing tests Muhammad Usama Anjum
4 siblings, 0 replies; 14+ messages in thread
From: Muhammad Usama Anjum @ 2024-01-25 15:46 UTC (permalink / raw)
To: Andrew Morton, Shuah Khan
Cc: Muhammad Usama Anjum, kernel, Ryan Roberts, linux-mm,
linux-kselftest, linux-kernel
Save and restore nr_hugepages before changing it during the test. A test
should not change system wide settings.
Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
tools/testing/selftests/mm/charge_reserved_hugetlb.sh | 4 ++++
tools/testing/selftests/mm/hugetlb_reparenting_test.sh | 3 +++
2 files changed, 7 insertions(+)
diff --git a/tools/testing/selftests/mm/charge_reserved_hugetlb.sh b/tools/testing/selftests/mm/charge_reserved_hugetlb.sh
index e14bdd4455f2d..d680c00d2853a 100755
--- a/tools/testing/selftests/mm/charge_reserved_hugetlb.sh
+++ b/tools/testing/selftests/mm/charge_reserved_hugetlb.sh
@@ -11,6 +11,8 @@ if [[ $(id -u) -ne 0 ]]; then
exit $ksft_skip
fi
+nr_hugepgs=$(cat /proc/sys/vm/nr_hugepages)
+
fault_limit_file=limit_in_bytes
reservation_limit_file=rsvd.limit_in_bytes
fault_usage_file=usage_in_bytes
@@ -582,3 +584,5 @@ if [[ $do_umount ]]; then
umount $cgroup_path
rmdir $cgroup_path
fi
+
+echo "$nr_hugepgs" > /proc/sys/vm/nr_hugepages
diff --git a/tools/testing/selftests/mm/hugetlb_reparenting_test.sh b/tools/testing/selftests/mm/hugetlb_reparenting_test.sh
index 615c4d766c909..11f9bbe7dc222 100755
--- a/tools/testing/selftests/mm/hugetlb_reparenting_test.sh
+++ b/tools/testing/selftests/mm/hugetlb_reparenting_test.sh
@@ -11,6 +11,7 @@ if [[ $(id -u) -ne 0 ]]; then
exit $ksft_skip
fi
+nr_hugepgs=$(cat /proc/sys/vm/nr_hugepages)
usage_file=usage_in_bytes
if [[ "$1" == "-cgroup-v2" ]]; then
@@ -252,3 +253,5 @@ if [[ $do_umount ]]; then
umount $CGROUP_ROOT
rm -rf $CGROUP_ROOT
fi
+
+echo "$nr_hugepgs" > /proc/sys/vm/nr_hugepages
--
2.42.0
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 4/5] selftests/mm: protection_keys: save/restore nr_hugepages settings
2024-01-25 15:46 [PATCH v3 0/5] selftests/mm: Improve run_vmtests.sh Muhammad Usama Anjum
` (2 preceding siblings ...)
2024-01-25 15:46 ` [PATCH v3 3/5] selftests/mm: save and restore nr_hugepages value Muhammad Usama Anjum
@ 2024-01-25 15:46 ` Muhammad Usama Anjum
2024-03-13 14:58 ` Joey Gouly
2024-01-25 15:46 ` [PATCH v3 5/5] selftests/mm: run_vmtests.sh: add missing tests Muhammad Usama Anjum
4 siblings, 1 reply; 14+ messages in thread
From: Muhammad Usama Anjum @ 2024-01-25 15:46 UTC (permalink / raw)
To: Andrew Morton, Shuah Khan
Cc: Muhammad Usama Anjum, kernel, Ryan Roberts, linux-mm,
linux-kselftest, linux-kernel
Save and restore nr_hugepages before changing it during the test. A test
should not change system wide settings.
Fixes: 5f23f6d082a9 ("x86/pkeys: Add self-tests")
Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
tools/testing/selftests/mm/protection_keys.c | 34 ++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/tools/testing/selftests/mm/protection_keys.c b/tools/testing/selftests/mm/protection_keys.c
index 48dc151f8fca8..f822ae31af22e 100644
--- a/tools/testing/selftests/mm/protection_keys.c
+++ b/tools/testing/selftests/mm/protection_keys.c
@@ -54,6 +54,7 @@ int test_nr;
u64 shadow_pkey_reg;
int dprint_in_signal;
char dprint_in_signal_buffer[DPRINT_IN_SIGNAL_BUF_SIZE];
+char buf[256];
void cat_into_file(char *str, char *file)
{
@@ -1744,6 +1745,38 @@ void pkey_setup_shadow(void)
shadow_pkey_reg = __read_pkey_reg();
}
+void restore_settings_atexit(void)
+{
+ cat_into_file(buf, "/proc/sys/vm/nr_hugepages");
+}
+
+void save_settings(void)
+{
+ int fd;
+ int err;
+
+ if (geteuid())
+ return;
+
+ fd = open("/proc/sys/vm/nr_hugepages", O_RDONLY);
+ if (fd < 0) {
+ fprintf(stderr, "error opening\n");
+ perror("error: ");
+ exit(__LINE__);
+ }
+
+ /* -1 to guarantee leaving the trailing \0 */
+ err = read(fd, buf, sizeof(buf)-1);
+ if (err < 0) {
+ fprintf(stderr, "error reading\n");
+ perror("error: ");
+ exit(__LINE__);
+ }
+
+ atexit(restore_settings_atexit);
+ close(fd);
+}
+
int main(void)
{
int nr_iterations = 22;
@@ -1751,6 +1784,7 @@ int main(void)
srand((unsigned int)time(NULL));
+ save_settings();
setup_handlers();
printf("has pkeys: %d\n", pkeys_supported);
--
2.42.0
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v3 4/5] selftests/mm: protection_keys: save/restore nr_hugepages settings
2024-01-25 15:46 ` [PATCH v3 4/5] selftests/mm: protection_keys: save/restore nr_hugepages settings Muhammad Usama Anjum
@ 2024-03-13 14:58 ` Joey Gouly
2024-03-13 18:12 ` Muhammad Usama Anjum
0 siblings, 1 reply; 14+ messages in thread
From: Joey Gouly @ 2024-03-13 14:58 UTC (permalink / raw)
To: Muhammad Usama Anjum
Cc: Andrew Morton, Shuah Khan, kernel, Ryan Roberts, linux-mm,
linux-kselftest, linux-kernel
Hi Muhammad,
On Thu, Jan 25, 2024 at 08:46:07PM +0500, Muhammad Usama Anjum wrote:
> Save and restore nr_hugepages before changing it during the test. A test
> should not change system wide settings.
>
> Fixes: 5f23f6d082a9 ("x86/pkeys: Add self-tests")
> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> ---
> tools/testing/selftests/mm/protection_keys.c | 34 ++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/tools/testing/selftests/mm/protection_keys.c b/tools/testing/selftests/mm/protection_keys.c
> index 48dc151f8fca8..f822ae31af22e 100644
> --- a/tools/testing/selftests/mm/protection_keys.c
> +++ b/tools/testing/selftests/mm/protection_keys.c
> @@ -54,6 +54,7 @@ int test_nr;
> u64 shadow_pkey_reg;
> int dprint_in_signal;
> char dprint_in_signal_buffer[DPRINT_IN_SIGNAL_BUF_SIZE];
> +char buf[256];
>
> void cat_into_file(char *str, char *file)
> {
> @@ -1744,6 +1745,38 @@ void pkey_setup_shadow(void)
> shadow_pkey_reg = __read_pkey_reg();
> }
>
> +void restore_settings_atexit(void)
> +{
> + cat_into_file(buf, "/proc/sys/vm/nr_hugepages");
> +}
> +
> +void save_settings(void)
> +{
> + int fd;
> + int err;
> +
> + if (geteuid())
> + return;
> +
> + fd = open("/proc/sys/vm/nr_hugepages", O_RDONLY);
> + if (fd < 0) {
> + fprintf(stderr, "error opening\n");
> + perror("error: ");
> + exit(__LINE__);
> + }
> +
> + /* -1 to guarantee leaving the trailing \0 */
> + err = read(fd, buf, sizeof(buf)-1);
> + if (err < 0) {
> + fprintf(stderr, "error reading\n");
> + perror("error: ");
> + exit(__LINE__);
> + }
> +
> + atexit(restore_settings_atexit);
> + close(fd);
> +}
> +
> int main(void)
> {
> int nr_iterations = 22;
> @@ -1751,6 +1784,7 @@ int main(void)
>
> srand((unsigned int)time(NULL));
>
> + save_settings();
> setup_handlers();
>
> printf("has pkeys: %d\n", pkeys_supported);
> --
> 2.42.0
>
This break the tests for me:
assert() at protection_keys.c::812 test_nr: 19 iteration: 1
running abort_hooks()...
This is because some of the tests fork, so on their atexit() they will set the
nr_hugepages back to the previous setting. Specifically the
test_pkey_alloc_exhaust() test.
Thanks,
Joey
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v3 4/5] selftests/mm: protection_keys: save/restore nr_hugepages settings
2024-03-13 14:58 ` Joey Gouly
@ 2024-03-13 18:12 ` Muhammad Usama Anjum
2024-03-14 9:31 ` Joey Gouly
0 siblings, 1 reply; 14+ messages in thread
From: Muhammad Usama Anjum @ 2024-03-13 18:12 UTC (permalink / raw)
To: Joey Gouly
Cc: Muhammad Usama Anjum, Andrew Morton, Shuah Khan, kernel,
Ryan Roberts, linux-mm, linux-kselftest, linux-kernel
On 3/13/24 7:58 PM, Joey Gouly wrote:
> Hi Muhammad,
>
> On Thu, Jan 25, 2024 at 08:46:07PM +0500, Muhammad Usama Anjum wrote:
>> Save and restore nr_hugepages before changing it during the test. A test
>> should not change system wide settings.
>>
>> Fixes: 5f23f6d082a9 ("x86/pkeys: Add self-tests")
>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>> ---
>> tools/testing/selftests/mm/protection_keys.c | 34 ++++++++++++++++++++
>> 1 file changed, 34 insertions(+)
>>
>> diff --git a/tools/testing/selftests/mm/protection_keys.c b/tools/testing/selftests/mm/protection_keys.c
>> index 48dc151f8fca8..f822ae31af22e 100644
>> --- a/tools/testing/selftests/mm/protection_keys.c
>> +++ b/tools/testing/selftests/mm/protection_keys.c
>> @@ -54,6 +54,7 @@ int test_nr;
>> u64 shadow_pkey_reg;
>> int dprint_in_signal;
>> char dprint_in_signal_buffer[DPRINT_IN_SIGNAL_BUF_SIZE];
>> +char buf[256];
>>
>> void cat_into_file(char *str, char *file)
>> {
>> @@ -1744,6 +1745,38 @@ void pkey_setup_shadow(void)
>> shadow_pkey_reg = __read_pkey_reg();
>> }
>>
>> +void restore_settings_atexit(void)
>> +{
>> + cat_into_file(buf, "/proc/sys/vm/nr_hugepages");
>> +}
>> +
>> +void save_settings(void)
>> +{
>> + int fd;
>> + int err;
>> +
>> + if (geteuid())
>> + return;
>> +
>> + fd = open("/proc/sys/vm/nr_hugepages", O_RDONLY);
>> + if (fd < 0) {
>> + fprintf(stderr, "error opening\n");
>> + perror("error: ");
>> + exit(__LINE__);
>> + }
>> +
>> + /* -1 to guarantee leaving the trailing \0 */
>> + err = read(fd, buf, sizeof(buf)-1);
>> + if (err < 0) {
>> + fprintf(stderr, "error reading\n");
>> + perror("error: ");
>> + exit(__LINE__);
>> + }
>> +
>> + atexit(restore_settings_atexit);
>> + close(fd);
>> +}
>> +
>> int main(void)
>> {
>> int nr_iterations = 22;
>> @@ -1751,6 +1784,7 @@ int main(void)
>>
>> srand((unsigned int)time(NULL));
>>
>> + save_settings();
>> setup_handlers();
>>
>> printf("has pkeys: %d\n", pkeys_supported);
>> --
>> 2.42.0
>>
>
> This break the tests for me:
>
> assert() at protection_keys.c::812 test_nr: 19 iteration: 1
> running abort_hooks()...
>
> This is because some of the tests fork, so on their atexit() they will set the
> nr_hugepages back to the previous setting. Specifically the
> test_pkey_alloc_exhaust() test.
Thank you for reporting. Please can you test the following patch:
--- a/tools/testing/selftests/mm/protection_keys.c
+++ b/tools/testing/selftests/mm/protection_keys.c
@@ -1745,9 +1745,12 @@ void pkey_setup_shadow(void)
shadow_pkey_reg = __read_pkey_reg();
}
+pid_t parent_pid;
+
void restore_settings_atexit(void)
{
- cat_into_file(buf, "/proc/sys/vm/nr_hugepages");
+ if (parent_pid == getpid())
+ cat_into_file(buf, "/proc/sys/vm/nr_hugepages");
}
void save_settings(void)
@@ -1773,6 +1776,7 @@ void save_settings(void)
exit(__LINE__);
}
+ parent_pid = getpid();
atexit(restore_settings_atexit);
close(fd);
}
>
> Thanks,
> Joey
>
--
BR,
Muhammad Usama Anjum
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v3 4/5] selftests/mm: protection_keys: save/restore nr_hugepages settings
2024-03-13 18:12 ` Muhammad Usama Anjum
@ 2024-03-14 9:31 ` Joey Gouly
0 siblings, 0 replies; 14+ messages in thread
From: Joey Gouly @ 2024-03-14 9:31 UTC (permalink / raw)
To: Muhammad Usama Anjum
Cc: Andrew Morton, Shuah Khan, kernel, Ryan Roberts, linux-mm,
linux-kselftest, linux-kernel
On Wed, Mar 13, 2024 at 11:12:58PM +0500, Muhammad Usama Anjum wrote:
> On 3/13/24 7:58 PM, Joey Gouly wrote:
> > Hi Muhammad,
> >
> > On Thu, Jan 25, 2024 at 08:46:07PM +0500, Muhammad Usama Anjum wrote:
> >> Save and restore nr_hugepages before changing it during the test. A test
> >> should not change system wide settings.
> >>
> >> Fixes: 5f23f6d082a9 ("x86/pkeys: Add self-tests")
> >> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> >> ---
> >> tools/testing/selftests/mm/protection_keys.c | 34 ++++++++++++++++++++
> >> 1 file changed, 34 insertions(+)
> >>
> >> diff --git a/tools/testing/selftests/mm/protection_keys.c b/tools/testing/selftests/mm/protection_keys.c
> >> index 48dc151f8fca8..f822ae31af22e 100644
> >> --- a/tools/testing/selftests/mm/protection_keys.c
> >> +++ b/tools/testing/selftests/mm/protection_keys.c
> >> @@ -54,6 +54,7 @@ int test_nr;
> >> u64 shadow_pkey_reg;
> >> int dprint_in_signal;
> >> char dprint_in_signal_buffer[DPRINT_IN_SIGNAL_BUF_SIZE];
> >> +char buf[256];
> >>
> >> void cat_into_file(char *str, char *file)
> >> {
> >> @@ -1744,6 +1745,38 @@ void pkey_setup_shadow(void)
> >> shadow_pkey_reg = __read_pkey_reg();
> >> }
> >>
> >> +void restore_settings_atexit(void)
> >> +{
> >> + cat_into_file(buf, "/proc/sys/vm/nr_hugepages");
> >> +}
> >> +
> >> +void save_settings(void)
> >> +{
> >> + int fd;
> >> + int err;
> >> +
> >> + if (geteuid())
> >> + return;
> >> +
> >> + fd = open("/proc/sys/vm/nr_hugepages", O_RDONLY);
> >> + if (fd < 0) {
> >> + fprintf(stderr, "error opening\n");
> >> + perror("error: ");
> >> + exit(__LINE__);
> >> + }
> >> +
> >> + /* -1 to guarantee leaving the trailing \0 */
> >> + err = read(fd, buf, sizeof(buf)-1);
> >> + if (err < 0) {
> >> + fprintf(stderr, "error reading\n");
> >> + perror("error: ");
> >> + exit(__LINE__);
> >> + }
> >> +
> >> + atexit(restore_settings_atexit);
> >> + close(fd);
> >> +}
> >> +
> >> int main(void)
> >> {
> >> int nr_iterations = 22;
> >> @@ -1751,6 +1784,7 @@ int main(void)
> >>
> >> srand((unsigned int)time(NULL));
> >>
> >> + save_settings();
> >> setup_handlers();
> >>
> >> printf("has pkeys: %d\n", pkeys_supported);
> >> --
> >> 2.42.0
> >>
> >
> > This break the tests for me:
> >
> > assert() at protection_keys.c::812 test_nr: 19 iteration: 1
> > running abort_hooks()...
> >
> > This is because some of the tests fork, so on their atexit() they will set the
> > nr_hugepages back to the previous setting. Specifically the
> > test_pkey_alloc_exhaust() test.
> Thank you for reporting. Please can you test the following patch:
>
> --- a/tools/testing/selftests/mm/protection_keys.c
> +++ b/tools/testing/selftests/mm/protection_keys.c
> @@ -1745,9 +1745,12 @@ void pkey_setup_shadow(void)
> shadow_pkey_reg = __read_pkey_reg();
> }
>
> +pid_t parent_pid;
> +
> void restore_settings_atexit(void)
> {
> - cat_into_file(buf, "/proc/sys/vm/nr_hugepages");
> + if (parent_pid == getpid())
> + cat_into_file(buf, "/proc/sys/vm/nr_hugepages");
> }
>
> void save_settings(void)
> @@ -1773,6 +1776,7 @@ void save_settings(void)
> exit(__LINE__);
> }
>
> + parent_pid = getpid();
> atexit(restore_settings_atexit);
> close(fd);
> }
>
Thanks, works for me:
Tested-by: Joey Gouly <joey.gouly@arm.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 5/5] selftests/mm: run_vmtests.sh: add missing tests
2024-01-25 15:46 [PATCH v3 0/5] selftests/mm: Improve run_vmtests.sh Muhammad Usama Anjum
` (3 preceding siblings ...)
2024-01-25 15:46 ` [PATCH v3 4/5] selftests/mm: protection_keys: save/restore nr_hugepages settings Muhammad Usama Anjum
@ 2024-01-25 15:46 ` Muhammad Usama Anjum
2024-02-01 12:11 ` Ryan Roberts
4 siblings, 1 reply; 14+ messages in thread
From: Muhammad Usama Anjum @ 2024-01-25 15:46 UTC (permalink / raw)
To: Andrew Morton, Shuah Khan
Cc: Muhammad Usama Anjum, kernel, Ryan Roberts, linux-mm,
linux-kselftest, linux-kernel
Add missing tests to run_vmtests.sh. The mm kselftests are run through
run_vmtests.sh. If a test isn't present in this script, it'll not run
with run_tests or `make -C tools/testing/selftests/mm run_tests`.
Cc: Ryan Roberts <ryan.roberts@arm.com>
Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
Changes since v1:
- Copy the original scripts and their dependence script to install directory as well
Changes since v2:
- Add a comment
- Move tests down in the file
- Add "-d" option which poisons the pages and aren't being useable after
the test
---
tools/testing/selftests/mm/Makefile | 5 +++++
tools/testing/selftests/mm/run_vmtests.sh | 8 ++++++++
2 files changed, 13 insertions(+)
diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
index 2453add65d12f..f3aec7be80730 100644
--- a/tools/testing/selftests/mm/Makefile
+++ b/tools/testing/selftests/mm/Makefile
@@ -114,6 +114,11 @@ TEST_PROGS := run_vmtests.sh
TEST_FILES := test_vmalloc.sh
TEST_FILES += test_hmm.sh
TEST_FILES += va_high_addr_switch.sh
+TEST_FILES += charge_reserved_hugetlb.sh
+TEST_FILES += hugetlb_reparenting_test.sh
+
+# required by charge_reserved_hugetlb.sh
+TEST_FILES += write_hugetlb_memory.sh
include ../lib.mk
diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh
index e373d592dbf5c..a0f37e4438937 100755
--- a/tools/testing/selftests/mm/run_vmtests.sh
+++ b/tools/testing/selftests/mm/run_vmtests.sh
@@ -19,6 +19,7 @@ usage: ${BASH_SOURCE[0]:-$0} [ options ]
-t: specify specific categories to tests to run
-h: display this message
-n: disable TAP output
+ -d: run destructive tests
The default behavior is to run required tests only. If -a is specified,
will run all tests.
@@ -79,6 +80,7 @@ EOF
}
RUN_ALL=false
+RUN_DESTRUCTIVE_TEST=false
TAP_PREFIX="# "
while getopts "aht:n" OPT; do
@@ -87,6 +89,7 @@ while getopts "aht:n" OPT; do
"h") usage ;;
"t") VM_SELFTEST_ITEMS=${OPTARG} ;;
"n") TAP_PREFIX= ;;
+ "a") RUN_DESTRUCTIVE_TEST=true ;;
esac
done
shift $((OPTIND -1))
@@ -304,6 +307,11 @@ CATEGORY="process_mrelease" run_test ./mrelease_test
CATEGORY="mremap" run_test ./mremap_test
CATEGORY="hugetlb" run_test ./thuge-gen
+CATEGORY="hugetlb" run_test ./charge_reserved_hugetlb.sh -cgroup-v2
+CATEGORY="hugetlb" run_test ./hugetlb_reparenting_test.sh -cgroup-v2
+if $RUN_DESTRUCTIVE_TEST; then
+CATEGORY="hugetlb" run_test ./hugetlb-read-hwpoison
+fi
if [ $VADDR64 -ne 0 ]; then
--
2.42.0
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v3 5/5] selftests/mm: run_vmtests.sh: add missing tests
2024-01-25 15:46 ` [PATCH v3 5/5] selftests/mm: run_vmtests.sh: add missing tests Muhammad Usama Anjum
@ 2024-02-01 12:11 ` Ryan Roberts
2024-02-01 12:26 ` Muhammad Usama Anjum
0 siblings, 1 reply; 14+ messages in thread
From: Ryan Roberts @ 2024-02-01 12:11 UTC (permalink / raw)
To: Muhammad Usama Anjum, Andrew Morton, Shuah Khan
Cc: kernel, linux-mm, linux-kselftest, linux-kernel
On 25/01/2024 15:46, Muhammad Usama Anjum wrote:
> Add missing tests to run_vmtests.sh. The mm kselftests are run through
> run_vmtests.sh. If a test isn't present in this script, it'll not run
> with run_tests or `make -C tools/testing/selftests/mm run_tests`.
>
> Cc: Ryan Roberts <ryan.roberts@arm.com>
> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> ---
> Changes since v1:
> - Copy the original scripts and their dependence script to install directory as well
>
> Changes since v2:
> - Add a comment
> - Move tests down in the file
> - Add "-d" option which poisons the pages and aren't being useable after
> the test
> ---
> tools/testing/selftests/mm/Makefile | 5 +++++
> tools/testing/selftests/mm/run_vmtests.sh | 8 ++++++++
> 2 files changed, 13 insertions(+)
>
> diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
> index 2453add65d12f..f3aec7be80730 100644
> --- a/tools/testing/selftests/mm/Makefile
> +++ b/tools/testing/selftests/mm/Makefile
> @@ -114,6 +114,11 @@ TEST_PROGS := run_vmtests.sh
> TEST_FILES := test_vmalloc.sh
> TEST_FILES += test_hmm.sh
> TEST_FILES += va_high_addr_switch.sh
> +TEST_FILES += charge_reserved_hugetlb.sh
> +TEST_FILES += hugetlb_reparenting_test.sh
> +
> +# required by charge_reserved_hugetlb.sh
> +TEST_FILES += write_hugetlb_memory.sh
>
> include ../lib.mk
>
> diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh
> index e373d592dbf5c..a0f37e4438937 100755
> --- a/tools/testing/selftests/mm/run_vmtests.sh
> +++ b/tools/testing/selftests/mm/run_vmtests.sh
> @@ -19,6 +19,7 @@ usage: ${BASH_SOURCE[0]:-$0} [ options ]
> -t: specify specific categories to tests to run
> -h: display this message
> -n: disable TAP output
> + -d: run destructive tests
You probably want to clarify the behaviour for -a (all). I guess providing -a
should NOT run destructive tests unless -d is also explicitly provided.
>
> The default behavior is to run required tests only. If -a is specified,
> will run all tests.
> @@ -79,6 +80,7 @@ EOF
> }
>
> RUN_ALL=false
> +RUN_DESTRUCTIVE_TEST=false
Either call this RUN_DESTRUCTIVE (my preference) or at least make it plural
(RUN_DESTRUCTIVE_TESTS).
> TAP_PREFIX="# "
>
> while getopts "aht:n" OPT; do
> @@ -87,6 +89,7 @@ while getopts "aht:n" OPT; do
> "h") usage ;;
> "t") VM_SELFTEST_ITEMS=${OPTARG} ;;
> "n") TAP_PREFIX= ;;
> + "a") RUN_DESTRUCTIVE_TEST=true ;;
The help you added says the option is -d, but this is looking for -a, and
conflicting with the existing -a=all option.
> esac
> done
> shift $((OPTIND -1))
> @@ -304,6 +307,11 @@ CATEGORY="process_mrelease" run_test ./mrelease_test
> CATEGORY="mremap" run_test ./mremap_test
>
> CATEGORY="hugetlb" run_test ./thuge-gen
> +CATEGORY="hugetlb" run_test ./charge_reserved_hugetlb.sh -cgroup-v2
> +CATEGORY="hugetlb" run_test ./hugetlb_reparenting_test.sh -cgroup-v2
> +if $RUN_DESTRUCTIVE_TEST; then
> +CATEGORY="hugetlb" run_test ./hugetlb-read-hwpoison
> +fi
>
> if [ $VADDR64 -ne 0 ]; then
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v3 5/5] selftests/mm: run_vmtests.sh: add missing tests
2024-02-01 12:11 ` Ryan Roberts
@ 2024-02-01 12:26 ` Muhammad Usama Anjum
0 siblings, 0 replies; 14+ messages in thread
From: Muhammad Usama Anjum @ 2024-02-01 12:26 UTC (permalink / raw)
To: Ryan Roberts, Andrew Morton, Shuah Khan
Cc: Muhammad Usama Anjum, kernel, linux-mm, linux-kselftest, linux-kernel
On 2/1/24 5:11 PM, Ryan Roberts wrote:
> On 25/01/2024 15:46, Muhammad Usama Anjum wrote:
>> Add missing tests to run_vmtests.sh. The mm kselftests are run through
>> run_vmtests.sh. If a test isn't present in this script, it'll not run
>> with run_tests or `make -C tools/testing/selftests/mm run_tests`.
>>
>> Cc: Ryan Roberts <ryan.roberts@arm.com>
>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>> ---
>> Changes since v1:
>> - Copy the original scripts and their dependence script to install directory as well
>>
>> Changes since v2:
>> - Add a comment
>> - Move tests down in the file
>> - Add "-d" option which poisons the pages and aren't being useable after
>> the test
>> ---
>> tools/testing/selftests/mm/Makefile | 5 +++++
>> tools/testing/selftests/mm/run_vmtests.sh | 8 ++++++++
>> 2 files changed, 13 insertions(+)
>>
>> diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
>> index 2453add65d12f..f3aec7be80730 100644
>> --- a/tools/testing/selftests/mm/Makefile
>> +++ b/tools/testing/selftests/mm/Makefile
>> @@ -114,6 +114,11 @@ TEST_PROGS := run_vmtests.sh
>> TEST_FILES := test_vmalloc.sh
>> TEST_FILES += test_hmm.sh
>> TEST_FILES += va_high_addr_switch.sh
>> +TEST_FILES += charge_reserved_hugetlb.sh
>> +TEST_FILES += hugetlb_reparenting_test.sh
>> +
>> +# required by charge_reserved_hugetlb.sh
>> +TEST_FILES += write_hugetlb_memory.sh
>>
>> include ../lib.mk
>>
>> diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh
>> index e373d592dbf5c..a0f37e4438937 100755
>> --- a/tools/testing/selftests/mm/run_vmtests.sh
>> +++ b/tools/testing/selftests/mm/run_vmtests.sh
>> @@ -19,6 +19,7 @@ usage: ${BASH_SOURCE[0]:-$0} [ options ]
>> -t: specify specific categories to tests to run
>> -h: display this message
>> -n: disable TAP output
>> + -d: run destructive tests
>
> You probably want to clarify the behaviour for -a (all). I guess providing -a
> should NOT run destructive tests unless -d is also explicitly provided.
>
>>
>> The default behavior is to run required tests only. If -a is specified,
>> will run all tests.
>> @@ -79,6 +80,7 @@ EOF
>> }
>>
>> RUN_ALL=false
>> +RUN_DESTRUCTIVE_TEST=false
>
> Either call this RUN_DESTRUCTIVE (my preference) or at least make it plural
> (RUN_DESTRUCTIVE_TESTS).
>
>> TAP_PREFIX="# "
>>
>> while getopts "aht:n" OPT; do
>> @@ -87,6 +89,7 @@ while getopts "aht:n" OPT; do
>> "h") usage ;;
>> "t") VM_SELFTEST_ITEMS=${OPTARG} ;;
>> "n") TAP_PREFIX= ;;
>> + "a") RUN_DESTRUCTIVE_TEST=true ;;
>
> The help you added says the option is -d, but this is looking for -a, and
> conflicting with the existing -a=all option.
Sorry, that's a typo. I'll resolve your above comments with fix patch as well.
>
>> esac
>> done
>> shift $((OPTIND -1))
>> @@ -304,6 +307,11 @@ CATEGORY="process_mrelease" run_test ./mrelease_test
>> CATEGORY="mremap" run_test ./mremap_test
>>
>> CATEGORY="hugetlb" run_test ./thuge-gen
>> +CATEGORY="hugetlb" run_test ./charge_reserved_hugetlb.sh -cgroup-v2
>> +CATEGORY="hugetlb" run_test ./hugetlb_reparenting_test.sh -cgroup-v2
>> +if $RUN_DESTRUCTIVE_TEST; then
>> +CATEGORY="hugetlb" run_test ./hugetlb-read-hwpoison
>> +fi
>>
>> if [ $VADDR64 -ne 0 ]; then
>>
>
>
--
BR,
Muhammad Usama Anjum
^ permalink raw reply [flat|nested] 14+ messages in thread