[PATCH 1/2] tools/rtla: drop __NR_sched_getattr

[PATCH 1/2] tools/rtla: drop __NR_sched_getattr

[Jan Stancek]

It's not used since commit 084ce16df0f0 ("tools/rtla:
Remove unused sched_getattr() function").

Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
 tools/tracing/rtla/src/utils.c | 32 ++++++++++++++------------------
 1 file changed, 14 insertions(+), 18 deletions(-)

diff --git a/tools/tracing/rtla/src/utils.c b/tools/tracing/rtla/src/utils.c
index 9ac71a66840c..05b2b3fc005e 100644
--- a/tools/tracing/rtla/src/utils.c
+++ b/tools/tracing/rtla/src/utils.c
@@ -211,24 +211,20 @@ long parse_ns_duration(char *val)
 /*
  * This is a set of helper functions to use SCHED_DEADLINE.
  */
-#ifdef __x86_64__
-# define __NR_sched_setattr	314
-# define __NR_sched_getattr	315
-#elif __i386__
-# define __NR_sched_setattr	351
-# define __NR_sched_getattr	352
-#elif __arm__
-# define __NR_sched_setattr	380
-# define __NR_sched_getattr	381
-#elif __aarch64__ || __riscv
-# define __NR_sched_setattr	274
-# define __NR_sched_getattr	275
-#elif __powerpc__
-# define __NR_sched_setattr	355
-# define __NR_sched_getattr	356
-#elif __s390x__
-# define __NR_sched_setattr	345
-# define __NR_sched_getattr	346
+#ifndef __NR_sched_setattr
+# ifdef __x86_64__
+#  define __NR_sched_setattr	314
+# elif __i386__
+#  define __NR_sched_setattr	351
+# elif __arm__
+#  define __NR_sched_setattr	380
+# elif __aarch64__ || __riscv
+#  define __NR_sched_setattr	274
+# elif __powerpc__
+#  define __NR_sched_setattr	355
+# elif __s390x__
+#  define __NR_sched_setattr	345
+# endif
 #endif
 
 #define SCHED_DEADLINE		6
-- 
2.43.0

[PATCH 2/2] tools/rtla: fix collision with glibc sched_attr/sched_set_attr

[Jan Stancek]

glibc commit 21571ca0d703 ("Linux: Add the sched_setattr
and sched_getattr functions") now also provides 'struct sched_attr'
and sched_setattr() which collide with the ones from rtla.

  In file included from src/trace.c:11:
  src/utils.h:49:8: error: redefinition of ‘struct sched_attr’
     49 | struct sched_attr {
        |        ^~~~~~~~~~
  In file included from /usr/include/bits/sched.h:60,
                   from /usr/include/sched.h:43,
                   from /usr/include/tracefs/tracefs.h:10,
                   from src/trace.c:4:
  /usr/include/linux/sched/types.h:98:8: note: originally defined here
     98 | struct sched_attr {
        |        ^~~~~~~~~~

Define 'struct sched_attr' conditionally, similar to what strace did:
  https://lore.kernel.org/all/20240930222913.3981407-1-raj.khem@gmail.com/
and rename rtla's version of sched_setattr() to avoid collision.

Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
 tools/tracing/rtla/src/utils.c | 4 ++--
 tools/tracing/rtla/src/utils.h | 2 ++
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/tracing/rtla/src/utils.c b/tools/tracing/rtla/src/utils.c
index 05b2b3fc005e..c62044215a48 100644
--- a/tools/tracing/rtla/src/utils.c
+++ b/tools/tracing/rtla/src/utils.c
@@ -229,7 +229,7 @@ long parse_ns_duration(char *val)
 
 #define SCHED_DEADLINE		6
 
-static inline int sched_setattr(pid_t pid, const struct sched_attr *attr,
+static inline int rtla_sched_setattr(pid_t pid, const struct sched_attr *attr,
 				unsigned int flags) {
 	return syscall(__NR_sched_setattr, pid, attr, flags);
 }
@@ -239,7 +239,7 @@ int __set_sched_attr(int pid, struct sched_attr *attr)
 	int flags = 0;
 	int retval;
 
-	retval = sched_setattr(pid, attr, flags);
+	retval = rtla_sched_setattr(pid, attr, flags);
 	if (retval < 0) {
 		err_msg("Failed to set sched attributes to the pid %d: %s\n",
 			pid, strerror(errno));
diff --git a/tools/tracing/rtla/src/utils.h b/tools/tracing/rtla/src/utils.h
index d44513e6c66a..99c9cf81bcd0 100644
--- a/tools/tracing/rtla/src/utils.h
+++ b/tools/tracing/rtla/src/utils.h
@@ -46,6 +46,7 @@ update_sum(unsigned long long *a, unsigned long long *b)
 	*a += *b;
 }
 
+#ifndef SCHED_ATTR_SIZE_VER0
 struct sched_attr {
 	uint32_t size;
 	uint32_t sched_policy;
@@ -56,6 +57,7 @@ struct sched_attr {
 	uint64_t sched_deadline;
 	uint64_t sched_period;
 };
+#endif /* SCHED_ATTR_SIZE_VER0 */
 
 int parse_prio(char *arg, struct sched_attr *sched_param);
 int parse_cpu_set(char *cpu_list, cpu_set_t *set);
-- 
2.43.0

Re: [PATCH 2/2] tools/rtla: fix collision with glibc sched_attr/sched_set_attr

[Tomas Glozar]

čt 10. 10. 2024 v 11:33 odesílatel Jan Stancek <jstancek@redhat.com> napsal:
>
> glibc commit 21571ca0d703 ("Linux: Add the sched_setattr
> and sched_getattr functions") now also provides 'struct sched_attr'
> and sched_setattr() which collide with the ones from rtla.
>
>   In file included from src/trace.c:11:
>   src/utils.h:49:8: error: redefinition of ‘struct sched_attr’
>      49 | struct sched_attr {
>         |        ^~~~~~~~~~
>   In file included from /usr/include/bits/sched.h:60,
>                    from /usr/include/sched.h:43,
>                    from /usr/include/tracefs/tracefs.h:10,
>                    from src/trace.c:4:
>   /usr/include/linux/sched/types.h:98:8: note: originally defined here
>      98 | struct sched_attr {
>         |        ^~~~~~~~~~
>
> Define 'struct sched_attr' conditionally, similar to what strace did:
>   https://lore.kernel.org/all/20240930222913.3981407-1-raj.khem@gmail.com/
> and rename rtla's version of sched_setattr() to avoid collision.
>
> Signed-off-by: Jan Stancek <jstancek@redhat.com>
> ---
>  tools/tracing/rtla/src/utils.c | 4 ++--
>  tools/tracing/rtla/src/utils.h | 2 ++
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/tools/tracing/rtla/src/utils.c b/tools/tracing/rtla/src/utils.c
> index 05b2b3fc005e..c62044215a48 100644
> --- a/tools/tracing/rtla/src/utils.c
> +++ b/tools/tracing/rtla/src/utils.c
> @@ -229,7 +229,7 @@ long parse_ns_duration(char *val)
>
>  #define SCHED_DEADLINE         6
>
> -static inline int sched_setattr(pid_t pid, const struct sched_attr *attr,
> +static inline int rtla_sched_setattr(pid_t pid, const struct sched_attr *attr,

Hmm, rtla_sched_attr sounds to me like the function does something
specific to rtla. Maybe syscall_sched_attr would be a better name?

>                                 unsigned int flags) {
>         return syscall(__NR_sched_setattr, pid, attr, flags);
>  }
> @@ -239,7 +239,7 @@ int __set_sched_attr(int pid, struct sched_attr *attr)
>         int flags = 0;
>         int retval;
>
> -       retval = sched_setattr(pid, attr, flags);
> +       retval = rtla_sched_setattr(pid, attr, flags);
>         if (retval < 0) {
>                 err_msg("Failed to set sched attributes to the pid %d: %s\n",
>                         pid, strerror(errno));
> diff --git a/tools/tracing/rtla/src/utils.h b/tools/tracing/rtla/src/utils.h
> index d44513e6c66a..99c9cf81bcd0 100644
> --- a/tools/tracing/rtla/src/utils.h
> +++ b/tools/tracing/rtla/src/utils.h
> @@ -46,6 +46,7 @@ update_sum(unsigned long long *a, unsigned long long *b)
>         *a += *b;
>  }
>
> +#ifndef SCHED_ATTR_SIZE_VER0
>  struct sched_attr {
>         uint32_t size;
>         uint32_t sched_policy;
> @@ -56,6 +57,7 @@ struct sched_attr {
>         uint64_t sched_deadline;
>         uint64_t sched_period;
>  };
> +#endif /* SCHED_ATTR_SIZE_VER0 */
>
>  int parse_prio(char *arg, struct sched_attr *sched_param);
>  int parse_cpu_set(char *cpu_list, cpu_set_t *set);
> --
> 2.43.0
>

Apart from that, LGTM.


Tomas

Re: [PATCH 2/2] tools/rtla: fix collision with glibc sched_attr/sched_set_attr

[Steven Rostedt]

On Thu, 10 Oct 2024 14:25:11 +0200
Tomas Glozar <tglozar@redhat.com> wrote:

> > --- a/tools/tracing/rtla/src/utils.c
> > +++ b/tools/tracing/rtla/src/utils.c
> > @@ -229,7 +229,7 @@ long parse_ns_duration(char *val)
> >
> >  #define SCHED_DEADLINE         6
> >
> > -static inline int sched_setattr(pid_t pid, const struct sched_attr *attr,
> > +static inline int rtla_sched_setattr(pid_t pid, const struct sched_attr *attr,  
> 
> Hmm, rtla_sched_attr sounds to me like the function does something
> specific to rtla. Maybe syscall_sched_attr would be a better name?

Should I be waiting for a v2 with this addressed?

-- Steve

Re: [PATCH 2/2] tools/rtla: fix collision with glibc sched_attr/sched_set_attr

[Jan Stancek]

On Thu, Oct 10, 2024 at 5:02 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Thu, 10 Oct 2024 14:25:11 +0200
> Tomas Glozar <tglozar@redhat.com> wrote:
>
> > > --- a/tools/tracing/rtla/src/utils.c
> > > +++ b/tools/tracing/rtla/src/utils.c
> > > @@ -229,7 +229,7 @@ long parse_ns_duration(char *val)
> > >
> > >  #define SCHED_DEADLINE         6
> > >
> > > -static inline int sched_setattr(pid_t pid, const struct sched_attr *attr,
> > > +static inline int rtla_sched_setattr(pid_t pid, const struct sched_attr *attr,
> >
> > Hmm, rtla_sched_attr sounds to me like the function does something
> > specific to rtla. Maybe syscall_sched_attr would be a better name?
>
> Should I be waiting for a v2 with this addressed?

I don't have strong opinion here, I sent v2 with name Tomas suggested.

Regards,
Jan

>
> -- Steve
>

[PATCH v2 2/2] tools/rtla: fix collision with glibc sched_attr/sched_set_attr

[Jan Stancek]

glibc commit 21571ca0d703 ("Linux: Add the sched_setattr
and sched_getattr functions") now also provides 'struct sched_attr'
and sched_setattr() which collide with the ones from rtla.

  In file included from src/trace.c:11:
  src/utils.h:49:8: error: redefinition of ‘struct sched_attr’
     49 | struct sched_attr {
        |        ^~~~~~~~~~
  In file included from /usr/include/bits/sched.h:60,
                   from /usr/include/sched.h:43,
                   from /usr/include/tracefs/tracefs.h:10,
                   from src/trace.c:4:
  /usr/include/linux/sched/types.h:98:8: note: originally defined here
     98 | struct sched_attr {
        |        ^~~~~~~~~~

Define 'struct sched_attr' conditionally, similar to what strace did:
  https://lore.kernel.org/all/20240930222913.3981407-1-raj.khem@gmail.com/
and rename rtla's version of sched_setattr() to avoid collision.

Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
 tools/tracing/rtla/src/utils.c | 4 ++--
 tools/tracing/rtla/src/utils.h | 2 ++
 2 files changed, 4 insertions(+), 2 deletions(-)

Changes in v2:
- used function name suggested by Thomas: syscall_sched_setattr()

diff --git a/tools/tracing/rtla/src/utils.c b/tools/tracing/rtla/src/utils.c
index 05b2b3fc005e..6fae234aaf36 100644
--- a/tools/tracing/rtla/src/utils.c
+++ b/tools/tracing/rtla/src/utils.c
@@ -229,7 +229,7 @@ long parse_ns_duration(char *val)
 
 #define SCHED_DEADLINE		6
 
-static inline int sched_setattr(pid_t pid, const struct sched_attr *attr,
+static inline int syscall_sched_setattr(pid_t pid, const struct sched_attr *attr,
 				unsigned int flags) {
 	return syscall(__NR_sched_setattr, pid, attr, flags);
 }
@@ -239,7 +239,7 @@ int __set_sched_attr(int pid, struct sched_attr *attr)
 	int flags = 0;
 	int retval;
 
-	retval = sched_setattr(pid, attr, flags);
+	retval = syscall_sched_setattr(pid, attr, flags);
 	if (retval < 0) {
 		err_msg("Failed to set sched attributes to the pid %d: %s\n",
 			pid, strerror(errno));
diff --git a/tools/tracing/rtla/src/utils.h b/tools/tracing/rtla/src/utils.h
index d44513e6c66a..99c9cf81bcd0 100644
--- a/tools/tracing/rtla/src/utils.h
+++ b/tools/tracing/rtla/src/utils.h
@@ -46,6 +46,7 @@ update_sum(unsigned long long *a, unsigned long long *b)
 	*a += *b;
 }
 
+#ifndef SCHED_ATTR_SIZE_VER0
 struct sched_attr {
 	uint32_t size;
 	uint32_t sched_policy;
@@ -56,6 +57,7 @@ struct sched_attr {
 	uint64_t sched_deadline;
 	uint64_t sched_period;
 };
+#endif /* SCHED_ATTR_SIZE_VER0 */
 
 int parse_prio(char *arg, struct sched_attr *sched_param);
 int parse_cpu_set(char *cpu_list, cpu_set_t *set);
-- 
2.43.0

Re: [PATCH 1/2] tools/rtla: drop __NR_sched_getattr

[Tomas Glozar]

čt 10. 10. 2024 v 11:33 odesílatel Jan Stancek <jstancek@redhat.com> napsal:
>
> It's not used since commit 084ce16df0f0 ("tools/rtla:
> Remove unused sched_getattr() function").
>
> Signed-off-by: Jan Stancek <jstancek@redhat.com>
> ---
>  tools/tracing/rtla/src/utils.c | 32 ++++++++++++++------------------
>  1 file changed, 14 insertions(+), 18 deletions(-)
>
> diff --git a/tools/tracing/rtla/src/utils.c b/tools/tracing/rtla/src/utils.c
> index 9ac71a66840c..05b2b3fc005e 100644
> --- a/tools/tracing/rtla/src/utils.c
> +++ b/tools/tracing/rtla/src/utils.c
> @@ -211,24 +211,20 @@ long parse_ns_duration(char *val)
>  /*
>   * This is a set of helper functions to use SCHED_DEADLINE.
>   */
> -#ifdef __x86_64__
> -# define __NR_sched_setattr    314
> -# define __NR_sched_getattr    315
> -#elif __i386__
> -# define __NR_sched_setattr    351
> -# define __NR_sched_getattr    352
> -#elif __arm__
> -# define __NR_sched_setattr    380
> -# define __NR_sched_getattr    381
> -#elif __aarch64__ || __riscv
> -# define __NR_sched_setattr    274
> -# define __NR_sched_getattr    275
> -#elif __powerpc__
> -# define __NR_sched_setattr    355
> -# define __NR_sched_getattr    356
> -#elif __s390x__
> -# define __NR_sched_setattr    345
> -# define __NR_sched_getattr    346
> +#ifndef __NR_sched_setattr
> +# ifdef __x86_64__
> +#  define __NR_sched_setattr   314
> +# elif __i386__
> +#  define __NR_sched_setattr   351
> +# elif __arm__
> +#  define __NR_sched_setattr   380
> +# elif __aarch64__ || __riscv
> +#  define __NR_sched_setattr   274
> +# elif __powerpc__
> +#  define __NR_sched_setattr   355
> +# elif __s390x__
> +#  define __NR_sched_setattr   345
> +# endif
>  #endif
>
>  #define SCHED_DEADLINE         6
> --
> 2.43.0
>

Right, the definition is now redundant.

Reviewed-by: Tomas Glozar <tglozar@redhat.com>

Tomas