All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] rt-numa: optionally ignore runtime cpumask
@ 2022-01-21 14:16 Marcelo Tosatti
  2022-01-21 18:16 ` Sebastian Andrzej Siewior
  2022-01-25 18:40 ` [PATCH] rt-numa: ignore runtime cpumask if -a CPULIST is specified Marcelo Tosatti
  0 siblings, 2 replies; 16+ messages in thread
From: Marcelo Tosatti @ 2022-01-21 14:16 UTC (permalink / raw)
  To: linux-rt-users; +Cc: John Kacur


use_current_cpuset() function does:

/*
 * After this function is called, affinity_mask is the intersection of
 * the user supplied affinity mask and the affinity mask from the run
 * time environment
 */
static void use_current_cpuset(int max_cpus, struct bitmask *cpumask)

However, when using isolcpus kernel command line option, the CPUs 
specificied at isolcpus= are not part of the run time environment
cpumask.

This causes "cyclictest -a isolatedcpus" to fail with:

WARN: Couldn't setaffinity in main thread: Invalid argument
FATAL: No allowable cpus to run on
# /dev/cpu_dma_latency set to 0us

To fix this, add an environment variable IGNORE_RUNTIME_CPU_AFFINITY_MASK
that when set to a value other than 0, will override the runtime cpu
affinity mask (retrieved with numa_sched_getaffinity) with a bit set
for each CPU in numa_num_configured_cpus:

numa_num_configured_cpus() returns the number of cpus in the system.
This count includes any cpus that are currently disabled. This count is
derived from the cpu numbers in /sys/devices/system/cpu. If the kernel
is configured without /sys (CONFIG_SYSFS=n) then it falls back to using
the number of online cpus.

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>

diff --git a/src/lib/rt-numa.c b/src/lib/rt-numa.c
index ee5ab99..3106f1e 100644
--- a/src/lib/rt-numa.c
+++ b/src/lib/rt-numa.c
@@ -9,6 +9,7 @@
 #include <errno.h>
 #include <sched.h>
 #include <pthread.h>
+#include <stdlib.h>
 
 #include "rt-error.h"
 #include "rt-numa.h"
@@ -99,11 +100,20 @@ int cpu_for_thread_ua(int thread_num, int max_cpus)
 static void use_current_cpuset(int max_cpus, struct bitmask *cpumask)
 {
 	struct bitmask *curmask;
+	char *ignore_affinity_mask;
 	int i;
 
 	curmask = numa_allocate_cpumask();
 	numa_sched_getaffinity(getpid(), curmask);
 
+	ignore_affinity_mask = getenv("IGNORE_RUNTIME_CPU_AFFINITY_MASK");
+	if (ignore_affinity_mask && *ignore_affinity_mask != '0') {
+		int conf_cpus = numa_num_configured_cpus();
+
+		for (i = 0; i < conf_cpus; i++)
+			numa_bitmask_setbit(curmask, i);
+	}
+
 	/*
 	 * Clear bits that are not set in both the cpuset from the
 	 * environment, and in the user specified affinity.


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] rt-numa: optionally ignore runtime cpumask
  2022-01-21 14:16 [PATCH] rt-numa: optionally ignore runtime cpumask Marcelo Tosatti
@ 2022-01-21 18:16 ` Sebastian Andrzej Siewior
  2022-01-24 12:58   ` Marcelo Tosatti
  2022-01-25 18:40 ` [PATCH] rt-numa: ignore runtime cpumask if -a CPULIST is specified Marcelo Tosatti
  1 sibling, 1 reply; 16+ messages in thread
From: Sebastian Andrzej Siewior @ 2022-01-21 18:16 UTC (permalink / raw)
  To: Marcelo Tosatti; +Cc: linux-rt-users, John Kacur

On 2022-01-21 11:16:59 [-0300], Marcelo Tosatti wrote:
> 
> use_current_cpuset() function does:
> 
> /*
>  * After this function is called, affinity_mask is the intersection of
>  * the user supplied affinity mask and the affinity mask from the run
>  * time environment
>  */
> static void use_current_cpuset(int max_cpus, struct bitmask *cpumask)
> 
> However, when using isolcpus kernel command line option, the CPUs 
> specificied at isolcpus= are not part of the run time environment
> cpumask.
> 
> This causes "cyclictest -a isolatedcpus" to fail with:
> 
> WARN: Couldn't setaffinity in main thread: Invalid argument
> FATAL: No allowable cpus to run on
> # /dev/cpu_dma_latency set to 0us
> 
> To fix this, add an environment variable IGNORE_RUNTIME_CPU_AFFINITY_MASK
> that when set to a value other than 0, will override the runtime cpu
> affinity mask (retrieved with numa_sched_getaffinity) with a bit set
> for each CPU in numa_num_configured_cpus:

This looks hacky and not documented. What about using all CPUs which
part of current affinity mask by default. And then either specify the
requested CPU mask or use explicitly all CPUs.

Sebastian

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] rt-numa: optionally ignore runtime cpumask
  2022-01-21 18:16 ` Sebastian Andrzej Siewior
@ 2022-01-24 12:58   ` Marcelo Tosatti
  2022-01-24 16:26     ` Sebastian Andrzej Siewior
  0 siblings, 1 reply; 16+ messages in thread
From: Marcelo Tosatti @ 2022-01-24 12:58 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior; +Cc: linux-rt-users, John Kacur

On Fri, Jan 21, 2022 at 07:16:48PM +0100, Sebastian Andrzej Siewior wrote:
> On 2022-01-21 11:16:59 [-0300], Marcelo Tosatti wrote:
> > 
> > use_current_cpuset() function does:
> > 
> > /*
> >  * After this function is called, affinity_mask is the intersection of
> >  * the user supplied affinity mask and the affinity mask from the run
> >  * time environment
> >  */
> > static void use_current_cpuset(int max_cpus, struct bitmask *cpumask)
> > 
> > However, when using isolcpus kernel command line option, the CPUs 
> > specificied at isolcpus= are not part of the run time environment
> > cpumask.
> > 
> > This causes "cyclictest -a isolatedcpus" to fail with:
> > 
> > WARN: Couldn't setaffinity in main thread: Invalid argument
> > FATAL: No allowable cpus to run on
> > # /dev/cpu_dma_latency set to 0us
> > 
> > To fix this, add an environment variable IGNORE_RUNTIME_CPU_AFFINITY_MASK
> > that when set to a value other than 0, will override the runtime cpu
> > affinity mask (retrieved with numa_sched_getaffinity) with a bit set
> > for each CPU in numa_num_configured_cpus:
> 
> This looks hacky and not documented. What about using all CPUs which
> part of current affinity mask by default. 

You mean "using all CPUs which are part of the current affinity mask by
default" ? (where current affinity mask would mean user specified CPU
mask).

> And then either specify the
> requested CPU mask or use explicitly all CPUs.

Do you mean to drop 

        /*
         * Clear bits that are not set in both the cpuset from the
         * environment, and in the user specified affinity.

And just attempt to use the user specified mask? (which will then return
failure to the user in which case he can correct it).



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] rt-numa: optionally ignore runtime cpumask
  2022-01-24 12:58   ` Marcelo Tosatti
@ 2022-01-24 16:26     ` Sebastian Andrzej Siewior
  2022-01-24 16:40       ` Marcelo Tosatti
  0 siblings, 1 reply; 16+ messages in thread
From: Sebastian Andrzej Siewior @ 2022-01-24 16:26 UTC (permalink / raw)
  To: Marcelo Tosatti; +Cc: linux-rt-users, John Kacur

On 2022-01-24 09:58:31 [-0300], Marcelo Tosatti wrote:
> You mean "using all CPUs which are part of the current affinity mask by
> default" ? (where current affinity mask would mean user specified CPU
> mask).
> 
> > And then either specify the
> > requested CPU mask or use explicitly all CPUs.
> 
> Do you mean to drop 
> 
>         /*
>          * Clear bits that are not set in both the cpuset from the
>          * environment, and in the user specified affinity.
> 
> And just attempt to use the user specified mask? (which will then return
> failure to the user in which case he can correct it).
> 
After reading it again, I don't get it.
  cyclictest -a

Uses all CPUs in the system. 

  cyclictest -a $CPU

Uses the $CPU (mask) specified. If $CPU is not part of the current CPU
mask, why shouldn't it work?

Sebastian

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] rt-numa: optionally ignore runtime cpumask
  2022-01-24 16:26     ` Sebastian Andrzej Siewior
@ 2022-01-24 16:40       ` Marcelo Tosatti
  2022-01-24 16:44         ` Marcelo Tosatti
  2022-01-24 17:07         ` Sebastian Andrzej Siewior
  0 siblings, 2 replies; 16+ messages in thread
From: Marcelo Tosatti @ 2022-01-24 16:40 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior; +Cc: linux-rt-users, John Kacur

On Mon, Jan 24, 2022 at 05:26:26PM +0100, Sebastian Andrzej Siewior wrote:
> On 2022-01-24 09:58:31 [-0300], Marcelo Tosatti wrote:
> > You mean "using all CPUs which are part of the current affinity mask by
> > default" ? (where current affinity mask would mean user specified CPU
> > mask).
> > 
> > > And then either specify the
> > > requested CPU mask or use explicitly all CPUs.
> > 
> > Do you mean to drop 
> > 
> >         /*
> >          * Clear bits that are not set in both the cpuset from the
> >          * environment, and in the user specified affinity.
> > 
> > And just attempt to use the user specified mask? (which will then return
> > failure to the user in which case he can correct it).
> > 
> After reading it again, I don't get it.
>   cyclictest -a
> 
> Uses all CPUs in the system. 
> 
>   cyclictest -a $CPU
> 
> Uses the $CPU (mask) specified. If $CPU is not part of the current CPU
> mask, why shouldn't it work?

       -a, --affinity[=PROC-SET]
              Run threads on the set of processors given by PROC-SET.  If PROC-SET is not specified, all processors will be used.  Threads will be assigned to processors
              in the set in numeric order, in a round-robin fashion.
              The set of processors can be specified as A,B,C, or A-C, or A-B,D-F, and so on*.  The ! character can be used to negate a set.  For example, !B-D means to
              use all available CPUs except B through D.  The cpu numbers are the same as shown in the processor field in /proc/cpuinfo.  See numa(3) for more
              information on specifying CPU sets.  * Support for CPU sets requires libnuma version >= 2.  For libnuma v1, PROC-SET, if specified, must be a single CPU
              number.


/*
 * After this function is called, affinity_mask is the intersection of
 * the user supplied affinity mask and the affinity mask from the run
 * time environment
 */
static void use_current_cpuset(int max_cpus, struct bitmask *cpumask)
{
        struct bitmask *curmask;
        int i;

        curmask = numa_allocate_cpumask();
        numa_sched_getaffinity(getpid(), curmask);

        /*
         * Clear bits that are not set in both the cpuset from the
         * environment, and in the user specified affinity.
         */
        for (i = 0; i < max_cpus; i++) {
                if ((!numa_bitmask_isbitset(cpumask, i)) ||
                    (!numa_bitmask_isbitset(curmask, i)))
                        numa_bitmask_clearbit(cpumask, i);
        }

        numa_bitmask_free(curmask);
}

Consider 8 CPU system booted with isolcpus=3-7, and execution of 
"cyclictest -a 3-7".

sched_getaffinity() returns mask with bits set for CPUs 0 and 1.
The user supplied mask has bits 3-7 set.

The intersection between the user supplied mask and the affinity mask
from the run time environment has no bits set.



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] rt-numa: optionally ignore runtime cpumask
  2022-01-24 16:40       ` Marcelo Tosatti
@ 2022-01-24 16:44         ` Marcelo Tosatti
  2022-01-24 17:07         ` Sebastian Andrzej Siewior
  1 sibling, 0 replies; 16+ messages in thread
From: Marcelo Tosatti @ 2022-01-24 16:44 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior; +Cc: linux-rt-users, John Kacur

On Mon, Jan 24, 2022 at 01:40:49PM -0300, Marcelo Tosatti wrote:
> On Mon, Jan 24, 2022 at 05:26:26PM +0100, Sebastian Andrzej Siewior wrote:
> > On 2022-01-24 09:58:31 [-0300], Marcelo Tosatti wrote:
> > > You mean "using all CPUs which are part of the current affinity mask by
> > > default" ? (where current affinity mask would mean user specified CPU
> > > mask).
> > > 
> > > > And then either specify the
> > > > requested CPU mask or use explicitly all CPUs.
> > > 
> > > Do you mean to drop 
> > > 
> > >         /*
> > >          * Clear bits that are not set in both the cpuset from the
> > >          * environment, and in the user specified affinity.
> > > 
> > > And just attempt to use the user specified mask? (which will then return
> > > failure to the user in which case he can correct it).
> > > 
> > After reading it again, I don't get it.
> >   cyclictest -a
> > 
> > Uses all CPUs in the system. 
> > 
> >   cyclictest -a $CPU
> > 
> > Uses the $CPU (mask) specified. If $CPU is not part of the current CPU
> > mask, why shouldn't it work?
> 
>        -a, --affinity[=PROC-SET]
>               Run threads on the set of processors given by PROC-SET.  If PROC-SET is not specified, all processors will be used.  Threads will be assigned to processors
>               in the set in numeric order, in a round-robin fashion.
>               The set of processors can be specified as A,B,C, or A-C, or A-B,D-F, and so on*.  The ! character can be used to negate a set.  For example, !B-D means to
>               use all available CPUs except B through D.  The cpu numbers are the same as shown in the processor field in /proc/cpuinfo.  See numa(3) for more
>               information on specifying CPU sets.  * Support for CPU sets requires libnuma version >= 2.  For libnuma v1, PROC-SET, if specified, must be a single CPU
>               number.
> 
> 
> /*
>  * After this function is called, affinity_mask is the intersection of
>  * the user supplied affinity mask and the affinity mask from the run
>  * time environment
>  */
> static void use_current_cpuset(int max_cpus, struct bitmask *cpumask)
> {
>         struct bitmask *curmask;
>         int i;
> 
>         curmask = numa_allocate_cpumask();
>         numa_sched_getaffinity(getpid(), curmask);
> 
>         /*
>          * Clear bits that are not set in both the cpuset from the
>          * environment, and in the user specified affinity.
>          */
>         for (i = 0; i < max_cpus; i++) {
>                 if ((!numa_bitmask_isbitset(cpumask, i)) ||
>                     (!numa_bitmask_isbitset(curmask, i)))
>                         numa_bitmask_clearbit(cpumask, i);
>         }
> 
>         numa_bitmask_free(curmask);
> }
> 
> Consider 8 CPU system booted with isolcpus=3-7, and execution of 
> "cyclictest -a 3-7".
> 
> sched_getaffinity() returns mask with bits set for CPUs 0 and 1, 

and 2.

> The user supplied mask has bits 3-7 set.
> 
> The intersection between the user supplied mask and the affinity mask
> from the run time environment has no bits set.
> 
> 
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] rt-numa: optionally ignore runtime cpumask
  2022-01-24 16:40       ` Marcelo Tosatti
  2022-01-24 16:44         ` Marcelo Tosatti
@ 2022-01-24 17:07         ` Sebastian Andrzej Siewior
  2022-01-24 17:50           ` Marcelo Tosatti
  1 sibling, 1 reply; 16+ messages in thread
From: Sebastian Andrzej Siewior @ 2022-01-24 17:07 UTC (permalink / raw)
  To: Marcelo Tosatti; +Cc: linux-rt-users, John Kacur

On 2022-01-24 13:40:49 [-0300], Marcelo Tosatti wrote:
> > Uses the $CPU (mask) specified. If $CPU is not part of the current CPU
> > mask, why shouldn't it work?
> 
>        -a, --affinity[=PROC-SET]
>               Run threads on the set of processors given by PROC-SET.  If PROC-SET is not specified, all processors will be used.  Threads will be assigned to processors
>               in the set in numeric order, in a round-robin fashion.
>               The set of processors can be specified as A,B,C, or A-C, or A-B,D-F, and so on*.  The ! character can be used to negate a set.  For example, !B-D means to
>               use all available CPUs except B through D.  The cpu numbers are the same as shown in the processor field in /proc/cpuinfo.  See numa(3) for more
>               information on specifying CPU sets.  * Support for CPU sets requires libnuma version >= 2.  For libnuma v1, PROC-SET, if specified, must be a single CPU
>               number.
> 
> 
> /*
>  * After this function is called, affinity_mask is the intersection of
>  * the user supplied affinity mask and the affinity mask from the run
>  * time environment
>  */
> static void use_current_cpuset(int max_cpus, struct bitmask *cpumask)
> {
>         struct bitmask *curmask;
>         int i;
> 
>         curmask = numa_allocate_cpumask();
>         numa_sched_getaffinity(getpid(), curmask);
> 
>         /*
>          * Clear bits that are not set in both the cpuset from the
>          * environment, and in the user specified affinity.
>          */
>         for (i = 0; i < max_cpus; i++) {
>                 if ((!numa_bitmask_isbitset(cpumask, i)) ||
>                     (!numa_bitmask_isbitset(curmask, i)))
>                         numa_bitmask_clearbit(cpumask, i);
>         }
> 
>         numa_bitmask_free(curmask);
> }
> 
> Consider 8 CPU system booted with isolcpus=3-7, and execution of 
> "cyclictest -a 3-7".
> 
> sched_getaffinity() returns mask with bits set for CPUs 0 and 1.
> The user supplied mask has bits 3-7 set.
> 
> The intersection between the user supplied mask and the affinity mask
> from the run time environment has no bits set.

Okay. But does this make to keep? I understand that the current CPU-mask
needs to be kept for masks like !B or !B-D. But is there a need to use
the current CPU-mask when a specific mask has been specified by the user?

Sebastian

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] rt-numa: optionally ignore runtime cpumask
  2022-01-24 17:07         ` Sebastian Andrzej Siewior
@ 2022-01-24 17:50           ` Marcelo Tosatti
  0 siblings, 0 replies; 16+ messages in thread
From: Marcelo Tosatti @ 2022-01-24 17:50 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior; +Cc: linux-rt-users, John Kacur

On Mon, Jan 24, 2022 at 06:07:37PM +0100, Sebastian Andrzej Siewior wrote:
> On 2022-01-24 13:40:49 [-0300], Marcelo Tosatti wrote:
> > > Uses the $CPU (mask) specified. If $CPU is not part of the current CPU
> > > mask, why shouldn't it work?
> > 
> >        -a, --affinity[=PROC-SET]
> >               Run threads on the set of processors given by PROC-SET.  If PROC-SET is not specified, all processors will be used.  Threads will be assigned to processors
> >               in the set in numeric order, in a round-robin fashion.
> >               The set of processors can be specified as A,B,C, or A-C, or A-B,D-F, and so on*.  The ! character can be used to negate a set.  For example, !B-D means to
> >               use all available CPUs except B through D.  The cpu numbers are the same as shown in the processor field in /proc/cpuinfo.  See numa(3) for more
> >               information on specifying CPU sets.  * Support for CPU sets requires libnuma version >= 2.  For libnuma v1, PROC-SET, if specified, must be a single CPU
> >               number.
> > 
> > 
> > /*
> >  * After this function is called, affinity_mask is the intersection of
> >  * the user supplied affinity mask and the affinity mask from the run
> >  * time environment
> >  */
> > static void use_current_cpuset(int max_cpus, struct bitmask *cpumask)
> > {
> >         struct bitmask *curmask;
> >         int i;
> > 
> >         curmask = numa_allocate_cpumask();
> >         numa_sched_getaffinity(getpid(), curmask);
> > 
> >         /*
> >          * Clear bits that are not set in both the cpuset from the
> >          * environment, and in the user specified affinity.
> >          */
> >         for (i = 0; i < max_cpus; i++) {
> >                 if ((!numa_bitmask_isbitset(cpumask, i)) ||
> >                     (!numa_bitmask_isbitset(curmask, i)))
> >                         numa_bitmask_clearbit(cpumask, i);
> >         }
> > 
> >         numa_bitmask_free(curmask);
> > }
> > 
> > Consider 8 CPU system booted with isolcpus=3-7, and execution of 
> > "cyclictest -a 3-7".
> > 
> > sched_getaffinity() returns mask with bits set for CPUs 0 and 1.
> > The user supplied mask has bits 3-7 set.
> > 
> > The intersection between the user supplied mask and the affinity mask
> > from the run time environment has no bits set.
> 
> Okay. But does this make to keep? I understand that the current CPU-mask
> needs to be kept for masks like !B or !B-D. But is there a need to use
> the current CPU-mask when a specific mask has been specified by the user?

Ok, then:

1) If user specifies -a CPULIST, ignore sched_getaffinity().

2) If user specifies -a, or -a !CPULIST, use sched_getaffinity().

Will send a patch.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH] rt-numa: ignore runtime cpumask if -a CPULIST is specified
  2022-01-21 14:16 [PATCH] rt-numa: optionally ignore runtime cpumask Marcelo Tosatti
  2022-01-21 18:16 ` Sebastian Andrzej Siewior
@ 2022-01-25 18:40 ` Marcelo Tosatti
  2022-01-25 18:46   ` [PATCH v2] " Marcelo Tosatti
  1 sibling, 1 reply; 16+ messages in thread
From: Marcelo Tosatti @ 2022-01-25 18:40 UTC (permalink / raw)
  To: linux-rt-users; +Cc: John Kacur, Sebastian Andrzej Siewior


When using isolcpus kernel command line option, the CPUs
specificied at isolcpus= are not part of the run time environment
cpumask.

This causes "cyclictest -a isolatedcpus" to fail with:

WARN: Couldn't setaffinity in main thread: Invalid argument
FATAL: No allowable cpus to run on
# /dev/cpu_dma_latency set to 0us

To fix this, ignore the runtime cpumask if neither "+", "!"
or "all" are specified in the cpu list string.

Suggested by Sebastian Andrzej Siewior.

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>

diff --git a/src/lib/rt-numa.c b/src/lib/rt-numa.c
index ee5ab99..d887355 100644
--- a/src/lib/rt-numa.c
+++ b/src/lib/rt-numa.c
@@ -9,6 +9,7 @@
 #include <errno.h>
 #include <sched.h>
 #include <pthread.h>
+#include <stdlib.h>
 
 #include "rt-error.h"
 #include "rt-numa.h"
@@ -96,13 +97,21 @@ int cpu_for_thread_ua(int thread_num, int max_cpus)
  * the user supplied affinity mask and the affinity mask from the run
  * time environment
  */
-static void use_current_cpuset(int max_cpus, struct bitmask *cpumask)
+static void use_current_cpuset(char *str, int max_cpus, struct bitmask *cpumask)
 {
 	struct bitmask *curmask;
 	int i;
 
 	curmask = numa_allocate_cpumask();
-	numa_sched_getaffinity(getpid(), curmask);
+
+	if (strchr(str, '!') == NULL && strchr(str, '+') == NULL &&
+	    strstr(str, "all") == NULL) {
+		int conf_cpus = numa_num_configured_cpus();
+
+		for (i = 0; i < conf_cpus; i++)
+			numa_bitmask_setbit(curmask, i);
+	} else
+		numa_sched_getaffinity(getpid(), curmask);
 
 	/*
 	 * Clear bits that are not set in both the cpuset from the
@@ -131,7 +140,7 @@ int parse_cpumask(char *str, int max_cpus, struct bitmask **cpumask)
 		return 0;
 	}
 
-	use_current_cpuset(max_cpus, mask);
+	use_current_cpuset(str, max_cpus, mask);
 	*cpumask = mask;
 
 	return 0;


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2] rt-numa: ignore runtime cpumask if -a CPULIST is specified
  2022-01-25 18:40 ` [PATCH] rt-numa: ignore runtime cpumask if -a CPULIST is specified Marcelo Tosatti
@ 2022-01-25 18:46   ` Marcelo Tosatti
  2022-01-26  7:21     ` Sebastian Andrzej Siewior
  2022-01-28 17:44     ` John Kacur
  0 siblings, 2 replies; 16+ messages in thread
From: Marcelo Tosatti @ 2022-01-25 18:46 UTC (permalink / raw)
  To: linux-rt-users; +Cc: John Kacur, Sebastian Andrzej Siewior

 
When using isolcpus kernel command line option, the CPUs
specified at isolcpus= are not part of the run time environment
cpumask.

This causes "cyclictest -a isolatedcpus" to fail with:

WARN: Couldn't setaffinity in main thread: Invalid argument
FATAL: No allowable cpus to run on
# /dev/cpu_dma_latency set to 0us

To fix this, ignore the runtime cpumask if neither "+", "!"
or "all" are specified in the cpu list string.

Suggested by Sebastian Andrzej Siewior.

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>

---

v2: fix changelog typo

diff --git a/src/lib/rt-numa.c b/src/lib/rt-numa.c
index ee5ab99..d887355 100644
--- a/src/lib/rt-numa.c
+++ b/src/lib/rt-numa.c
@@ -9,6 +9,7 @@
 #include <errno.h>
 #include <sched.h>
 #include <pthread.h>
+#include <stdlib.h>
 
 #include "rt-error.h"
 #include "rt-numa.h"
@@ -96,13 +97,21 @@ int cpu_for_thread_ua(int thread_num, int max_cpus)
  * the user supplied affinity mask and the affinity mask from the run
  * time environment
  */
-static void use_current_cpuset(int max_cpus, struct bitmask *cpumask)
+static void use_current_cpuset(char *str, int max_cpus, struct bitmask *cpumask)
 {
 	struct bitmask *curmask;
 	int i;
 
 	curmask = numa_allocate_cpumask();
-	numa_sched_getaffinity(getpid(), curmask);
+
+	if (strchr(str, '!') == NULL && strchr(str, '+') == NULL &&
+	    strstr(str, "all") == NULL) {
+		int conf_cpus = numa_num_configured_cpus();
+
+		for (i = 0; i < conf_cpus; i++)
+			numa_bitmask_setbit(curmask, i);
+	} else
+		numa_sched_getaffinity(getpid(), curmask);
 
 	/*
 	 * Clear bits that are not set in both the cpuset from the
@@ -131,7 +140,7 @@ int parse_cpumask(char *str, int max_cpus, struct bitmask **cpumask)
 		return 0;
 	}
 
-	use_current_cpuset(max_cpus, mask);
+	use_current_cpuset(str, max_cpus, mask);
 	*cpumask = mask;
 
 	return 0;


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v2] rt-numa: ignore runtime cpumask if -a CPULIST is specified
  2022-01-25 18:46   ` [PATCH v2] " Marcelo Tosatti
@ 2022-01-26  7:21     ` Sebastian Andrzej Siewior
  2022-01-28 17:44     ` John Kacur
  1 sibling, 0 replies; 16+ messages in thread
From: Sebastian Andrzej Siewior @ 2022-01-26  7:21 UTC (permalink / raw)
  To: Marcelo Tosatti; +Cc: linux-rt-users, John Kacur

On 2022-01-25 15:46:00 [-0300], Marcelo Tosatti wrote:
>  
> When using isolcpus kernel command line option, the CPUs
> specified at isolcpus= are not part of the run time environment
> cpumask.
> 
> This causes "cyclictest -a isolatedcpus" to fail with:
> 
> WARN: Couldn't setaffinity in main thread: Invalid argument
> FATAL: No allowable cpus to run on
> # /dev/cpu_dma_latency set to 0us
> 
> To fix this, ignore the runtime cpumask if neither "+", "!"
> or "all" are specified in the cpu list string.
> 
> Suggested by Sebastian Andrzej Siewior.
> 
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>

Yes, thanks. Looks nice.

Acked-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

Sebastian

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2] rt-numa: ignore runtime cpumask if -a CPULIST is specified
  2022-01-25 18:46   ` [PATCH v2] " Marcelo Tosatti
  2022-01-26  7:21     ` Sebastian Andrzej Siewior
@ 2022-01-28 17:44     ` John Kacur
  2022-01-28 18:17       ` Sebastian Andrzej Siewior
  2022-01-28 18:39       ` [PATCH v3] " Marcelo Tosatti
  1 sibling, 2 replies; 16+ messages in thread
From: John Kacur @ 2022-01-28 17:44 UTC (permalink / raw)
  To: Marcelo Tosatti; +Cc: linux-rt-users, Sebastian Andrzej Siewior



On Tue, 25 Jan 2022, Marcelo Tosatti wrote:

>  
> When using isolcpus kernel command line option, the CPUs
> specified at isolcpus= are not part of the run time environment
> cpumask.
> 
> This causes "cyclictest -a isolatedcpus" to fail with:
> 
> WARN: Couldn't setaffinity in main thread: Invalid argument
> FATAL: No allowable cpus to run on
> # /dev/cpu_dma_latency set to 0us
> 
> To fix this, ignore the runtime cpumask if neither "+", "!"
> or "all" are specified in the cpu list string.
> 
> Suggested by Sebastian Andrzej Siewior.
> 
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> 
> ---
> 
> v2: fix changelog typo
> 
> diff --git a/src/lib/rt-numa.c b/src/lib/rt-numa.c
> index ee5ab99..d887355 100644
> --- a/src/lib/rt-numa.c
> +++ b/src/lib/rt-numa.c
> @@ -9,6 +9,7 @@
>  #include <errno.h>
>  #include <sched.h>
>  #include <pthread.h>
> +#include <stdlib.h>
>  
>  #include "rt-error.h"
>  #include "rt-numa.h"
> @@ -96,13 +97,21 @@ int cpu_for_thread_ua(int thread_num, int max_cpus)
>   * the user supplied affinity mask and the affinity mask from the run
>   * time environment
>   */
> -static void use_current_cpuset(int max_cpus, struct bitmask *cpumask)
> +static void use_current_cpuset(char *str, int max_cpus, struct bitmask *cpumask)
>  {
>  	struct bitmask *curmask;
>  	int i;
>  
>  	curmask = numa_allocate_cpumask();
> -	numa_sched_getaffinity(getpid(), curmask);
> +
> +	if (strchr(str, '!') == NULL && strchr(str, '+') == NULL &&
> +	    strstr(str, "all") == NULL) {
> +		int conf_cpus = numa_num_configured_cpus();
> +
> +		for (i = 0; i < conf_cpus; i++)
> +			numa_bitmask_setbit(curmask, i);
> +	} else
> +		numa_sched_getaffinity(getpid(), curmask);
>  
>  	/*
>  	 * Clear bits that are not set in both the cpuset from the
> @@ -131,7 +140,7 @@ int parse_cpumask(char *str, int max_cpus, struct bitmask **cpumask)
>  		return 0;
>  	}
>  
> -	use_current_cpuset(max_cpus, mask);
> +	use_current_cpuset(str, max_cpus, mask);
>  	*cpumask = mask;
>  
>  	return 0;
> 
> 

I agree that our current method is too restrictive, so I like the basic 
idea here.

I think we also need to differentiate between -a with no arguments
and -a all. -a with no arguments should inherit the cpumask from the 
runtime environment but -a all, should be the same as specifying all the 
cpus and ignoring the inherited cpumask.

The work of the function parse_cpumask() is essentially in the call to 
numa_parse_cpustring_all() and then pointing the affinity_mask to the 
result. So your patch is unnecessarily complicated. Just don't call 
use_current_cpuset(). All that is needed is to change parse_cpumask()
with

-       use_current_cpuset(max_cpus, mask);
+       if (strchr(str, '!') != NULL || strchr(str, '+') != NULL)
+               use_current_cpuset(max_cpus, mask);

So, if you want to send me the above patch, I'll apply it.

While testing, I noticed that there is a bug in calling parse_cpumask
such that -aall works but -a all (with a space) was not working correctly.
I will send a patch for that.

Thanks

John


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2] rt-numa: ignore runtime cpumask if -a CPULIST is specified
  2022-01-28 17:44     ` John Kacur
@ 2022-01-28 18:17       ` Sebastian Andrzej Siewior
  2022-01-28 21:08         ` John Kacur
  2022-01-28 18:39       ` [PATCH v3] " Marcelo Tosatti
  1 sibling, 1 reply; 16+ messages in thread
From: Sebastian Andrzej Siewior @ 2022-01-28 18:17 UTC (permalink / raw)
  To: John Kacur; +Cc: Marcelo Tosatti, linux-rt-users

On 2022-01-28 12:44:14 [-0500], John Kacur wrote:
> I agree that our current method is too restrictive, so I like the basic 
> idea here.
> 
> I think we also need to differentiate between -a with no arguments
> and -a all. -a with no arguments should inherit the cpumask from the 
> runtime environment but -a all, should be the same as specifying all the 
> cpus and ignoring the inherited cpumask.

do you consider number of online CPUs to not deploy more threads than
CPUs online in case some were shutdown?

> Thanks
> 
> John

Sebastian

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v3] rt-numa: ignore runtime cpumask if -a CPULIST is specified
  2022-01-28 17:44     ` John Kacur
  2022-01-28 18:17       ` Sebastian Andrzej Siewior
@ 2022-01-28 18:39       ` Marcelo Tosatti
  2022-01-28 21:11         ` John Kacur
  1 sibling, 1 reply; 16+ messages in thread
From: Marcelo Tosatti @ 2022-01-28 18:39 UTC (permalink / raw)
  To: John Kacur; +Cc: linux-rt-users, Sebastian Andrzej Siewior


When using isolcpus kernel command line option, the CPUs
specified at isolcpus= are not part of the run time environment
cpumask.

This causes "cyclictest -a isolatedcpus" to fail with:

WARN: Couldn't setaffinity in main thread: Invalid argument
FATAL: No allowable cpus to run on
# /dev/cpu_dma_latency set to 0us

To fix this, ignore the runtime cpumask if neither "+", "!"
are specified in the cpu list string.

Suggested by Sebastian Andrzej Siewior.

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>

v3: simplified version (John Kacur)
v2: fix changelog typo

diff --git a/src/lib/rt-numa.c b/src/lib/rt-numa.c
index ee5ab99..3eead80 100644
--- a/src/lib/rt-numa.c
+++ b/src/lib/rt-numa.c
@@ -131,7 +131,8 @@ int parse_cpumask(char *str, int max_cpus, struct bitmask **cpumask)
 		return 0;
 	}
 
-	use_current_cpuset(max_cpus, mask);
+	if (strchr(str, '!') != NULL || strchr(str, '+') != NULL)
+		use_current_cpuset(max_cpus, mask);
 	*cpumask = mask;
 
 	return 0;


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v2] rt-numa: ignore runtime cpumask if -a CPULIST is specified
  2022-01-28 18:17       ` Sebastian Andrzej Siewior
@ 2022-01-28 21:08         ` John Kacur
  0 siblings, 0 replies; 16+ messages in thread
From: John Kacur @ 2022-01-28 21:08 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior; +Cc: Marcelo Tosatti, linux-rt-users



On Fri, 28 Jan 2022, Sebastian Andrzej Siewior wrote:

> On 2022-01-28 12:44:14 [-0500], John Kacur wrote:
> > I agree that our current method is too restrictive, so I like the basic 
> > idea here.
> > 
> > I think we also need to differentiate between -a with no arguments
> > and -a all. -a with no arguments should inherit the cpumask from the 
> > runtime environment but -a all, should be the same as specifying all the 
> > cpus and ignoring the inherited cpumask.
> 
> do you consider number of online CPUs to not deploy more threads than
> CPUs online in case some were shutdown?

Not sure I understand your question, but what I am saying is that we can 
inherit a cpumask that has less cpus than cpus that are online. So in the 
case of -a, just use the cpus available in the inherited cpumask
in the case of -a all, use all available cpus.

> 
> > Thanks
> > 
> > John
> 
> Sebastian
> 
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3] rt-numa: ignore runtime cpumask if -a CPULIST is specified
  2022-01-28 18:39       ` [PATCH v3] " Marcelo Tosatti
@ 2022-01-28 21:11         ` John Kacur
  0 siblings, 0 replies; 16+ messages in thread
From: John Kacur @ 2022-01-28 21:11 UTC (permalink / raw)
  To: Marcelo Tosatti; +Cc: linux-rt-users, Sebastian Andrzej Siewior



On Fri, 28 Jan 2022, Marcelo Tosatti wrote:

> 
> When using isolcpus kernel command line option, the CPUs
> specified at isolcpus= are not part of the run time environment
> cpumask.
> 
> This causes "cyclictest -a isolatedcpus" to fail with:
> 
> WARN: Couldn't setaffinity in main thread: Invalid argument
> FATAL: No allowable cpus to run on
> # /dev/cpu_dma_latency set to 0us
> 
> To fix this, ignore the runtime cpumask if neither "+", "!"
> are specified in the cpu list string.
> 
> Suggested by Sebastian Andrzej Siewior.
> 
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> 
> v3: simplified version (John Kacur)
> v2: fix changelog typo
> 
> diff --git a/src/lib/rt-numa.c b/src/lib/rt-numa.c
> index ee5ab99..3eead80 100644
> --- a/src/lib/rt-numa.c
> +++ b/src/lib/rt-numa.c
> @@ -131,7 +131,8 @@ int parse_cpumask(char *str, int max_cpus, struct bitmask **cpumask)
>  		return 0;
>  	}
>  
> -	use_current_cpuset(max_cpus, mask);
> +	if (strchr(str, '!') != NULL || strchr(str, '+') != NULL)
> +		use_current_cpuset(max_cpus, mask);
>  	*cpumask = mask;
>  
>  	return 0;
> 
> 

Signed-off-by: John Kacur <jkacur@redhat.com>


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2022-01-28 21:11 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-21 14:16 [PATCH] rt-numa: optionally ignore runtime cpumask Marcelo Tosatti
2022-01-21 18:16 ` Sebastian Andrzej Siewior
2022-01-24 12:58   ` Marcelo Tosatti
2022-01-24 16:26     ` Sebastian Andrzej Siewior
2022-01-24 16:40       ` Marcelo Tosatti
2022-01-24 16:44         ` Marcelo Tosatti
2022-01-24 17:07         ` Sebastian Andrzej Siewior
2022-01-24 17:50           ` Marcelo Tosatti
2022-01-25 18:40 ` [PATCH] rt-numa: ignore runtime cpumask if -a CPULIST is specified Marcelo Tosatti
2022-01-25 18:46   ` [PATCH v2] " Marcelo Tosatti
2022-01-26  7:21     ` Sebastian Andrzej Siewior
2022-01-28 17:44     ` John Kacur
2022-01-28 18:17       ` Sebastian Andrzej Siewior
2022-01-28 21:08         ` John Kacur
2022-01-28 18:39       ` [PATCH v3] " Marcelo Tosatti
2022-01-28 21:11         ` John Kacur

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.