* [Patch] remove unneeded test for cpu in cpupool
@ 2011-03-16 6:34 Juergen Gross
2011-03-16 10:13 ` Keir Fraser
0 siblings, 1 reply; 4+ messages in thread
From: Juergen Gross @ 2011-03-16 6:34 UTC (permalink / raw)
To: xen-devel, Przywara, Andre
[-- Attachment #1: Type: text/plain, Size: 671 bytes --]
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
--
Juergen Gross Principal Developer Operating Systems
TSP ES&S SWE OS6 Telephone: +49 (0) 89 3222 2967
Fujitsu Technology Solutions e-mail: juergen.gross@ts.fujitsu.com
Domagkstr. 28 Internet: ts.fujitsu.com
D-80807 Muenchen Company details: ts.fujitsu.com/imprint.html
[-- Attachment #2: cputest.patch --]
[-- Type: text/x-patch, Size: 1240 bytes --]
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: juergen.gross@ts.fujitsu.com
diff -r 3caed2112c65 xen/common/schedule.c
--- a/xen/common/schedule.c Tue Mar 15 10:14:27 2011 +0000
+++ b/xen/common/schedule.c Wed Mar 16 07:26:38 2011 +0100
@@ -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;
}
[-- Attachment #3: Type: text/plain, Size: 138 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Patch] remove unneeded test for cpu in cpupool
2011-03-16 6:34 [Patch] remove unneeded test for cpu in cpupool Juergen Gross
@ 2011-03-16 10:13 ` Keir Fraser
2011-03-16 10:36 ` Juergen Gross
0 siblings, 1 reply; 4+ messages in thread
From: Keir Fraser @ 2011-03-16 10:13 UTC (permalink / raw)
To: Juergen Gross, xen-devel, Przywara, Andre
On 16/03/2011 06:34, "Juergen Gross" <juergen.gross@ts.fujitsu.com> wrote:
> 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
>
> Juergen
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Patch] remove unneeded test for cpu in cpupool
2011-03-16 10:13 ` Keir Fraser
@ 2011-03-16 10:36 ` Juergen Gross
2011-03-16 11:14 ` Keir Fraser
0 siblings, 1 reply; 4+ messages in thread
From: Juergen Gross @ 2011-03-16 10:36 UTC (permalink / raw)
To: Keir Fraser; +Cc: Przywara, Andre, xen-devel
On 03/16/11 11:13, Keir Fraser wrote:
> On 16/03/2011 06:34, "Juergen Gross"<juergen.gross@ts.fujitsu.com> wrote:
>
>> 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.
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
--
Juergen Gross Principal Developer Operating Systems
TSP ES&S SWE OS6 Telephone: +49 (0) 89 3222 2967
Fujitsu Technology Solutions e-mail: juergen.gross@ts.fujitsu.com
Domagkstr. 28 Internet: ts.fujitsu.com
D-80807 Muenchen Company details: ts.fujitsu.com/imprint.html
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Patch] remove unneeded test for cpu in cpupool
2011-03-16 10:36 ` Juergen Gross
@ 2011-03-16 11:14 ` Keir Fraser
0 siblings, 0 replies; 4+ messages in thread
From: Keir Fraser @ 2011-03-16 11:14 UTC (permalink / raw)
To: Juergen Gross; +Cc: Przywara, Andre, xen-devel
On 16/03/2011 10:36, "Juergen Gross" <juergen.gross@ts.fujitsu.com> wrote:
> On 03/16/11 11:13, Keir Fraser wrote:
>> On 16/03/2011 06:34, "Juergen Gross"<juergen.gross@ts.fujitsu.com> wrote:
>>
>>> 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.
>
> 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.
Ah no, if it's more subtle than it appears, and it's not a straighjtforward
post-condition, then I wouldn't bother.
-- Keir
>
> Juergen
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-03-16 11:14 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-03-16 6:34 [Patch] remove unneeded test for cpu in cpupool Juergen Gross
2011-03-16 10:13 ` Keir Fraser
2011-03-16 10:36 ` Juergen Gross
2011-03-16 11:14 ` Keir Fraser
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.