* [LTP] [PATCH] getcpu01: Reinstate node_id test
@ 2022-12-06 16:58 Richard Palethorpe via ltp
2022-12-12 14:01 ` Richard Palethorpe
2022-12-12 15:53 ` Cyril Hrubis
0 siblings, 2 replies; 5+ messages in thread
From: Richard Palethorpe via ltp @ 2022-12-06 16:58 UTC (permalink / raw)
To: ltp; +Cc: Richard Palethorpe
Presently the node_id is only checked on i386 and it is broken. The
sched_getcpu call was substituted for getcpu when
available. sched_getcpu does not have the node_id parameter and does
not even call SYS_getcpu if it can be completed by vDSO.
Also we can at least check the node_id on x86_64 as well.
Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
testcases/kernel/syscalls/getcpu/getcpu01.c | 22 +++++++++------------
1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/testcases/kernel/syscalls/getcpu/getcpu01.c b/testcases/kernel/syscalls/getcpu/getcpu01.c
index fcc273e29..21c67f412 100644
--- a/testcases/kernel/syscalls/getcpu/getcpu01.c
+++ b/testcases/kernel/syscalls/getcpu/getcpu01.c
@@ -15,20 +15,16 @@
#include <stdio.h>
#include <stdlib.h>
#include <sys/types.h>
+#include "tst_test.h"
#include "lapi/syscalls.h"
#include "lapi/cpuset.h"
-#include "tst_test.h"
#include "config.h"
static inline int get_cpu(unsigned *cpu_id,
- unsigned *node_id LTP_ATTRIBUTE_UNUSED,
- void *cache_struct LTP_ATTRIBUTE_UNUSED)
+ unsigned *node_id)
{
-#ifndef HAVE_SCHED_GETCPU
- return tst_syscall(__NR_getcpu, cpu_id, node_id, cache_struct);
-#else
- *cpu_id = sched_getcpu();
-#endif
+ return tst_syscall(__NR_getcpu, cpu_id, node_id, NULL);
+
return 0;
}
@@ -78,7 +74,7 @@ realloc:
return cpu_max;
}
-#ifdef __i386__
+#if defined(__i386__) || defined(__x86_64__)
static unsigned int get_nodeid(unsigned int cpu_id)
{
DIR *directory_parent, *directory_node;
@@ -125,22 +121,22 @@ static void run(void)
{
unsigned int cpu_id, node_id = 0;
unsigned int cpu_set;
-#ifdef __i386__
+#if defined(__i386__) || defined(__x86_64__)
unsigned int node_set;
#endif
cpu_set = set_cpu_affinity();
-#ifdef __i386__
+#if defined(__i386__) || defined(__x86_64__)
node_set = get_nodeid(cpu_set);
#endif
- TEST(get_cpu(&cpu_id, &node_id, NULL));
+ TEST(get_cpu(&cpu_id, &node_id));
if (TST_RET == 0) {
if (cpu_id != cpu_set)
tst_res(TFAIL, "getcpu() returned wrong value"
" expected cpuid:%d, returned value cpuid: %d",
cpu_set, cpu_id);
-#ifdef __i386__
+#if defined(__i386__) || defined(__x86_64__)
else if (node_id != node_set)
tst_res(TFAIL, "getcpu() returned wrong value"
" expected node id:%d returned node id:%d",
--
2.38.1
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [LTP] [PATCH] getcpu01: Reinstate node_id test
2022-12-06 16:58 [LTP] [PATCH] getcpu01: Reinstate node_id test Richard Palethorpe via ltp
@ 2022-12-12 14:01 ` Richard Palethorpe
2022-12-12 15:53 ` Cyril Hrubis
1 sibling, 0 replies; 5+ messages in thread
From: Richard Palethorpe @ 2022-12-12 14:01 UTC (permalink / raw)
To: Richard Palethorpe; +Cc: ltp
Hello,
Ping! I will merge soon if there are no comments.
Richard Palethorpe via ltp <ltp@lists.linux.it> writes:
> Presently the node_id is only checked on i386 and it is broken. The
> sched_getcpu call was substituted for getcpu when
> available. sched_getcpu does not have the node_id parameter and does
> not even call SYS_getcpu if it can be completed by vDSO.
>
> Also we can at least check the node_id on x86_64 as well.
>
> Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
> ---
> testcases/kernel/syscalls/getcpu/getcpu01.c | 22 +++++++++------------
> 1 file changed, 9 insertions(+), 13 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/getcpu/getcpu01.c b/testcases/kernel/syscalls/getcpu/getcpu01.c
> index fcc273e29..21c67f412 100644
> --- a/testcases/kernel/syscalls/getcpu/getcpu01.c
> +++ b/testcases/kernel/syscalls/getcpu/getcpu01.c
> @@ -15,20 +15,16 @@
> #include <stdio.h>
> #include <stdlib.h>
> #include <sys/types.h>
> +#include "tst_test.h"
> #include "lapi/syscalls.h"
> #include "lapi/cpuset.h"
> -#include "tst_test.h"
> #include "config.h"
>
> static inline int get_cpu(unsigned *cpu_id,
> - unsigned *node_id LTP_ATTRIBUTE_UNUSED,
> - void *cache_struct LTP_ATTRIBUTE_UNUSED)
> + unsigned *node_id)
> {
> -#ifndef HAVE_SCHED_GETCPU
> - return tst_syscall(__NR_getcpu, cpu_id, node_id, cache_struct);
> -#else
> - *cpu_id = sched_getcpu();
> -#endif
> + return tst_syscall(__NR_getcpu, cpu_id, node_id, NULL);
> +
> return 0;
> }
>
> @@ -78,7 +74,7 @@ realloc:
> return cpu_max;
> }
>
> -#ifdef __i386__
> +#if defined(__i386__) || defined(__x86_64__)
> static unsigned int get_nodeid(unsigned int cpu_id)
> {
> DIR *directory_parent, *directory_node;
> @@ -125,22 +121,22 @@ static void run(void)
> {
> unsigned int cpu_id, node_id = 0;
> unsigned int cpu_set;
> -#ifdef __i386__
> +#if defined(__i386__) || defined(__x86_64__)
> unsigned int node_set;
> #endif
>
> cpu_set = set_cpu_affinity();
> -#ifdef __i386__
> +#if defined(__i386__) || defined(__x86_64__)
> node_set = get_nodeid(cpu_set);
> #endif
>
> - TEST(get_cpu(&cpu_id, &node_id, NULL));
> + TEST(get_cpu(&cpu_id, &node_id));
> if (TST_RET == 0) {
> if (cpu_id != cpu_set)
> tst_res(TFAIL, "getcpu() returned wrong value"
> " expected cpuid:%d, returned value cpuid: %d",
> cpu_set, cpu_id);
> -#ifdef __i386__
> +#if defined(__i386__) || defined(__x86_64__)
> else if (node_id != node_set)
> tst_res(TFAIL, "getcpu() returned wrong value"
> " expected node id:%d returned node id:%d",
> --
> 2.38.1
--
Thank you,
Richard.
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [LTP] [PATCH] getcpu01: Reinstate node_id test
2022-12-06 16:58 [LTP] [PATCH] getcpu01: Reinstate node_id test Richard Palethorpe via ltp
2022-12-12 14:01 ` Richard Palethorpe
@ 2022-12-12 15:53 ` Cyril Hrubis
2022-12-12 16:07 ` Richard Palethorpe
1 sibling, 1 reply; 5+ messages in thread
From: Cyril Hrubis @ 2022-12-12 15:53 UTC (permalink / raw)
To: Richard Palethorpe; +Cc: ltp
Hi!
> Presently the node_id is only checked on i386 and it is broken. The
> sched_getcpu call was substituted for getcpu when
> available. sched_getcpu does not have the node_id parameter and does
> not even call SYS_getcpu if it can be completed by vDSO.
>
> Also we can at least check the node_id on x86_64 as well.
I suppose that the getcpu manual page is a bit confusing, the function
is implemented on most of the major achitectures and as VDSO on most of
them too. It was only added first to x86 architectures.
> Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
> ---
> testcases/kernel/syscalls/getcpu/getcpu01.c | 22 +++++++++------------
> 1 file changed, 9 insertions(+), 13 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/getcpu/getcpu01.c b/testcases/kernel/syscalls/getcpu/getcpu01.c
> index fcc273e29..21c67f412 100644
> --- a/testcases/kernel/syscalls/getcpu/getcpu01.c
> +++ b/testcases/kernel/syscalls/getcpu/getcpu01.c
> @@ -15,20 +15,16 @@
> #include <stdio.h>
> #include <stdlib.h>
> #include <sys/types.h>
> +#include "tst_test.h"
> #include "lapi/syscalls.h"
> #include "lapi/cpuset.h"
> -#include "tst_test.h"
> #include "config.h"
>
> static inline int get_cpu(unsigned *cpu_id,
> - unsigned *node_id LTP_ATTRIBUTE_UNUSED,
> - void *cache_struct LTP_ATTRIBUTE_UNUSED)
> + unsigned *node_id)
> {
> -#ifndef HAVE_SCHED_GETCPU
> - return tst_syscall(__NR_getcpu, cpu_id, node_id, cache_struct);
> -#else
> - *cpu_id = sched_getcpu();
> -#endif
> + return tst_syscall(__NR_getcpu, cpu_id, node_id, NULL);
The call is mostly implemneted as VDSO so it would make much more sense
to test the libc function instead and test the implementation that is
actually used in production.
> return 0;
> }
>
> @@ -78,7 +74,7 @@ realloc:
> return cpu_max;
> }
>
> -#ifdef __i386__
> +#if defined(__i386__) || defined(__x86_64__)
> static unsigned int get_nodeid(unsigned int cpu_id)
> {
> DIR *directory_parent, *directory_node;
> @@ -125,22 +121,22 @@ static void run(void)
> {
> unsigned int cpu_id, node_id = 0;
> unsigned int cpu_set;
The get_nodeid() just parses sysfs, that should be portable, can't we
just get rid of the ifdefs completely instead?
> -#ifdef __i386__
> +#if defined(__i386__) || defined(__x86_64__)
> unsigned int node_set;
> #endif
>
> cpu_set = set_cpu_affinity();
> -#ifdef __i386__
> +#if defined(__i386__) || defined(__x86_64__)
> node_set = get_nodeid(cpu_set);
> #endif
>
> - TEST(get_cpu(&cpu_id, &node_id, NULL));
> + TEST(get_cpu(&cpu_id, &node_id));
> if (TST_RET == 0) {
> if (cpu_id != cpu_set)
> tst_res(TFAIL, "getcpu() returned wrong value"
> " expected cpuid:%d, returned value cpuid: %d",
> cpu_set, cpu_id);
> -#ifdef __i386__
> +#if defined(__i386__) || defined(__x86_64__)
> else if (node_id != node_set)
> tst_res(TFAIL, "getcpu() returned wrong value"
> " expected node id:%d returned node id:%d",
> --
> 2.38.1
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
--
Cyril Hrubis
chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [LTP] [PATCH] getcpu01: Reinstate node_id test
2022-12-12 15:53 ` Cyril Hrubis
@ 2022-12-12 16:07 ` Richard Palethorpe
2022-12-13 9:48 ` Petr Vorel
0 siblings, 1 reply; 5+ messages in thread
From: Richard Palethorpe @ 2022-12-12 16:07 UTC (permalink / raw)
To: Cyril Hrubis; +Cc: ltp
Hello,
Cyril Hrubis <chrubis@suse.cz> writes:
> Hi!
>> Presently the node_id is only checked on i386 and it is broken. The
>> sched_getcpu call was substituted for getcpu when
>> available. sched_getcpu does not have the node_id parameter and does
>> not even call SYS_getcpu if it can be completed by vDSO.
>>
>> Also we can at least check the node_id on x86_64 as well.
>
> I suppose that the getcpu manual page is a bit confusing, the function
> is implemented on most of the major achitectures and as VDSO on most of
> them too. It was only added first to x86 architectures.
I suppose I overlooked that there is a libc implementation of
getcpu. IDK why sched_getcpu is here at all and my vDSO comment is just
about how sched_getcpu would not result in a call to getcpu. However I
agree we should just call the libc version.
>
>> Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
>> ---
>> testcases/kernel/syscalls/getcpu/getcpu01.c | 22 +++++++++------------
>> 1 file changed, 9 insertions(+), 13 deletions(-)
>>
>> diff --git a/testcases/kernel/syscalls/getcpu/getcpu01.c b/testcases/kernel/syscalls/getcpu/getcpu01.c
>> index fcc273e29..21c67f412 100644
>> --- a/testcases/kernel/syscalls/getcpu/getcpu01.c
>> +++ b/testcases/kernel/syscalls/getcpu/getcpu01.c
>> @@ -15,20 +15,16 @@
>> #include <stdio.h>
>> #include <stdlib.h>
>> #include <sys/types.h>
>> +#include "tst_test.h"
>> #include "lapi/syscalls.h"
>> #include "lapi/cpuset.h"
>> -#include "tst_test.h"
>> #include "config.h"
>>
>> static inline int get_cpu(unsigned *cpu_id,
>> - unsigned *node_id LTP_ATTRIBUTE_UNUSED,
>> - void *cache_struct LTP_ATTRIBUTE_UNUSED)
>> + unsigned *node_id)
>> {
>> -#ifndef HAVE_SCHED_GETCPU
>> - return tst_syscall(__NR_getcpu, cpu_id, node_id, cache_struct);
>> -#else
>> - *cpu_id = sched_getcpu();
>> -#endif
>> + return tst_syscall(__NR_getcpu, cpu_id, node_id, NULL);
>
> The call is mostly implemneted as VDSO so it would make much more sense
> to test the libc function instead and test the implementation that is
> actually used in production.
>
>> return 0;
>> }
>>
>> @@ -78,7 +74,7 @@ realloc:
>> return cpu_max;
>> }
>>
>> -#ifdef __i386__
>> +#if defined(__i386__) || defined(__x86_64__)
>> static unsigned int get_nodeid(unsigned int cpu_id)
>> {
>> DIR *directory_parent, *directory_node;
>> @@ -125,22 +121,22 @@ static void run(void)
>> {
>> unsigned int cpu_id, node_id = 0;
>> unsigned int cpu_set;
>
>
> The get_nodeid() just parses sysfs, that should be portable, can't we
> just get rid of the ifdefs completely instead?
That's what I thought too. I Will remove them.
>
>> -#ifdef __i386__
>> +#if defined(__i386__) || defined(__x86_64__)
>> unsigned int node_set;
>> #endif
>>
>> cpu_set = set_cpu_affinity();
>> -#ifdef __i386__
>> +#if defined(__i386__) || defined(__x86_64__)
>> node_set = get_nodeid(cpu_set);
>> #endif
>>
>> - TEST(get_cpu(&cpu_id, &node_id, NULL));
>> + TEST(get_cpu(&cpu_id, &node_id));
>> if (TST_RET == 0) {
>> if (cpu_id != cpu_set)
>> tst_res(TFAIL, "getcpu() returned wrong value"
>> " expected cpuid:%d, returned value cpuid: %d",
>> cpu_set, cpu_id);
>> -#ifdef __i386__
>> +#if defined(__i386__) || defined(__x86_64__)
>> else if (node_id != node_set)
>> tst_res(TFAIL, "getcpu() returned wrong value"
>> " expected node id:%d returned node id:%d",
>> --
>> 2.38.1
>>
>>
>> --
>> Mailing list info: https://lists.linux.it/listinfo/ltp
Also I guess the kver check for 2.6 can be removed.
--
Thank you,
Richard.
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [LTP] [PATCH] getcpu01: Reinstate node_id test
2022-12-12 16:07 ` Richard Palethorpe
@ 2022-12-13 9:48 ` Petr Vorel
0 siblings, 0 replies; 5+ messages in thread
From: Petr Vorel @ 2022-12-13 9:48 UTC (permalink / raw)
To: Richard Palethorpe; +Cc: ltp
Hi all,
...
> Also I guess the kver check for 2.6 can be removed.
This is part o Xu's patchset, I'd keep it there (otherwise that patchset will
not be applicable):
https://patchwork.ozlabs.org/project/ltp/patch/1670845229-1981-3-git-send-email-xuyang2018.jy@fujitsu.com/
Kind regards,
Petr
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-12-13 9:48 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-06 16:58 [LTP] [PATCH] getcpu01: Reinstate node_id test Richard Palethorpe via ltp
2022-12-12 14:01 ` Richard Palethorpe
2022-12-12 15:53 ` Cyril Hrubis
2022-12-12 16:07 ` Richard Palethorpe
2022-12-13 9:48 ` Petr Vorel
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.