Skip to contentSkip to conversation

remove unneeded test for cpu in cpupool

PATCH In review Juergen Gross 2 participants 4 messages Mar 16, 2011

lore
Share & tools
lore

Juergen Gross

author
#0 lore raw

PATCH 1/1 remove unneeded test for cpu in cpupool

Hi,

attached patch removes an unneeded check in vcpu_migrate().

Andre, could you please give it a try on your 48 core machine with

xl cpupool-numa-split

I'd like to make sure the check is really not needed. On my machines it worked okay, but your machine seems to trigger races more easily.

Juergen

Signature · 5 lines
Juergen Gross                 Principal Developer Operating Systems
TSP ES&S SWE OS6                       Telephone: +49 (0) 89 3222 2967
Fujitsu Technology Solutions              e-mail: [email protected]
Domagkstr. 28                           Internet: ts.fujitsu.com
D-80807 Muenchen                 Company details: ts.fujitsu.com/imprint.html

This patch removes an unneeded check in vcpu_migrate() for the picked cpu to be in the current cpupool. pick_cpu should only return cpus in the correct cpupool.

Signed-off-by [email protected]
xen/common/schedule.c +2 -3
diff -r 3caed2112c65 xen/common/schedule.c
@@ -431,7 +431,7 @@ static void vcpu_migrate(struct vcpu *v)
         if ( old_lock == per_cpu(schedule_data, old_cpu).schedule_lock )
         {
             /*
-             * If we selected a CPU on the previosu iteration, check if it
+             * If we selected a CPU on the previous iteration, check if it
              * remains suitable for running this vCPU.
              */
             if ( pick_called &&

@@ -442,8 +442,7 @@ static void vcpu_migrate(struct vcpu *v)
 
             /* Select a new CPU. */
             new_cpu = SCHED_OP(VCPU2OP(v), pick_cpu, v);
-            if ( (new_lock == per_cpu(schedule_data, new_cpu).schedule_lock) &&
-                 cpu_isset(new_cpu, v->domain->cpupool->cpu_valid) )
+            if ( new_lock == per_cpu(schedule_data, new_cpu).schedule_lock )
                 break;
             pick_called = 1;
         }

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

#1 lore raw
Quoted 10 lines · Juergen Gross

Hi,

attached patch removes an unneeded check in vcpu_migrate().

Andre, could you please give it a try on your 48 core machine with

xl cpupool-numa-split

I'd like to make sure the check is really not needed. On my machines it worked okay, but your machine seems to trigger races more easily.

Could you BUG_ON failing this test when we exit the retry loop? It seems a suitable loop post-condition.

-- Keir

Quoted 2 lines

Juergen

author
#2 lore raw
Quoted 8 lines · Keir Fraser
Quoted 6 lines · Juergen Gross

Hi,

attached patch removes an unneeded check in vcpu_migrate().

Andre, could you please give it a try on your 48 core machine with

Quoted 7 lines
Quoted 4 lines

xl cpupool-numa-split

I'd like to make sure the check is really not needed. On my machines it worked okay, but your machine seems to trigger races more easily.

Could you BUG_ON failing this test when we exit the retry loop? It seems a suitable loop post-condition.

Not easily. The cpu_valid mask of a cpupool might change between the call of pick_cpu and the test. In the normal case this is okay, as removing a cpu from a cpupool (which is the critical case) will call cpu_disable_scheduler() after removing the cpu from the mask, which will migrate the vcpu away.

Using a saved copy of cpu_valid for the test isn't working either, if a cpu is added to and removed from the cpupool in a very short time. I would have to use a generation count for the cpu_valid mask and do the check only if the count didn't change. I think this is little bit of overkill here, but if you'd prefer it, I can do a patch.

Juergen

Signature · 5 lines
Juergen Gross                 Principal Developer Operating Systems
TSP ES&S SWE OS6                       Telephone: +49 (0) 89 3222 2967
Fujitsu Technology Solutions              e-mail: [email protected]
Domagkstr. 28                           Internet: ts.fujitsu.com
D-80807 Muenchen                 Company details: ts.fujitsu.com/imprint.html
#3 lore raw
Quoted 23 lines · Juergen Gross
Quoted 8 lines · Keir Fraser
Quoted 6 lines · Juergen Gross

Hi,

attached patch removes an unneeded check in vcpu_migrate().

Andre, could you please give it a try on your 48 core machine with

Quoted 8 lines
Quoted 5 lines

xl cpupool-numa-split

I'd like to make sure the check is really not needed. On my machines it worked
okay, but your machine seems to trigger races more easily.

Could you BUG_ON failing this test when we exit the retry loop? It seems a suitable loop post-condition.

Not easily. The cpu_valid mask of a cpupool might change between the call of pick_cpu and the test. In the normal case this is okay, as removing a cpu from a cpupool (which is the critical case) will call cpu_disable_scheduler() after removing the cpu from the mask, which will migrate the vcpu away.

Quoted 5 lines

Using a saved copy of cpu_valid for the test isn't working either, if a cpu is added to and removed from the cpupool in a very short time. I would have to use a generation count for the cpu_valid mask and do the check only if the count didn't change. I think this is little bit of overkill here, but if you'd prefer it, I can do a patch.

Ah no, if it's more subtle than it appears, and it's not a straighjtforward post-condition, then I wouldn't bother.

-- Keir

Quoted 2 lines

Juergen