for_each_cpu_and() is very convenient as it saves having to allocate a
temporary cpumask to store the result of cpumask_and(). The same issue
applies to cpumask_andnot() which doesn't actually need temporary storage
for iteration purposes.

Following what has been done for for_each_cpu_and(), introduce
for_each_cpu_andnot().

Signed-off-by: Valentin Schneider <vschneid@redhat.com>
---
 include/linux/cpumask.h | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
index 0d435d0edbcb..295b137717bb 100644
--- a/include/linux/cpumask.h
+++ b/include/linux/cpumask.h
@@ -237,6 +237,25 @@ unsigned int cpumask_next_and(int n, const struct cpumask *src1p,
 		nr_cpumask_bits, n + 1);
 }
 
+/**
+ * cpumask_next_andnot - 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 *src1p & ~*src2p
+ */
+static inline
+unsigned int cpumask_next_andnot(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_andnot_bit(cpumask_bits(src1p), cpumask_bits(src2p),
+		nr_cpumask_bits, n + 1);
+}
+
 /**
  * for_each_cpu - iterate over every cpu in a mask
  * @cpu: the (optionally unsigned) integer iterator
@@ -297,6 +316,25 @@ unsigned int __pure cpumask_next_wrap(int n, const struct cpumask *mask, int sta
 		(cpu) = cpumask_next_and((cpu), (mask1), (mask2)),	\
 		(cpu) < nr_cpu_ids;)
 
+/**
+ * for_each_cpu_andnot - iterate over every cpu in one mask but not in the other
+ * @cpu: the (optionally unsigned) integer iterator
+ * @mask1: the first cpumask pointer
+ * @mask2: the second cpumask pointer
+ *
+ * This saves a temporary CPU mask in many places.  It is equivalent to:
+ *	struct cpumask tmp;
+ *	cpumask_andnot(&tmp, &mask1, &mask2);
+ *	for_each_cpu(cpu, &tmp)
+ *		...
+ *
+ * After the loop, cpu is >= nr_cpu_ids.
+ */
+#define for_each_cpu_andnot(cpu, mask1, mask2)				\
+	for ((cpu) = -1;						\
+		(cpu) = cpumask_next_andnot((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
-- 
2.31.1

On Wed, Aug 17, 2022 at 06:58:09PM +0100, Valentin Schneider wrote:
> for_each_cpu_and() is very convenient as it saves having to allocate a
> temporary cpumask to store the result of cpumask_and(). The same issue
> applies to cpumask_andnot() which doesn't actually need temporary storage
> for iteration purposes.
> 
> Following what has been done for for_each_cpu_and(), introduce
> for_each_cpu_andnot().
> 
> Signed-off-by: Valentin Schneider <vschneid@redhat.com>

I'm concerned that this series doesn't give us real examples and tests
for the new API. If we take it as-is, we'll end up with a dead code for
a while, quite probably for long.

Can you please submit a new code with a real application for the new API?
Alternatively, you can rework some existing code.

Briefly grepping, I found good candidate in a core code: __sched_core_flip(),
and one candidate in arch code: arch/powerpc/kernel/smp.c: update_coregroup_mask.
I believe there are much more.

Regarding the test, I don't think it's strictly necessary to have it as soon as
we'll have real users, but it's always good to backup with tests.

Thanks,
Yury

On 18/08/22 15:38, Yury Norov wrote:
> On Wed, Aug 17, 2022 at 06:58:09PM +0100, Valentin Schneider wrote:
>> for_each_cpu_and() is very convenient as it saves having to allocate a
>> temporary cpumask to store the result of cpumask_and(). The same issue
>> applies to cpumask_andnot() which doesn't actually need temporary storage
>> for iteration purposes.
>>
>> Following what has been done for for_each_cpu_and(), introduce
>> for_each_cpu_andnot().
>>
>> Signed-off-by: Valentin Schneider <vschneid@redhat.com>
>
> I'm concerned that this series doesn't give us real examples and tests
> for the new API. If we take it as-is, we'll end up with a dead code for
> a while, quite probably for long.
>

Tariq has at least two uses of for_each_numa_hop_cpu() (which uses
for_each_cpu_andnot()) in net/mlx5e and net/enic). My plan here is to make
sure the cpumask and sched/topology changes are OK, and then I'd let Tariq
carry the whole set with actual users on top.

I wouldn't want to see this merged without users, especially given the
EXPORT_SYMBOL_GPL() in 3/5.

> Can you please submit a new code with a real application for the new API?
> Alternatively, you can rework some existing code.
>
> Briefly grepping, I found good candidate in a core code: __sched_core_flip(),
> and one candidate in arch code: arch/powerpc/kernel/smp.c: update_coregroup_mask.
> I believe there are much more.
>

Some of these look fairly trivial, I'll have a look around.

> Regarding the test, I don't think it's strictly necessary to have it as soon as
> we'll have real users, but it's always good to backup with tests.
>

That sounds sensible enough, I'll have a look at that.

> Thanks,
> Yury