| Message ID | 20240926092623.399577-2-costa.shul@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 6da1cab4f5f8eb778fd61f0eb6ca5b0a011dd44d |
| Headers | show |
| Series | [v2] powerpc/xive: Use cpumask_intersects() | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/github-powerpc_ppctests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_selftests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_sparse | success | Successfully ran 4 jobs. |
| snowpatch_ozlabs/github-powerpc_clang | success | Successfully ran 5 jobs. |
| snowpatch_ozlabs/github-powerpc_kernel_qemu | success | Successfully ran 21 jobs. |
Costa Shulyupin <costa.shul@redhat.com> writes: > Replace `cpumask_any_and(a, b) >= nr_cpu_ids` > with the more readable `!cpumask_intersects(a, b)`. > > Comparison between cpumask_any_and() and cpumask_intersects() > > The cpumask_any_and() function expands using FIND_FIRST_BIT(), > resulting in a loop that iterates through each bit of the bitmask: > > for (idx = 0; idx * BITS_PER_LONG < sz; idx++) { > val = (FETCH); > if (val) { > sz = min(idx * BITS_PER_LONG + __ffs(MUNGE(val)), sz); > break; > } > } > > The cpumask_intersects() function expands using __bitmap_intersects(), > resulting in that the first loop iterates through each long word of the bitmask, > and the second through each bit within a long word: > > unsigned int k, lim = bits/BITS_PER_LONG; > for (k = 0; k < lim; ++k) > if (bitmap1[k] & bitmap2[k]) > return true; > > if (bits % BITS_PER_LONG) > if ((bitmap1[k] & bitmap2[k]) & BITMAP_LAST_WORD_MASK(bits)) > return true; > > Conclusion: cpumask_intersects() is at least as efficient as cpumask_any_and(), > if not more so, as it typically performs fewer loops and comparisons. > I agree with the analysis in above. cpumask_any_and() has to get the first set bit from the two cpumask for which it also does some additional calculations like __ffs(). whereas cpumask_intersects() has to only check if any of the bits is set hence does fewer operations. Looks good to me. Please feel free to add - Reviewed-by: Ritesh Harjani (IBM) <ritesh.harjani@gmail.com> > Signed-off-by: Costa Shulyupin <costa.shul@redhat.com> > Reviewed-by: Ming Lei <ming.lei@redhat.com> > > --- > > v2: add comparison between cpumask_any_and() and cpumask_intersects() > > --- > arch/powerpc/sysdev/xive/common.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c > index fa01818c1972c..a6c388bdf5d08 100644 > --- a/arch/powerpc/sysdev/xive/common.c > +++ b/arch/powerpc/sysdev/xive/common.c > @@ -726,7 +726,7 @@ static int xive_irq_set_affinity(struct irq_data *d, > pr_debug("%s: irq %d/0x%x\n", __func__, d->irq, hw_irq); > > /* Is this valid ? */ > - if (cpumask_any_and(cpumask, cpu_online_mask) >= nr_cpu_ids) > + if (!cpumask_intersects(cpumask, cpu_online_mask)) > return -EINVAL; > > /* > -- > 2.45.0
On Thu, 26 Sep 2024 12:26:22 +0300, Costa Shulyupin wrote: > Replace `cpumask_any_and(a, b) >= nr_cpu_ids` > with the more readable `!cpumask_intersects(a, b)`. > > Comparison between cpumask_any_and() and cpumask_intersects() > > The cpumask_any_and() function expands using FIND_FIRST_BIT(), > resulting in a loop that iterates through each bit of the bitmask: > > [...] Applied to powerpc/next. [1/1] powerpc/xive: Use cpumask_intersects() https://git.kernel.org/powerpc/c/6da1cab4f5f8eb778fd61f0eb6ca5b0a011dd44d cheers
diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c index fa01818c1972c..a6c388bdf5d08 100644 --- a/arch/powerpc/sysdev/xive/common.c +++ b/arch/powerpc/sysdev/xive/common.c @@ -726,7 +726,7 @@ static int xive_irq_set_affinity(struct irq_data *d, pr_debug("%s: irq %d/0x%x\n", __func__, d->irq, hw_irq); /* Is this valid ? */ - if (cpumask_any_and(cpumask, cpu_online_mask) >= nr_cpu_ids) + if (!cpumask_intersects(cpumask, cpu_online_mask)) return -EINVAL; /*
Replace `cpumask_any_and(a, b) >= nr_cpu_ids` with the more readable `!cpumask_intersects(a, b)`. Comparison between cpumask_any_and() and cpumask_intersects() The cpumask_any_and() function expands using FIND_FIRST_BIT(), resulting in a loop that iterates through each bit of the bitmask: for (idx = 0; idx * BITS_PER_LONG < sz; idx++) { val = (FETCH); if (val) { sz = min(idx * BITS_PER_LONG + __ffs(MUNGE(val)), sz); break; } } The cpumask_intersects() function expands using __bitmap_intersects(), resulting in that the first loop iterates through each long word of the bitmask, and the second through each bit within a long word: unsigned int k, lim = bits/BITS_PER_LONG; for (k = 0; k < lim; ++k) if (bitmap1[k] & bitmap2[k]) return true; if (bits % BITS_PER_LONG) if ((bitmap1[k] & bitmap2[k]) & BITMAP_LAST_WORD_MASK(bits)) return true; Conclusion: cpumask_intersects() is at least as efficient as cpumask_any_and(), if not more so, as it typically performs fewer loops and comparisons. Signed-off-by: Costa Shulyupin <costa.shul@redhat.com> Reviewed-by: Ming Lei <ming.lei@redhat.com> --- v2: add comparison between cpumask_any_and() and cpumask_intersects() --- arch/powerpc/sysdev/xive/common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)