[LTP] [PATCH v2 0/4] sched_{g,s}etattr01: Add missing policies

[LTP] [PATCH v2 0/4] sched_{g,s}etattr01: Add missing policies

[Petr Vorel]

Hi,

changes v1->v2:
3 new commits

sched_getattr and sched_setattr are 99% identical (2 values are
different). I was thinking to use the same approach from e197796f22
("sethostname: Convert to new API"), but not sure if it's a good
approach.

Do we want to reduce files needed to be updated after new policy is
added? If yes, shouldn't there be just a single directory?
(what name should be using to show 2 syscalls are in sources in this
directory?)

Also docparse expect source file to be just for one binary, thus not
sure whether it should just document both binaries (and thus also
rename source file to show 2 syscalls are for this source?)

Kind regards,
Petr

Petr Vorel (4):
  lapi/sched.h: Add include <sched.h>
  lapi: Move SCHED_DEADLINE definition from tests
  sched_get_priority_min01: Add missing policies
  sched_get_priority_max01: Add missing policies

 include/lapi/sched.h                             | 16 +++++++++++-----
 testcases/kernel/syscalls/kcmp/kcmp03.c          |  1 -
 .../sched_get_priority_max01.c                   | 13 +++++++++----
 .../sched_get_priority_min01.c                   | 13 +++++++++----
 .../syscalls/sched_getattr/sched_getattr01.c     |  3 ---
 .../syscalls/sched_getattr/sched_getattr02.c     |  2 --
 .../syscalls/sched_setattr/sched_setattr01.c     |  2 --
 7 files changed, 29 insertions(+), 21 deletions(-)

-- 
2.34.1


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

[LTP] [PATCH v2 1/4] lapi/sched.h: Add include <sched.h>

[Petr Vorel]

We including headers in lapi headers instead of depending headers are
included in tests which use lapi headers.

Also remove redundant include lapi/syscalls.h from tests which use
lapi/sched.h (included in lapi/sched.h).

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
New in v2

 include/lapi/sched.h                                      | 3 ++-
 testcases/kernel/syscalls/kcmp/kcmp03.c                   | 1 -
 testcases/kernel/syscalls/sched_getattr/sched_getattr01.c | 2 --
 testcases/kernel/syscalls/sched_getattr/sched_getattr02.c | 2 --
 testcases/kernel/syscalls/sched_setattr/sched_setattr01.c | 1 -
 5 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/include/lapi/sched.h b/include/lapi/sched.h
index 036edd51d1..2fc32a38ca 100644
--- a/include/lapi/sched.h
+++ b/include/lapi/sched.h
@@ -6,9 +6,10 @@
 #ifndef LAPI_SCHED_H__
 #define LAPI_SCHED_H__
 
-#include "lapi/syscalls.h"
+#include <sched.h>
 #include <stdint.h>
 #include <inttypes.h>
+#include "lapi/syscalls.h"
 
 struct sched_attr {
 	uint32_t size;
diff --git a/testcases/kernel/syscalls/kcmp/kcmp03.c b/testcases/kernel/syscalls/kcmp/kcmp03.c
index 30ac3ec9db..df8633c5fc 100644
--- a/testcases/kernel/syscalls/kcmp/kcmp03.c
+++ b/testcases/kernel/syscalls/kcmp/kcmp03.c
@@ -18,7 +18,6 @@
 
 #include <errno.h>
 #include <stdlib.h>
-#include <sched.h>
 #include <sys/wait.h>
 #include "tst_test.h"
 #include "kcmp.h"
diff --git a/testcases/kernel/syscalls/sched_getattr/sched_getattr01.c b/testcases/kernel/syscalls/sched_getattr/sched_getattr01.c
index 2a2c13a40f..0ffbce9c8d 100644
--- a/testcases/kernel/syscalls/sched_getattr/sched_getattr01.c
+++ b/testcases/kernel/syscalls/sched_getattr/sched_getattr01.c
@@ -22,11 +22,9 @@
 #include <linux/types.h>
 #include <sys/syscall.h>
 #include <pthread.h>
-#include <sched.h>
 #include <errno.h>
 
 #include "test.h"
-#include "lapi/syscalls.h"
 #include "lapi/sched.h"
 
 char *TCID = "sched_getattr01";
diff --git a/testcases/kernel/syscalls/sched_getattr/sched_getattr02.c b/testcases/kernel/syscalls/sched_getattr/sched_getattr02.c
index 2d7e15a470..9f4a09f88f 100644
--- a/testcases/kernel/syscalls/sched_getattr/sched_getattr02.c
+++ b/testcases/kernel/syscalls/sched_getattr/sched_getattr02.c
@@ -31,11 +31,9 @@
 #include <linux/types.h>
 #include <sys/syscall.h>
 #include <pthread.h>
-#include <sched.h>
 #include <errno.h>
 
 #include "test.h"
-#include "lapi/syscalls.h"
 #include "lapi/sched.h"
 
 char *TCID = "sched_getattr02";
diff --git a/testcases/kernel/syscalls/sched_setattr/sched_setattr01.c b/testcases/kernel/syscalls/sched_setattr/sched_setattr01.c
index 37ca56a0aa..82156b95cb 100644
--- a/testcases/kernel/syscalls/sched_setattr/sched_setattr01.c
+++ b/testcases/kernel/syscalls/sched_setattr/sched_setattr01.c
@@ -29,7 +29,6 @@
 #include <linux/types.h>
 #include <sys/syscall.h>
 #include <pthread.h>
-#include <sched.h>
 #include <errno.h>
 
 #include "test.h"
-- 
2.34.1


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

[LTP] [PATCH v2 2/4] lapi: Move SCHED_DEADLINE definition from tests

[Petr Vorel]

DRY

SCHED_DEADLINE is currently missing on uclibc-ng (v1.0.40).

+ add indent for definitions (formatting).

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
New in v2

 include/lapi/sched.h                                | 13 +++++++++----
 .../kernel/syscalls/sched_getattr/sched_getattr01.c |  1 -
 .../kernel/syscalls/sched_setattr/sched_setattr01.c |  1 -
 3 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/include/lapi/sched.h b/include/lapi/sched.h
index 2fc32a38ca..226d3109a0 100644
--- a/include/lapi/sched.h
+++ b/include/lapi/sched.h
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright (c) 2015 Cui Bixuan <cuibixuan@huawei.com>
+ * Copyright (c) Linux Test Project, 2016-2022
  */
 
 #ifndef LAPI_SCHED_H__
@@ -41,20 +42,24 @@ static inline int sched_getattr(pid_t pid, struct sched_attr *attr,
 	return syscall(__NR_sched_getattr, pid, attr, size, flags);
 }
 
+#ifndef SCHED_DEADLINE
+# define SCHED_DEADLINE	6
+#endif
+
 #ifndef CLONE_VM
-#define CLONE_VM   0x00000100
+# define CLONE_VM   0x00000100
 #endif
 
 #ifndef CLONE_FS
-#define CLONE_FS   0x00000200
+# define CLONE_FS   0x00000200
 #endif
 
 #ifndef CLONE_SYSVSEM
-#define CLONE_SYSVSEM   0x00040000
+# define CLONE_SYSVSEM   0x00040000
 #endif
 
 #ifndef CLONE_IO
-#define CLONE_IO        0x80000000
+# define CLONE_IO        0x80000000
 #endif
 
 #endif /* LAPI_SCHED_H__ */
diff --git a/testcases/kernel/syscalls/sched_getattr/sched_getattr01.c b/testcases/kernel/syscalls/sched_getattr/sched_getattr01.c
index 0ffbce9c8d..c1715d8acf 100644
--- a/testcases/kernel/syscalls/sched_getattr/sched_getattr01.c
+++ b/testcases/kernel/syscalls/sched_getattr/sched_getattr01.c
@@ -30,7 +30,6 @@
 char *TCID = "sched_getattr01";
 int TST_TOTAL = 1;
 
-#define SCHED_DEADLINE	6
 #define RUNTIME_VAL 10000000
 #define PERIOD_VAL 30000000
 #define DEADLINE_VAL 30000000
diff --git a/testcases/kernel/syscalls/sched_setattr/sched_setattr01.c b/testcases/kernel/syscalls/sched_setattr/sched_setattr01.c
index 82156b95cb..d5178e0169 100644
--- a/testcases/kernel/syscalls/sched_setattr/sched_setattr01.c
+++ b/testcases/kernel/syscalls/sched_setattr/sched_setattr01.c
@@ -36,7 +36,6 @@
 
 char *TCID = "sched_setattr01";
 
-#define SCHED_DEADLINE	6
 #define RUNTIME_VAL 10000000
 #define PERIOD_VAL 30000000
 #define DEADLINE_VAL 30000000
-- 
2.34.1


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

[LTP] [PATCH v2 3/4] sched_get_priority_min01: Add missing policies

[Petr Vorel]

+ improve doc wording and update copyright.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
New in v2

 .../sched_get_priority_min01.c                      | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/testcases/kernel/syscalls/sched_get_priority_min/sched_get_priority_min01.c b/testcases/kernel/syscalls/sched_get_priority_min/sched_get_priority_min01.c
index cebd08d3f9..e7ab761249 100644
--- a/testcases/kernel/syscalls/sched_get_priority_min/sched_get_priority_min01.c
+++ b/testcases/kernel/syscalls/sched_get_priority_min/sched_get_priority_min01.c
@@ -2,6 +2,7 @@
 /*
  * Copyright (c) Wipro Technologies Ltd, 2002.  All Rights Reserved.
  * Copyright (c) 2021 sujiaxun <sujiaxun@uniontech.com>
+ * Copyright (c) Linux Test Project, 2009-2022
  */
 
 /*\
@@ -9,11 +10,12 @@
  *
  * Basic test for the sched_get_priority_min(2) system call.
  *
- * Obtain different minimum priority scheduling strategies and
+ * Obtain different minimum priority for different schedulling policies and
  * compare them with the expected value.
  */
 
-#include <errno.h>
+#define _GNU_SOURCE
+
 #include <sched.h>
 #include "tst_test.h"
 #include "lapi/syscalls.h"
@@ -25,9 +27,12 @@ static struct test_case {
 	int policy;
 	int retval;
 } tcases[] = {
-	{POLICY_DESC(SCHED_OTHER), 0},
+	{POLICY_DESC(SCHED_BATCH), 0},
+	{POLICY_DESC(SCHED_DEADLINE), 0},
 	{POLICY_DESC(SCHED_FIFO), 1},
-	{POLICY_DESC(SCHED_RR), 1}
+	{POLICY_DESC(SCHED_IDLE), 0},
+	{POLICY_DESC(SCHED_OTHER), 0},
+	{POLICY_DESC(SCHED_RR), 1},
 };
 
 static void run_test(unsigned int nr)
-- 
2.34.1


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

[LTP] [PATCH v2 4/4] sched_get_priority_max01: Add missing policies

[Petr Vorel]

+ improve doc wording and update copyright.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
changes v1->v2:
* code is the same, only copyright and doc updated

 .../sched_get_priority_max01.c                      | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/testcases/kernel/syscalls/sched_get_priority_max/sched_get_priority_max01.c b/testcases/kernel/syscalls/sched_get_priority_max/sched_get_priority_max01.c
index f6fe1add9c..b9a58a46a4 100644
--- a/testcases/kernel/syscalls/sched_get_priority_max/sched_get_priority_max01.c
+++ b/testcases/kernel/syscalls/sched_get_priority_max/sched_get_priority_max01.c
@@ -2,6 +2,7 @@
 /*
  * Copyright (c) Wipro Technologies Ltd, 2002.  All Rights Reserved.
  * Copyright (c) 2021 sujiaxun <sujiaxun@uniontech.com>
+ * Copyright (c) Linux Test Project, 2009-2022
  */
 
 /*\
@@ -10,9 +11,11 @@
  * Basic test for the sched_get_priority_max(2) system call.
  *
  * Obtain different maximum priority for different schedulling policies and
- * compare them with expected value.
+ * compare them with the expected value.
  */
 
+#define _GNU_SOURCE
+
 #include <sched.h>
 #include "tst_test.h"
 #include "lapi/syscalls.h"
@@ -24,14 +27,16 @@ static struct test_case {
 	int policy;
 	int retval;
 } tcases[] = {
-	{POLICY_DESC(SCHED_OTHER), 0},
+	{POLICY_DESC(SCHED_BATCH), 0},
+	{POLICY_DESC(SCHED_DEADLINE), 0},
 	{POLICY_DESC(SCHED_FIFO), 99},
-	{POLICY_DESC(SCHED_RR), 99}
+	{POLICY_DESC(SCHED_IDLE), 0},
+	{POLICY_DESC(SCHED_OTHER), 0},
+	{POLICY_DESC(SCHED_RR), 99},
 };
 
 static void run_test(unsigned int nr)
 {
-
 	struct test_case *tc = &tcases[nr];
 
 	TST_EXP_VAL(tst_syscall(__NR_sched_get_priority_max, tc->policy),
-- 
2.34.1


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

Re: [LTP] [PATCH v2 3/4] sched_get_priority_min01: Add missing policies

[Petr Vorel]

Hi,

this test and sched_get_priority_max01 (latest commit) need to include lapi/sched.h.
I'll fix it before merge.

Kind regards,
Petr

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

Re: [LTP] [PATCH v2 3/4] sched_get_priority_min01: Add missing policies

[Petr Vorel]

> this test and sched_get_priority_max01 (latest commit) need to include lapi/sched.h.
> I'll fix it before merge.
For a record: on traditional distros with glibc it's required only on CentOS 7
and Ubuntu Xenial (very old distros, but Centos 7 EOL in 2024).

Kind regards,
Petr

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

Re: [LTP] [PATCH v2 0/4] sched_{g,s}etattr01: Add missing policies

[Cyril Hrubis]

Hi!
> sched_getattr and sched_setattr are 99% identical (2 values are
> different). I was thinking to use the same approach from e197796f22
> ("sethostname: Convert to new API"), but not sure if it's a good
> approach.

Actually I do not think that the approach in sethostname is good. There
should be a C file for each test. If they share code that should be put
into headers or libraries.

We used to have more tests like that that build binaries in different
directories from a single source with different macros and I find it
utterly confusing.

> Do we want to reduce files needed to be updated after new policy is
> added? If yes, shouldn't there be just a single directory?
> (what name should be using to show 2 syscalls are in sources in this
> directory?)

I would vote against this.

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v2 2/4] lapi: Move SCHED_DEADLINE definition from tests

[Cyril Hrubis]

Hi!
Looks obviously fine.

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v2 3/4] sched_get_priority_min01: Add missing policies

[Cyril Hrubis]

Hi!
Looks good.

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v2 4/4] sched_get_priority_max01: Add missing policies

[Cyril Hrubis]

Hi!
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v2 0/4] sched_{g,s}etattr01: Add missing policies

[Petr Vorel]

Hi Cyril,

> Hi!
> > sched_getattr and sched_setattr are 99% identical (2 values are
> > different). I was thinking to use the same approach from e197796f22
> > ("sethostname: Convert to new API"), but not sure if it's a good
> > approach.

> Actually I do not think that the approach in sethostname is good. There
> should be a C file for each test. If they share code that should be put
> into headers or libraries.

> We used to have more tests like that that build binaries in different
> directories from a single source with different macros and I find it
> utterly confusing.
Thanks for info. Agree, it's confusing.

I guess in tests which are very simple like sethostname or even these
sched_getattr we'll just endup with duplicity, right?
Because putting one function into header which is shared with tests in different
directory is just confusing and not worth of doing.

So I can recreate sethostname01.c.

And for these tests I can make a note just to remember update struct for the
other test.

> > Do we want to reduce files needed to be updated after new policy is
> > added? If yes, shouldn't there be just a single directory?
> > (what name should be using to show 2 syscalls are in sources in this
> > directory?)

> I would vote against this.
Understand now.

Kind regards,
Petr

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

Re: [LTP] [PATCH v2 1/4] lapi/sched.h: Add include <sched.h>

[Cyril Hrubis]

Hi!
> We including headers in lapi headers instead of depending headers are
> included in tests which use lapi headers.

-ENOPARSE

The patch itself looks good though.

So maybe this should be something as:

Include <sched.h> and "lapi/syscalls.h" in "lapi/sched.h" instead of the
tests.

> Also remove redundant include lapi/syscalls.h from tests which use
> lapi/sched.h (included in lapi/sched.h).
> 
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> New in v2
> 
>  include/lapi/sched.h                                      | 3 ++-
>  testcases/kernel/syscalls/kcmp/kcmp03.c                   | 1 -
>  testcases/kernel/syscalls/sched_getattr/sched_getattr01.c | 2 --
>  testcases/kernel/syscalls/sched_getattr/sched_getattr02.c | 2 --
>  testcases/kernel/syscalls/sched_setattr/sched_setattr01.c | 1 -
>  5 files changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/include/lapi/sched.h b/include/lapi/sched.h
> index 036edd51d1..2fc32a38ca 100644
> --- a/include/lapi/sched.h
> +++ b/include/lapi/sched.h
> @@ -6,9 +6,10 @@
>  #ifndef LAPI_SCHED_H__
>  #define LAPI_SCHED_H__
>  
> -#include "lapi/syscalls.h"
> +#include <sched.h>
>  #include <stdint.h>
>  #include <inttypes.h>
> +#include "lapi/syscalls.h"
>  
>  struct sched_attr {
>  	uint32_t size;
> diff --git a/testcases/kernel/syscalls/kcmp/kcmp03.c b/testcases/kernel/syscalls/kcmp/kcmp03.c
> index 30ac3ec9db..df8633c5fc 100644
> --- a/testcases/kernel/syscalls/kcmp/kcmp03.c
> +++ b/testcases/kernel/syscalls/kcmp/kcmp03.c
> @@ -18,7 +18,6 @@
>  
>  #include <errno.h>
>  #include <stdlib.h>
> -#include <sched.h>
>  #include <sys/wait.h>
>  #include "tst_test.h"
>  #include "kcmp.h"
> diff --git a/testcases/kernel/syscalls/sched_getattr/sched_getattr01.c b/testcases/kernel/syscalls/sched_getattr/sched_getattr01.c
> index 2a2c13a40f..0ffbce9c8d 100644
> --- a/testcases/kernel/syscalls/sched_getattr/sched_getattr01.c
> +++ b/testcases/kernel/syscalls/sched_getattr/sched_getattr01.c
> @@ -22,11 +22,9 @@
>  #include <linux/types.h>
>  #include <sys/syscall.h>
>  #include <pthread.h>
> -#include <sched.h>
>  #include <errno.h>
>  
>  #include "test.h"
> -#include "lapi/syscalls.h"
>  #include "lapi/sched.h"
>  
>  char *TCID = "sched_getattr01";
> diff --git a/testcases/kernel/syscalls/sched_getattr/sched_getattr02.c b/testcases/kernel/syscalls/sched_getattr/sched_getattr02.c
> index 2d7e15a470..9f4a09f88f 100644
> --- a/testcases/kernel/syscalls/sched_getattr/sched_getattr02.c
> +++ b/testcases/kernel/syscalls/sched_getattr/sched_getattr02.c
> @@ -31,11 +31,9 @@
>  #include <linux/types.h>
>  #include <sys/syscall.h>
>  #include <pthread.h>
> -#include <sched.h>
>  #include <errno.h>
>  
>  #include "test.h"
> -#include "lapi/syscalls.h"
>  #include "lapi/sched.h"
>  
>  char *TCID = "sched_getattr02";
> diff --git a/testcases/kernel/syscalls/sched_setattr/sched_setattr01.c b/testcases/kernel/syscalls/sched_setattr/sched_setattr01.c
> index 37ca56a0aa..82156b95cb 100644
> --- a/testcases/kernel/syscalls/sched_setattr/sched_setattr01.c
> +++ b/testcases/kernel/syscalls/sched_setattr/sched_setattr01.c
> @@ -29,7 +29,6 @@
>  #include <linux/types.h>
>  #include <sys/syscall.h>
>  #include <pthread.h>
> -#include <sched.h>
>  #include <errno.h>
>  
>  #include "test.h"
> -- 
> 2.34.1
> 

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v2 1/4] lapi/sched.h: Add include <sched.h>

[Cyril Hrubis]

Hi!
> > We including headers in lapi headers instead of depending headers are
> > included in tests which use lapi headers.
> 
> -ENOPARSE
> 
> The patch itself looks good though.
> 
> So maybe this should be something as:
> 
> Include <sched.h> and "lapi/syscalls.h" in "lapi/sched.h" instead of the
> tests.

Also with the description fixed:

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v2 1/4] lapi/sched.h: Add include <sched.h>

[Petr Vorel]

Hi Cyril,

> Hi!
> > We including headers in lapi headers instead of depending headers are
> > included in tests which use lapi headers.

> -ENOPARSE
:)

> The patch itself looks good though.

> So maybe this should be something as:

> Include <sched.h> and "lapi/syscalls.h" in "lapi/sched.h" instead of the
> tests.

Thanks! Fixed and merged.

Kind regards,
Petr

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