Hi,

On 04/02/2022 20:31, Andrew Cooper wrote:
cpumask_weight() is a horribly expensive way to find if no bits are set, made
worse by the fact that the calculation is performed with the global call_lock
held.

I looked at the archive because I was wondering why we were using cpumask_weight here. It looks like this was a left-over of the rework in ac3fc35d919c "x86: Fix flush_area_mask() and on_selected_cpus() to not race updates".


Switch to using cpumask_empty() instead, which will short circuit as soon as
it find any set bit in the cpumask.

Signed-off-by: Andrew Cooper <[email protected]>

Reviewed-by: Julien Grall <[email protected]>

---
CC: Jan Beulich <[email protected]>
CC: Roger Pau MonnĂ© <[email protected]>
CC: Wei Liu <[email protected]>
CC: Juergen Gross <[email protected]>
CC: Stefano Stabellini <[email protected]>
CC: Julien Grall <[email protected]>
CC: Volodymyr Babchuk <[email protected]>
CC: Bertrand Marquis <[email protected]>

I have not done performance testing, but I would be surprised if this cannot
be measured on a busy or large box.
---
  xen/common/smp.c | 5 +----
  1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/xen/common/smp.c b/xen/common/smp.c
index 781bcf2c246c..a011f541f1ea 100644
--- a/xen/common/smp.c
+++ b/xen/common/smp.c
@@ -50,8 +50,6 @@ void on_selected_cpus(
      void *info,
      int wait)
  {
-    unsigned int nr_cpus;
-
      ASSERT(local_irq_is_enabled());
      ASSERT(cpumask_subset(selected, &cpu_online_map));
@@ -59,8 +57,7 @@ void on_selected_cpus( cpumask_copy(&call_data.selected, selected); - nr_cpus = cpumask_weight(&call_data.selected);
-    if ( nr_cpus == 0 )
+    if ( cpumask_empty(&call_data.selected) )
          goto out;
call_data.func = func;

--
Julien Grall

Reply via email to