[PATCH v2] powerpc/xive: Use cpumask_intersects()

[PATCH v2] powerpc/xive: Use cpumask_intersects()

[Costa Shulyupin]

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(-)

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

Re: [PATCH v2] powerpc/xive: Use cpumask_intersects()

[Ritesh Harjani]

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

Re: [PATCH v2] powerpc/xive: Use cpumask_intersects()

[Michael Ellerman]

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