lkml.org 
[lkml]   [2021]   [Oct]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [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?

\
 
 \ /
  Last update: 2021-10-27 10:45    [W:0.056 / U:25.188 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site