Re: [PATCH] cobalt/sched: Use nr_cpumask_bits instead of BITS_PER_LONG
On 14.03.22 22:13, Bezdeka, Florian via Xenomai wrote:
> On Mon, 2022-03-14 at 21:05 +, 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
>>> ---
>>> 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, _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();
>>> - 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
Re: [PATCH] cobalt/sched: Use nr_cpumask_bits instead of BITS_PER_LONG
- Ursprüngliche Mail - > Von: "Florian Bezdeka" >> 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
On Mon, 2022-03-14 at 22:10 +0100, Richard Weinberger wrote: > - Ursprüngliche Mail - > > Von: "Florian Bezdeka" > > 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
On Mon, 2022-03-14 at 21:05 +, 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
> > ---
> > 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, _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();
> > - 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
- Ursprüngliche Mail - > Von: "Florian Bezdeka" > 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
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
> ---
> 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, _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();
> - 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
