Messages in this thread Patch in this message | | | Date | Wed, 27 Oct 2021 10:43:21 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH] cpumask: Fix implicit type conversion |
| |
On Wed, Oct 27, 2021 at 06:45:23AM +0000, Jiasheng Jiang wrote: > Since the 'nr_cpu_ids' is unsigned int, then we can assume its > value is (2^32 - 1). > Also, the 'cpu' is unsigned int. > As the number of cycles increases, the value of 'cpu' can be > (2^31 - 1). > Although in the cpumask_next() 'cpu' is implicitly casted to int, > its actual value is also (2^31 - 1). > However, the return value of cpumask_next(), the updated value of > 'cpu', is (2^31). > That means the restriction 'cpu < nr_cpu_ids' is still statisfied > and in cpumask_next() when 'cpu' is implicitly casted to int, its > actual value is (-2^31). > Obviously, it is illegal and dangerous for cpumask_next(), as well
It is not illegal, police will not come for you.
> as others. > Therefore, we should fix the macro description of 'cpu' that remove > the '(optionally unsigned)' and restrict the value of 'cpu' to be > non-negative integer. > Moreover, all the existing issues should be dealed with. > > Fixes: c743f0a ("sched/fair, cpumask: Export for_each_cpu_wrap()") > Fixes: 8bd93a2 ("rcu: Accelerate grace period if last non-dynticked CPU") > Fixes: 984f2f3 ("cpumask: introduce new API, without changing anything, v3")
There is no actual bug, Fixes tag is unwarranted.
> Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn> > --- > include/linux/cpumask.h | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h > index bfc4690..8a8e59f 100644 > --- a/include/linux/cpumask.h > +++ b/include/linux/cpumask.h > @@ -232,7 +232,7 @@ int cpumask_any_distribute(const struct cpumask *srcp); > > /** > * for_each_cpu - iterate over every cpu in a mask > - * @cpu: the (optionally unsigned) integer iterator > + * @cpu: the integer iterator > * @mask: the cpumask pointer > * > * After the loop, cpu is >= nr_cpu_ids. > @@ -240,11 +240,11 @@ int cpumask_any_distribute(const struct cpumask *srcp); > #define for_each_cpu(cpu, mask) \ > for ((cpu) = -1; \ > (cpu) = cpumask_next((cpu), (mask)), \ > - (cpu) < nr_cpu_ids;) > + (cpu) < nr_cpu_ids && (cpu) >= 0;)
So now you're generating worse code for no actual gain?
| |