| Message ID | 20220706174253.4175492-7-yury.norov@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | lib: cleanup bitmap-related headers | expand |
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
On Sun, Jul 31, 2022 at 11:42:52AM +0200, Sander Vanheule wrote: > 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/ __pure is a promise to the compiler that the function will not modify system state (i.e. will not write into the memory). Now that the cpumask_next etc. became static inline, there's no reason for the hint because the compiler inlines the code, and there's no a real function. Maybe then it's worth to propagate the __pure to find_bit() helpers... Would be great to get comments form compiler people. Rasmus? Thanks, Yury
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) +{ + /* -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
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(-)