[LTP] [PATCH] ht_affinity.c: fix ht_affinity test failure

[LTP] [PATCH] ht_affinity.c: fix ht_affinity test failure

[Haifeng Xu via ltp]

Running ht_affinity test fails:

	smt_smp_affinity    0  TINFO  :  Set affinity through system call
	smt_smp_affinity    0  TINFO  :  Set test process affinity.
	mask: 1
	smt_smp_affinity    0  TINFO  :  ...Error

	...

	smt_smp_affinity    0  TINFO  :  Set test process affinity.
	mask: c0000000
	smt_smp_affinity    0  TINFO  :  ...Error
	smt_smp_affinity    3  TFAIL  :  ht_affinity.c:240: System call setaffinity() is error.

The type of cpumask pointer used in set_affinity() is unsigned long, but
ht_affinity used a unsigned int pointer. When kernel copy cpumask from
user-space pointer, the high 32bit of cpumask is a random value. So the
process can't be bind to the cpu specified by users.

After converting the cpumask from unsigned int to unsigned long, ht_affinity
test succeeds:

	smt_smp_affinity    0  TINFO  :  Set affinity through system call
	smt_smp_affinity    0  TINFO  :  Set test process affinity.
	mask: 1
	smt_smp_affinity    0  TINFO  :  ...OK

	...

	smt_smp_affinity    0  TINFO  :  Set test process affinity.
	mask: c0000000
	smt_smp_affinity    0  TINFO  :  ...OK
	smt_smp_affinity    3  TPASS  :  System call setaffinity() is OK.

Signed-off-by: Haifeng Xu <haifeng.xu@shopee.com>
---
 .../kernel/sched/hyperthreading/ht_affinity/ht_affinity.c   | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/testcases/kernel/sched/hyperthreading/ht_affinity/ht_affinity.c b/testcases/kernel/sched/hyperthreading/ht_affinity/ht_affinity.c
index f6e9f2745..e6829c2d2 100644
--- a/testcases/kernel/sched/hyperthreading/ht_affinity/ht_affinity.c
+++ b/testcases/kernel/sched/hyperthreading/ht_affinity/ht_affinity.c
@@ -48,7 +48,7 @@ len - length in bytes of the bitmask pointed to by user_mask_ptr.
 
 int HT_SetAffinity(void)
 {
-	unsigned int mask;
+	unsigned long mask;
 	pid_t pid;
 	int result = 1;
 	int cpu_count, i, j, k, cpuid;
@@ -65,7 +65,7 @@ int HT_SetAffinity(void)
 
 	for (i = 0, mask = 0x1; i < cpu_count; i++, mask = mask << 1) {
 		tst_resm(TINFO, "Set test process affinity.");
-		printf("mask: %x\n", mask);
+		printf("mask: %lx\n", mask);
 
 		sched_setaffinity(pid, sizeof(unsigned long), &mask);
 
@@ -93,7 +93,7 @@ int HT_SetAffinity(void)
 
 	for (i = 0, mask = 0x3; i < cpu_count - 1; i++, mask = mask << 1) {
 		tst_resm(TINFO, "Set test process affinity.");
-		printf("mask: %x\n", mask);
+		printf("mask: %lx\n", mask);
 
 		sched_setaffinity(pid, sizeof(unsigned long), &mask);
 
-- 
2.25.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH] ht_affinity.c: fix ht_affinity test failure

[Cyril Hrubis]

Hi!
> The type of cpumask pointer used in set_affinity() is unsigned long, but
> ht_affinity used a unsigned int pointer. When kernel copy cpumask from
> user-space pointer, the high 32bit of cpumask is a random value. So the
> process can't be bind to the cpu specified by users.

Good catch, however it would be better if we used sizeof on the mask
instead of sizeof(unsigned long) in the sched_setaffinity() as well:

diff --git a/testcases/kernel/sched/hyperthreading/ht_affinity/ht_affinity.c b/testcases/kernel/sched/hyperthreading/ht_affinity/ht_affinity.c
index f6e9f2745..3c2fe1bf1 100644
--- a/testcases/kernel/sched/hyperthreading/ht_affinity/ht_affinity.c
+++ b/testcases/kernel/sched/hyperthreading/ht_affinity/ht_affinity.c
@@ -67,7 +67,7 @@ int HT_SetAffinity(void)
                tst_resm(TINFO, "Set test process affinity.");
                printf("mask: %x\n", mask);

-               sched_setaffinity(pid, sizeof(unsigned long), &mask);
+               sched_setaffinity(pid, sizeof(mask), &mask);

                for (j = 0; j < 10; j++) {
                        for (k = 0; k < 10; k++) {
@@ -95,7 +95,7 @@ int HT_SetAffinity(void)
                tst_resm(TINFO, "Set test process affinity.");
                printf("mask: %x\n", mask);

-               sched_setaffinity(pid, sizeof(unsigned long), &mask);
+               sched_setaffinity(pid, sizeof(mask), &mask);

                for (j = 0; j < 10; j++) {
                        for (k = 0; k < 10; k++) {



Most of the code does that already which makes it impossible to pass
different size than the actual size.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH] ht_affinity.c: fix ht_affinity test failure

[Haifeng Xu via ltp]

On 2024/9/20 16:58, Cyril Hrubis wrote:
> Hi!
>> The type of cpumask pointer used in set_affinity() is unsigned long, but
>> ht_affinity used a unsigned int pointer. When kernel copy cpumask from
>> user-space pointer, the high 32bit of cpumask is a random value. So the
>> process can't be bind to the cpu specified by users.
> 
> Good catch, however it would be better if we used sizeof on the mask
> instead of sizeof(unsigned long) in the sched_setaffinity() as well:
> 
> diff --git a/testcases/kernel/sched/hyperthreading/ht_affinity/ht_affinity.c b/testcases/kernel/sched/hyperthreading/ht_affinity/ht_affinity.c
> index f6e9f2745..3c2fe1bf1 100644
> --- a/testcases/kernel/sched/hyperthreading/ht_affinity/ht_affinity.c
> +++ b/testcases/kernel/sched/hyperthreading/ht_affinity/ht_affinity.c
> @@ -67,7 +67,7 @@ int HT_SetAffinity(void)
>                 tst_resm(TINFO, "Set test process affinity.");
>                 printf("mask: %x\n", mask);
> 
> -               sched_setaffinity(pid, sizeof(unsigned long), &mask);
> +               sched_setaffinity(pid, sizeof(mask), &mask);
> 
>                 for (j = 0; j < 10; j++) {
>                         for (k = 0; k < 10; k++) {
> @@ -95,7 +95,7 @@ int HT_SetAffinity(void)
>                 tst_resm(TINFO, "Set test process affinity.");
>                 printf("mask: %x\n", mask);
> 
> -               sched_setaffinity(pid, sizeof(unsigned long), &mask);
> +               sched_setaffinity(pid, sizeof(mask), &mask);
> 
>                 for (j = 0; j < 10; j++) {
>                         for (k = 0; k < 10; k++) {
> 
> 
> 
> Most of the code does that already which makes it impossible to pass
> different size than the actual size.
> 

From the kernel source code, the user_mask_ptr is unsigned long.

SYSCALL_DEFINE3(sched_setaffinity, pid_t, pid, unsigned int, len,
		unsigned long __user *, user_mask_ptr)

so maybe we can keep the type of user_mask_ptr consistent with it.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH] ht_affinity.c: fix ht_affinity test failure

[Cyril Hrubis]

Hi!
> From the kernel source code, the user_mask_ptr is unsigned long.
> 
> SYSCALL_DEFINE3(sched_setaffinity, pid_t, pid, unsigned int, len,
> 		unsigned long __user *, user_mask_ptr)
> 
> so maybe we can keep the type of user_mask_ptr consistent with it.

It's actually accessed as an array. See get_user_cpu_mask() function just
above this syscall definition and userspace is suppose to pass an array
big enough for all the flags of CPUs it cares about.

There are also macros in the libc sched.h header that allows you to
allocate an array big enough for the current machine and manipulate the
bits, see man CPU_ALLOC.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH] ht_affinity.c: fix ht_affinity test failure

[Haifeng Xu via ltp]

On 2024/9/20 20:22, Cyril Hrubis wrote:
> Hi!
>> From the kernel source code, the user_mask_ptr is unsigned long.
>>
>> SYSCALL_DEFINE3(sched_setaffinity, pid_t, pid, unsigned int, len,
>> 		unsigned long __user *, user_mask_ptr)
>>
>> so maybe we can keep the type of user_mask_ptr consistent with it.
> 
> It's actually accessed as an array. See get_user_cpu_mask() function just
> above this syscall definition and userspace is suppose to pass an array
> big enough for all the flags of CPUs it cares about.
> 
> There are also macros in the libc sched.h header that allows you to
> allocate an array big enough for the current machine and manipulate the
> bits, see man CPU_ALLOC.
> 
OK. I'll post next version with your suggestions. 
Thanks!

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v2] ht_affinity.c: fix ht_affinity test failure

[Haifeng Xu via ltp]

Running ht_affinity test fails:

	smt_smp_affinity    0  TINFO  :  Set affinity through system call
	smt_smp_affinity    0  TINFO  :  Set test process affinity.
	mask: 1
	smt_smp_affinity    0  TINFO  :  ...Error

	...

	smt_smp_affinity    0  TINFO  :  Set test process affinity.
	mask: c0000000
	smt_smp_affinity    0  TINFO  :  ...Error
	smt_smp_affinity    3  TFAIL  :  ht_affinity.c:240: System call setaffinity() is error.

The type of cpumask pointer used in set_affinity() is unsigned long, but
ht_affinity used a unsigned int pointer. When kernel copy cpumask from
user-space pointer, the high 32bit of cpumask is a random value. So the
process can't be bind to the cpu specified by users.

After using sizeof on the mask instead of sizeof(unsigned long), ht_affinity
test succeeds:

	smt_smp_affinity    0  TINFO  :  Set affinity through system call
	smt_smp_affinity    0  TINFO  :  Set test process affinity.
	mask: 1
	smt_smp_affinity    0  TINFO  :  ...OK

	...

	smt_smp_affinity    0  TINFO  :  Set test process affinity.
	mask: c0000000
	smt_smp_affinity    0  TINFO  :  ...OK
	smt_smp_affinity    3  TPASS  :  System call setaffinity() is OK.

Signed-off-by: Haifeng Xu <haifeng.xu@shopee.com>
Suggested-by: Cyril Hrubis <chrubis@suse.cz>
---
Changes since v1:
- Don't change the type of the mask and use sizeof on it. 
---
 .../kernel/sched/hyperthreading/ht_affinity/ht_affinity.c     | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/testcases/kernel/sched/hyperthreading/ht_affinity/ht_affinity.c b/testcases/kernel/sched/hyperthreading/ht_affinity/ht_affinity.c
index f6e9f2745..3c2fe1bf1 100644
--- a/testcases/kernel/sched/hyperthreading/ht_affinity/ht_affinity.c
+++ b/testcases/kernel/sched/hyperthreading/ht_affinity/ht_affinity.c
@@ -67,7 +67,7 @@ int HT_SetAffinity(void)
 		tst_resm(TINFO, "Set test process affinity.");
 		printf("mask: %x\n", mask);
 
-		sched_setaffinity(pid, sizeof(unsigned long), &mask);
+		sched_setaffinity(pid, sizeof(mask), &mask);
 
 		for (j = 0; j < 10; j++) {
 			for (k = 0; k < 10; k++) {
@@ -95,7 +95,7 @@ int HT_SetAffinity(void)
 		tst_resm(TINFO, "Set test process affinity.");
 		printf("mask: %x\n", mask);
 
-		sched_setaffinity(pid, sizeof(unsigned long), &mask);
+		sched_setaffinity(pid, sizeof(mask), &mask);
 
 		for (j = 0; j < 10; j++) {
 			for (k = 0; k < 10; k++) {
-- 
2.25.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2] ht_affinity.c: fix ht_affinity test failure

[Cyril Hrubis]

Hi!
Applied, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp