Juergen Gross
2011-Mar-16 06:34 UTC
[Xen-devel] [Patch] 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 -- 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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2011-Mar-16 10:13 UTC
Re: [Xen-devel] [Patch] remove unneeded test for cpu in cpupool
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_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Juergen Gross
2011-Mar-16 10:36 UTC
Re: [Xen-devel] [Patch] remove unneeded test for cpu in cpupool
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2011-Mar-16 11:14 UTC
Re: [Xen-devel] [Patch] remove unneeded test for cpu in cpupool
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_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel