Re: [Xen-devel] [PATCH for 4.7 1/4] xen: sched: avoid spuriously re-enabling IRQs in csched2_switch_sched()

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Fri, May 06, 2016 at 03:21:40PM +0200, Dario Faggioli wrote:
> On Wed, 2016-05-04 at 18:34 +0100, George Dunlap wrote:
> > On 04/05/16 18:21, Dario Faggioli wrote:
> > > 
> > > After all, I'm fine with an ASSERT() too, but then I think we
> > > should
> > > add one to the same effect to csched_switch_sched() too.
> > Well an ASSERT() is sort of like a comment, in that if you see
> > ASSERT(irqs_disabled()), you know there's no need to save irqs
> > because
> > they should already disabled.  But it has the advantage that osstest
> > will be able to "read" it once we get some proper cpupool tests for
> > osstest. :-)
> > 
> > If we weren't in the feature freeze, I'd definitely favor adding an
> > ASSERT to credit1.  As it is, I think either way (adding now or
> > waiting
> > until the 4.8 development window) should be fine.
> > 
> Ok, here you go (inline and attached) the patch with ASSERT()-s both in
> Credit2 and Credit1 (despite the freeze, I think it's the best thing to
> do, see the changelog).
> 
> Thanks and Regards,
> Dario
> --
> commit cbabd44e171d0bd2169f1c7100e69a9e48289980
> Author: Dario Faggioli <dario.faggioli@xxxxxxxxxx>
> Date:   Tue Apr 26 18:56:56 2016 +0200
> 
>     xen: sched: avoid spuriously re-enabling IRQs in csched2_switch_sched()
>     
>     interrupts are already disabled when calling the hook
>     (from schedule_cpu_switch()), so we must use spin_lock()
>     and spin_unlock().
>     
>     Add an ASSERT(), so we will notice if this code and its
>     caller get out of sync with respect to disabling interrupts
>     (and add one at the same exact occurrence of this pattern
>     in Credit1 too)
>     
>     Signed-off-by: Dario Faggioli <dario.faggioli@xxxxxxxxxx>
>     ---
>     Cc: George Dunlap <george.dunlap@xxxxxxxxxx>
>     Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
>     --
>     Changes from v1:
>      * add the ASSERT(), as requested by George
>      * add the ASSERT in Credit1 too
>     --
>     For Wei:
>      - the Credit2 spin_lock_irq()-->spin_lock() change needs
>        to go in, as it fixes a bug;
>      - adding the ASSERT was requested during review;
>      - adding the ASSERT in Credit1 is not strictly necessary,
>        but imptoves code quality and consistency at zero cost
>        and risk, so I think we should just go for it now, instead
>        of waitign for 4.8 (it's basically like I'm adding a
>        comment!).
> 

Release-acked-by: Wei Liu <wei.liu2@xxxxxxxxxx>

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel