[PATCH] cobalt/sched: Use nr_cpumask_bits instead of BITS_PER_LONG

[PATCH] cobalt/sched: Use nr_cpumask_bits instead of BITS_PER_LONG

[Richard Weinberger]

BITS_PER_LONG is too broad, the max number of usable bits is limited
by nr_cpumask_bits.
Found while debugging a system with CONFIG_DEBUG_PER_CPU_MAPS enabled.

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 kernel/cobalt/sched.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/cobalt/sched.c b/kernel/cobalt/sched.c
index 88c4951ed814..aa65fd7f5d63 100644
--- a/kernel/cobalt/sched.c
+++ b/kernel/cobalt/sched.c
@@ -1370,7 +1370,7 @@ static int affinity_vfile_show(struct xnvfile_regular_iterator *it,
 	unsigned long val = 0;
 	int cpu;
 
-	for (cpu = 0; cpu < BITS_PER_LONG; cpu++)
+	for (cpu = 0; cpu < nr_cpumask_bits; cpu++)
 		if (cpumask_test_cpu(cpu, &cobalt_cpu_affinity))
 			val |= (1UL << cpu);
 
@@ -1395,7 +1395,7 @@ static ssize_t affinity_vfile_store(struct xnvfile_input *input)
 		affinity = xnsched_realtime_cpus; /* Reset to default. */
 	else {
 		cpumask_clear(&affinity);
-		for (cpu = 0; cpu < BITS_PER_LONG; cpu++, val >>= 1) {
+		for (cpu = 0; cpu < nr_cpumask_bits; cpu++, val >>= 1) {
 			if (val & 1) {
 				/*
 				 * The new dynamic affinity must be a strict
-- 
2.26.2

Re: [PATCH] cobalt/sched: Use nr_cpumask_bits instead of BITS_PER_LONG

[Bezdeka, Florian]

Hi Richard,

On Mon, 2022-03-14 at 21:38 +0100, Richard Weinberger via Xenomai
wrote:
> BITS_PER_LONG is too broad, the max number of usable bits is limited
> by nr_cpumask_bits.

I agree, BITS_PER_LONG seems wrong. But couldn't it be too small as
well? It depends on NR_CPUS which might be > BITS_PER_LONG.

Regards,
Florian

> Found while debugging a system with CONFIG_DEBUG_PER_CPU_MAPS enabled.
> 
> Signed-off-by: Richard Weinberger <richard@nod.at>
> ---
>  kernel/cobalt/sched.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/cobalt/sched.c b/kernel/cobalt/sched.c
> index 88c4951ed814..aa65fd7f5d63 100644
> --- a/kernel/cobalt/sched.c
> +++ b/kernel/cobalt/sched.c
> @@ -1370,7 +1370,7 @@ static int affinity_vfile_show(struct xnvfile_regular_iterator *it,
>  	unsigned long val = 0;
>  	int cpu;
>  
> -	for (cpu = 0; cpu < BITS_PER_LONG; cpu++)
> +	for (cpu = 0; cpu < nr_cpumask_bits; cpu++)
>  		if (cpumask_test_cpu(cpu, &cobalt_cpu_affinity))
>  			val |= (1UL << cpu);
>  
> @@ -1395,7 +1395,7 @@ static ssize_t affinity_vfile_store(struct xnvfile_input *input)
>  		affinity = xnsched_realtime_cpus; /* Reset to default. */
>  	else {
>  		cpumask_clear(&affinity);
> -		for (cpu = 0; cpu < BITS_PER_LONG; cpu++, val >>= 1) {
> +		for (cpu = 0; cpu < nr_cpumask_bits; cpu++, val >>= 1) {
>  			if (val & 1) {
>  				/*
>  				 * The new dynamic affinity must be a strict

Re: [PATCH] cobalt/sched: Use nr_cpumask_bits instead of BITS_PER_LONG

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "Florian Bezdeka" <florian.bezdeka@siemens.com>
> I agree, BITS_PER_LONG seems wrong. But couldn't it be too small as
> well? It depends on NR_CPUS which might be > BITS_PER_LONG.

Hmm, nr_cpumask_bits is defined as:

#ifdef CONFIG_CPUMASK_OFFSTACK
/* Assuming NR_CPUS is huge, a runtime limit is more efficient.  Also,
 * not all bits may be allocated. */
#define nr_cpumask_bits nr_cpu_ids
#else
#define nr_cpumask_bits ((unsigned int)NR_CPUS)
#endif

Where do you see a problem?

Thanks,
//richard

Re: [PATCH] cobalt/sched: Use nr_cpumask_bits instead of BITS_PER_LONG

[Bezdeka, Florian]

On Mon, 2022-03-14 at 21:05 +0000, Bezdeka, Florian via Xenomai wrote:
> Hi Richard,
> 
> On Mon, 2022-03-14 at 21:38 +0100, Richard Weinberger via Xenomai
> wrote:
> > BITS_PER_LONG is too broad, the max number of usable bits is limited
> > by nr_cpumask_bits.
> 
> I agree, BITS_PER_LONG seems wrong. But couldn't it be too small as
> well? It depends on NR_CPUS which might be > BITS_PER_LONG.

Sorry that was unclear. I assume that the size of the cpumask depends
somehow on NR_CPUS, which might be > BITS_PER_LONG. So BITS_PER_LONG
might be too small AND might be too broad.

> 
> Regards,
> Florian
> 
> > Found while debugging a system with CONFIG_DEBUG_PER_CPU_MAPS enabled.
> > 
> > Signed-off-by: Richard Weinberger <richard@nod.at>
> > ---
> >  kernel/cobalt/sched.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/kernel/cobalt/sched.c b/kernel/cobalt/sched.c
> > index 88c4951ed814..aa65fd7f5d63 100644
> > --- a/kernel/cobalt/sched.c
> > +++ b/kernel/cobalt/sched.c
> > @@ -1370,7 +1370,7 @@ static int affinity_vfile_show(struct xnvfile_regular_iterator *it,
> >  	unsigned long val = 0;
> >  	int cpu;
> >  
> > -	for (cpu = 0; cpu < BITS_PER_LONG; cpu++)
> > +	for (cpu = 0; cpu < nr_cpumask_bits; cpu++)
> >  		if (cpumask_test_cpu(cpu, &cobalt_cpu_affinity))
> >  			val |= (1UL << cpu);
> >  
> > @@ -1395,7 +1395,7 @@ static ssize_t affinity_vfile_store(struct xnvfile_input *input)
> >  		affinity = xnsched_realtime_cpus; /* Reset to default. */
> >  	else {
> >  		cpumask_clear(&affinity);
> > -		for (cpu = 0; cpu < BITS_PER_LONG; cpu++, val >>= 1) {
> > +		for (cpu = 0; cpu < nr_cpumask_bits; cpu++, val >>= 1) {
> >  			if (val & 1) {
> >  				/*
> >  				 * The new dynamic affinity must be a strict
> 

Re: [PATCH] cobalt/sched: Use nr_cpumask_bits instead of BITS_PER_LONG

[Bezdeka, Florian]

On Mon, 2022-03-14 at 22:10 +0100, Richard Weinberger wrote:
> ----- Ursprüngliche Mail -----
> > Von: "Florian Bezdeka" <florian.bezdeka@siemens.com>
> > I agree, BITS_PER_LONG seems wrong. But couldn't it be too small as
> > well? It depends on NR_CPUS which might be > BITS_PER_LONG.
> 
> Hmm, nr_cpumask_bits is defined as:
> 
> #ifdef CONFIG_CPUMASK_OFFSTACK
> /* Assuming NR_CPUS is huge, a runtime limit is more efficient.  Also,
>  * not all bits may be allocated. */
> #define nr_cpumask_bits nr_cpu_ids
> #else
> #define nr_cpumask_bits ((unsigned int)NR_CPUS)
> #endif
> 
> Where do you see a problem?

I'm fine with the implementation. Just struggled with the wording /
commit message. Sorry for the race in the mail thread...

> 
> Thanks,
> //richard

Re: [PATCH] cobalt/sched: Use nr_cpumask_bits instead of BITS_PER_LONG

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "Florian Bezdeka" <florian.bezdeka@siemens.com>
>> Where do you see a problem?
> 
> I'm fine with the implementation. Just struggled with the wording /
> commit message. Sorry for the race in the mail thread...

No need to worry. :-)

Thanks,
//richard

Re: [PATCH] cobalt/sched: Use nr_cpumask_bits instead of BITS_PER_LONG

[Jan Kiszka]

On 14.03.22 22:13, Bezdeka, Florian via Xenomai wrote:
> On Mon, 2022-03-14 at 21:05 +0000, Bezdeka, Florian via Xenomai wrote:
>> Hi Richard,
>>
>> On Mon, 2022-03-14 at 21:38 +0100, Richard Weinberger via Xenomai
>> wrote:
>>> BITS_PER_LONG is too broad, the max number of usable bits is limited
>>> by nr_cpumask_bits.
>>
>> I agree, BITS_PER_LONG seems wrong. But couldn't it be too small as
>> well? It depends on NR_CPUS which might be > BITS_PER_LONG.
> 
> Sorry that was unclear. I assume that the size of the cpumask depends
> somehow on NR_CPUS, which might be > BITS_PER_LONG. So BITS_PER_LONG
> might be too small AND might be too broad.
> 

Good hint, I've adjusted this on merge.

Thanks,
Jan

>>
>> Regards,
>> Florian
>>
>>> Found while debugging a system with CONFIG_DEBUG_PER_CPU_MAPS enabled.
>>>
>>> Signed-off-by: Richard Weinberger <richard@nod.at>
>>> ---
>>>  kernel/cobalt/sched.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/kernel/cobalt/sched.c b/kernel/cobalt/sched.c
>>> index 88c4951ed814..aa65fd7f5d63 100644
>>> --- a/kernel/cobalt/sched.c
>>> +++ b/kernel/cobalt/sched.c
>>> @@ -1370,7 +1370,7 @@ static int affinity_vfile_show(struct xnvfile_regular_iterator *it,
>>>  	unsigned long val = 0;
>>>  	int cpu;
>>>  
>>> -	for (cpu = 0; cpu < BITS_PER_LONG; cpu++)
>>> +	for (cpu = 0; cpu < nr_cpumask_bits; cpu++)
>>>  		if (cpumask_test_cpu(cpu, &cobalt_cpu_affinity))
>>>  			val |= (1UL << cpu);
>>>  
>>> @@ -1395,7 +1395,7 @@ static ssize_t affinity_vfile_store(struct xnvfile_input *input)
>>>  		affinity = xnsched_realtime_cpus; /* Reset to default. */
>>>  	else {
>>>  		cpumask_clear(&affinity);
>>> -		for (cpu = 0; cpu < BITS_PER_LONG; cpu++, val >>= 1) {
>>> +		for (cpu = 0; cpu < nr_cpumask_bits; cpu++, val >>= 1) {
>>>  			if (val & 1) {
>>>  				/*
>>>  				 * The new dynamic affinity must be a strict
>>
> 

-- 
Siemens AG, Technology
Competence Center Embedded Linux