Re: [PATCH 06/10] lib/cpumask: move trivial wrappers around find_bit to the header

[Sander Vanheule <sander@svanheule.net>]

Hi Yury,

On Wed, 2022-07-06 at 10:42 -0700, Yury Norov wrote:
> To avoid circular dependencies, cpumask keeps simple (almost) one-line
> wrappers around find_bit() in a c-file.
> 
> Commit 47d8c15615c0a2 ("include: move find.h from asm_generic to linux")
> moved find.h header out of asm_generic include path, and it helped to fix
> many circular dependencies, including some in cpumask.h.
> 
> This patch moves those one-liners to header files.
> 
> Signed-off-by: Yury Norov <yury.norov@gmail.com>
> ---
>  include/linux/cpumask.h | 57 ++++++++++++++++++++++++++++++++++++++---
>  lib/cpumask.c           | 55 ---------------------------------------
>  2 files changed, 54 insertions(+), 58 deletions(-)
> 
> diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
> index 760022bcb925..ea3de2c2c180 100644
> --- a/include/linux/cpumask.h
> +++ b/include/linux/cpumask.h
> @@ -241,7 +241,21 @@ static inline unsigned int cpumask_last(const struct
> cpumask *srcp)
>         return find_last_bit(cpumask_bits(srcp), nr_cpumask_bits);
>  }
>  
> -unsigned int __pure cpumask_next(int n, const struct cpumask *srcp);
> +/**
> + * cpumask_next - get the next cpu in a cpumask
> + * @n: the cpu prior to the place to search (ie. return will be > @n)
> + * @srcp: the cpumask pointer
> + *
> + * Returns >= nr_cpu_ids if no further cpus set.
> + */
> +static inline
> +unsigned int cpumask_next(int n, const struct cpumask *srcp)

This also drops the __pure speficier for these functions. Since I have a patch
that does the opposite for cpumask_next_wrap() [1], I was wondering what your
reasoning behind this is.

Since a cpumask like cpu_online_mask may change between subsequent calls, I'm
considering to drop my patch adding __pure, and to follow the changes you've
made here.

[1]
https://lore.kernel.org/all/06eebdc46cfb21eb437755a2a5a56d55c41400f5.1659077534.git.sander@svanheule.net/

Best,
Sander

> +{
> +       /* -1 is a legal arg here. */
> +       if (n != -1)
> +               cpumask_check(n);
> +       return find_next_bit(cpumask_bits(srcp), nr_cpumask_bits, n + 1);
> +}
>  
>  /**
>   * cpumask_next_zero - get the next unset cpu in a cpumask
> @@ -258,8 +272,25 @@ static inline unsigned int cpumask_next_zero(int n, const
> struct cpumask *srcp)
>         return find_next_zero_bit(cpumask_bits(srcp), nr_cpumask_bits, n+1);
>  }
>  
> -unsigned int __pure cpumask_next_and(int n, const struct cpumask *, const
> struct cpumask *);
> -unsigned int __pure cpumask_any_but(const struct cpumask *mask, unsigned int
> cpu);
> +/**
> + * cpumask_next_and - get the next cpu in *src1p & *src2p
> + * @n: the cpu prior to the place to search (ie. return will be > @n)
> + * @src1p: the first cpumask pointer
> + * @src2p: the second cpumask pointer
> + *
> + * Returns >= nr_cpu_ids if no further cpus set in both.
> + */
> +static inline
> +unsigned int cpumask_next_and(int n, const struct cpumask *src1p,
> +                    const struct cpumask *src2p)
> +{
> +       /* -1 is a legal arg here. */
> +       if (n != -1)
> +               cpumask_check(n);
> +       return find_next_and_bit(cpumask_bits(src1p), cpumask_bits(src2p),
> +               nr_cpumask_bits, n + 1);
> +}
> +
>  unsigned int cpumask_local_spread(unsigned int i, int node);
>  unsigned int cpumask_any_and_distribute(const struct cpumask *src1p,
>                                const struct cpumask *src2p);
> @@ -324,6 +355,26 @@ unsigned int cpumask_next_wrap(int n, const struct
> cpumask *mask, int start, boo
>         for ((cpu) = -1;                                                \
>                 (cpu) = cpumask_next_and((cpu), (mask1), (mask2)),      \
>                 (cpu) < nr_cpu_ids;)
> +
> +/**
> + * cpumask_any_but - return a "random" in a cpumask, but not this one.
> + * @mask: the cpumask to search
> + * @cpu: the cpu to ignore.
> + *
> + * Often used to find any cpu but smp_processor_id() in a mask.
> + * Returns >= nr_cpu_ids if no cpus set.
> + */
> +static inline
> +unsigned int cpumask_any_but(const struct cpumask *mask, unsigned int cpu)
> +{
> +       unsigned int i;
> +
> +       cpumask_check(cpu);
> +       for_each_cpu(i, mask)
> +               if (i != cpu)
> +                       break;
> +       return i;
> +}
>  #endif /* SMP */
>  
>  #define CPU_BITS_NONE                                          \
> diff --git a/lib/cpumask.c b/lib/cpumask.c
> index da68f6bbde44..cb7262ff8633 100644
> --- a/lib/cpumask.c
> +++ b/lib/cpumask.c
> @@ -7,61 +7,6 @@
>  #include <linux/memblock.h>
>  #include <linux/numa.h>
>  
> -/**
> - * cpumask_next - get the next cpu in a cpumask
> - * @n: the cpu prior to the place to search (ie. return will be > @n)
> - * @srcp: the cpumask pointer
> - *
> - * Returns >= nr_cpu_ids if no further cpus set.
> - */
> -unsigned int cpumask_next(int n, const struct cpumask *srcp)
> -{
> -       /* -1 is a legal arg here. */
> -       if (n != -1)
> -               cpumask_check(n);
> -       return find_next_bit(cpumask_bits(srcp), nr_cpumask_bits, n + 1);
> -}
> -EXPORT_SYMBOL(cpumask_next);
> -
> -/**
> - * cpumask_next_and - get the next cpu in *src1p & *src2p
> - * @n: the cpu prior to the place to search (ie. return will be > @n)
> - * @src1p: the first cpumask pointer
> - * @src2p: the second cpumask pointer
> - *
> - * Returns >= nr_cpu_ids if no further cpus set in both.
> - */
> -unsigned int cpumask_next_and(int n, const struct cpumask *src1p,
> -                    const struct cpumask *src2p)
> -{
> -       /* -1 is a legal arg here. */
> -       if (n != -1)
> -               cpumask_check(n);
> -       return find_next_and_bit(cpumask_bits(src1p), cpumask_bits(src2p),
> -               nr_cpumask_bits, n + 1);
> -}
> -EXPORT_SYMBOL(cpumask_next_and);
> -
> -/**
> - * cpumask_any_but - return a "random" in a cpumask, but not this one.
> - * @mask: the cpumask to search
> - * @cpu: the cpu to ignore.
> - *
> - * Often used to find any cpu but smp_processor_id() in a mask.
> - * Returns >= nr_cpu_ids if no cpus set.
> - */
> -unsigned int cpumask_any_but(const struct cpumask *mask, unsigned int cpu)
> -{
> -       unsigned int i;
> -
> -       cpumask_check(cpu);
> -       for_each_cpu(i, mask)
> -               if (i != cpu)
> -                       break;
> -       return i;
> -}
> -EXPORT_SYMBOL(cpumask_any_but);
> -
>  /**
>   * cpumask_next_wrap - helper to implement for_each_cpu_wrap
>   * @n: the cpu prior to the place to search


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel