Xen devel - Apr 2009 - next->vcpu_dirty_cpumask checking at the top of context_switch()

In an attempt to create a patch to remove some of the cpumask copying
(in order to reduce stack usage when NR_CPUS is huge) one of the obvious
things to do was to change function parameters to pointer-to-cpumask.
However, doing so for flush_area_mask() creates the unintended side
effect of triggering the WARN_ON() at the top of send_IPI_mask_flat(),
apparently because next->vcpu_dirty_cpumask can occasionally change
between the call site of flush_tlb_mask() in context_switch() and that low
level routine.

That by itself certainly is not a problem, what puzzles me are the redundant
!cpus_empty() checks prior to the call to flush_tlb_mask() as well as the
fact that if I''m hitting a possible timing window here, then I
can''t see why
it shouldn''t be possible to hit the (albeit much smaller) window
between the
second !cpus_empty() check and the point where the cpumask got fully
copied to the stack as flush_tlb_mask()''s argument.

Bottom line question is - can''t the second !cpus_empty() check go away
altogether, and shouldn''t the argument passed to flush_tlb_mask() be
dirty_mask instead of next->vcpu_dirty_cpumask?

Thanks for any insights,
Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

How big NR_CPUS are we talking about? Is the overhead measurable, or is this
a premature micro-optimisation?

 -- Keir

On 16/04/2009 16:16, "Jan Beulich" <jbeulich@novell.com> wrote:
> In an attempt to create a patch to remove some of the cpumask copying
> (in order to reduce stack usage when NR_CPUS is huge) one of the obvious
> things to do was to change function parameters to pointer-to-cpumask.
> However, doing so for flush_area_mask() creates the unintended side
> effect of triggering the WARN_ON() at the top of send_IPI_mask_flat(),
> apparently because next->vcpu_dirty_cpumask can occasionally change
> between the call site of flush_tlb_mask() in context_switch() and that low
> level routine.
> 
> That by itself certainly is not a problem, what puzzles me are the
redundant
> !cpus_empty() checks prior to the call to flush_tlb_mask() as well as the
> fact that if I''m hitting a possible timing window here, then I
can''t see why
> it shouldn''t be possible to hit the (albeit much smaller) window
between the
> second !cpus_empty() check and the point where the cpumask got fully
> copied to the stack as flush_tlb_mask()''s argument.
> 
> Bottom line question is - can''t the second !cpus_empty() check go
away
> altogether, and shouldn''t the argument passed to flush_tlb_mask()
be
> dirty_mask instead of next->vcpu_dirty_cpumask?
> 
> Thanks for any insights,
> Jan
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 16/04/2009 16:16, "Jan Beulich" <jbeulich@novell.com> wrote:
> Bottom line question is - can''t the second !cpus_empty() check go
away
> altogether, and shouldn''t the argument passed to flush_tlb_mask()
be
> dirty_mask instead of next->vcpu_dirty_cpumask?
If you think cpus_empty() checks/warns/bugs could do with sanitising, please
send that on separately from other performance-related changes. And
I''ll let
you know what I think of it when I can see the all the details.

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

>>> Keir Fraser <keir.fraser@eu.citrix.com> 04/16/09 6:00 PM
>>>
>How big NR_CPUS are we talking about? Is the overhead measurable, or is this
>a premature micro-optimisation?
We''re shipping Xen in SLE11 with NR_CPUS=255, and I''m
convinced we''ll be asked to up this further in the service packs.

Also, micro-optimization reads for me like I was aiming at a performance issue,
but the goal really just is to get the use of stack space down (and in
particular, make it as independent as possible from configuration settings).

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 17/04/2009 14:58, "Jan Beulich" <jbeulich@novell.com> wrote:
>>>> Keir Fraser <keir.fraser@eu.citrix.com> 04/16/09 6:00 PM
>>>
>> How big NR_CPUS are we talking about? Is the overhead measurable, or is
this
>> a premature micro-optimisation?
> 
> We''re shipping Xen in SLE11 with NR_CPUS=255, and I''m
convinced we''ll be asked
> to up this further in the service packs.
> 
> Also, micro-optimization reads for me like I was aiming at a performance
> issue, but the goal really just is to get the use of stack space down (and
in
> particular, make it as independent as possible from configuration
settings).
Well, I''m not against it, at least if it''s a reasonably
straightforward
patch. Any untangling of cpus_empty() logic should be a separate patch
though. And that should mean that the patch to convert from pass-by-value to
pass-by-pointer is nice and trivial.

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel