Mailing List Archive

On 11/23/2011 03:55 PM, Dario Faggioli wrote:
> Hi everyone,
>
> This series changes how locks are dealt with while adjusting domains'
> scheduling parameters.
>
> I've done and am still doing tests for credit and credit2, and it's
> surviving to all I threw at it up to now. Unfortunately, I can't test
> the sedf part yet, since it is not working on my test boxes due to other
> issues (which I'm also trying to track down). I'm sending the series out
> anyway, so that at least you can have a look at it, and maybe give a
> spin on the sedf part, if you don't have anything more interesting to
> do. ;-P

Sorry, does not work with sched=sedf:

nehalem1 login: (XEN) Xen BUG at spinlock.c:47
(XEN) ----[ Xen-4.2-unstable x86_64 debug=y Tainted: C ]----
(XEN) CPU: 0
(XEN) RIP: e008:[<ffff82c480124e84>] check_lock+0x44/0x50
(XEN) RFLAGS: 0000000000010046 CONTEXT: hypervisor
(XEN) rax: 0000000000000000 rbx: ffff83033eb17868 rcx: 0000000000000001
(XEN) rdx: 0000000000000000 rsi: 0000000000000001 rdi: ffff83033eb1786c
(XEN) rbp: ffff82c4802afc10 rsp: ffff82c4802afc10 r8: 0000000000000004
(XEN) r9: 000000000000003c r10: ffff82c4802285b0 r11: 0000000000000212
(XEN) r12: ffff83033eb16000 r13: 0000000000000010 r14: ffff82c4802e3c60
(XEN) r15: ffff83033eb17868 cr0: 0000000080050033 cr4: 00000000000026f0
(XEN) cr3: 000000032b937000 cr2: 00007f09a92342c0
(XEN) ds: 0000 es: 0000 fs: 0000 gs: 0000 ss: e010 cs: e008
(XEN) Xen stack trace from rsp=ffff82c4802afc10:
(XEN) ffff82c4802afc28 ffff82c480124ec1 0000000000000010 ffff82c4802afc68
(XEN) ffff82c48012c085 7400000000000002 0000000000000010 0000000000000010
(XEN) 0000000000000020 ffff82c4802e3c60 ffff83033eaf8fb0 ffff82c4802afca8
(XEN) ffff82c48012c557 ffff82c4802afd18 0000000000000010 0000000000000003
(XEN) 0000000000000004 ffff82c4802e3c60 ffff83033eaf8fb0 ffff82c4802afcc8
(XEN) ffff82c48012c660 ffff82c480124ec1 0000000000000004 ffff82c4802afd58
(XEN) ffff82c48011f8ab ffff82c480124ec1 ffff83033ea97048 ffff82c4802afd18
(XEN) ffff82c480121084 ffff82c4802afe48 ffff83033ea92000 ffff83033eb11f60
(XEN) 0000000000000296 ffff82c4802afd38 ffff82c480121aab 00000004bf2fb000
(XEN) ffff83033ea92000 000000000064b004 ffff83033ea92000 0000000000000000
(XEN) 00007fff37431340 ffff82c4802afd88 ffff82c480120fec ffff82c480125104
(XEN) fffffffffffffff3 ffff82c4802afd88 fffffffffffffff3 ffff82c4802afef8
(XEN) ffff82c4801037e3 000000000061f004 0000000000000000 000000000061f004
(XEN) 0000000000000001 ffff82c4802afef8 ffff82c480126cb8 ffff82c4802afdf8
(XEN) ffff82c480106f0e ffff002000000000 00000000000c0000 00000000ffffffff
(XEN) 0000000000000000 0000000000000000 00000000000bf7b3 00000018d63a6d18
(XEN) 0000000300000004 0000000000000000 0000000000000000 0000000000000000
(XEN) ffff82c4802afe88 0000000800000010 00007f09a9890000 0000000000000004
(XEN) 0000000000000000 0000000000000000 0000000000000000 0000000a00000001
(XEN) 0000000000000000 00007fff374314e0 00007fff37431540 000000000061e050
(XEN) Xen call trace:
(XEN) [<ffff82c480124e84>] check_lock+0x44/0x50
(XEN) [<ffff82c480124ec1>] _spin_lock+0x11/0x5d
(XEN) [<ffff82c48012c085>] xmem_pool_alloc+0x138/0x4d2
(XEN) [<ffff82c48012c557>] _xmalloc+0x138/0x230
(XEN) [<ffff82c48012c660>] _xzalloc+0x11/0x2d
(XEN) [<ffff82c48011f8ab>] sedf_adjust+0x37c/0x9b2
(XEN) [<ffff82c480120fec>] sched_adjust+0x5f/0xb7
(XEN) [<ffff82c4801037e3>] do_domctl+0xf32/0x1a9f
(XEN) [<ffff82c48021f128>] syscall_enter+0xc8/0x122
(XEN)
(XEN)
(XEN) ****************************************
(XEN) Panic on CPU 0:
(XEN) Xen BUG at spinlock.c:47
(XEN) ****************************************
(XEN)

--
Juergen Gross Principal Developer Operating Systems
PDG 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

On Tue, 2011-12-06 at 09:38 +0100, Juergen Gross wrote:
> Sorry, does not work with sched=sedf:
>
Hi Juergen,

Thanks for reporting. I'm reworking this as per Ian's suggestions, but
I'll check this out. Can I as how you made this happening? Just trying
to change a sedf-domain scheduling parameter I guess... Is that right?

Thanks again and Regards,
Dario

--
<<This happens because I choose it to happen!>> (Raistlin Majere)
-------------------------------------------------------------------
Dario Faggioli, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
PhD Candidate, ReTiS Lab, Scuola Superiore Sant'Anna, Pisa (Italy)

Hi Dario,

On 12/06/2011 11:10 AM, Dario Faggioli wrote:
> On Tue, 2011-12-06 at 09:38 +0100, Juergen Gross wrote:
>> Sorry, does not work with sched=sedf:
>>
> Hi Juergen,
>
> Thanks for reporting. I'm reworking this as per Ian's suggestions, but
> I'll check this out. Can I as how you made this happening? Just trying
> to change a sedf-domain scheduling parameter I guess... Is that right?

I booted with sched=sedf and tried to change Domain-0 parameters with:

xl sched-sedf -d Domain-0 -w 10


Juergen

--
Juergen Gross Principal Developer Operating Systems
PDG 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

On Wed, Nov 23, 2011 at 2:55 PM, Dario Faggioli <raistlin@linux.it> wrote:
> Hi everyone,
>
> This series changes how locks are dealt with while adjusting domains'
> scheduling parameters.
>
> I've done and am still doing tests for credit and credit2, and it's
> surviving to all I threw at it up to now. Unfortunately, I can't test
> the sedf part yet, since it is not working on my test boxes due to other
> issues (which I'm also trying to track down). I'm sending the series out
> anyway, so that at least you can have a look at it, and maybe give a
> spin on the sedf part, if you don't have anything more interesting to
> do. ;-P
>
> Juergen series on xl scheduling support attached to this mail, in the
> form of a single patch, for testing convenience.

Hey Dario, sorry for the long delay in responding.

Overall the patch series looks good to me. The only issue I see is
that it looks like you introduce a regression between patch 2 and 3 --
that is, if you apply patches 1 and 2, but not 3, then the lock you
grab in sched_sedf.c:sedf_adjust() won't protect against concurrent
accesses from other vcpus; nor will it correctly handle updates to the
per-vcpu EDOM_INFO() variables.

Regressions in mid-patch series are bad because it can mess up bisection.

I think a better way of breaking it down would be:
* Add scheduler global locks, and have them called in the pluggable
scheduler *_adjust() function. This is redundant, but shouldn't be
harmful.
* Atomically remove both the pause and the lock in schedule.c, and add
the scheduling locks around the per-vcpu EDOM_INFO variables in
sched_sedf.c, all in one patch. This makes the patch bigger, but it
avoids a regression. The two things are related anwyay: the reason
you now need scheduling locks around EDOM_INFO variables is because
you're getting rid of the pausing and the lock in schedule.c.

Thoughts?

-George

>
> --
>
> xen/common/sched_credit.c  |   10 +++++++---
> xen/common/sched_credit2.c |   21 +++++++++++----------
> xen/common/sched_sedf.c    |  131 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------
> xen/common/schedule.c      |   34 ++--------------------------------
> 4 files changed, 130 insertions(+), 66 deletions(-)
>
> --
> <<This happens because I choose it to happen!>> (Raistlin Majere)
> -------------------------------------------------------------------
> Dario Faggioli, http://retis.sssup.it/people/faggioli
> Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
> PhD Candidate, ReTiS Lab, Scuola Superiore Sant'Anna, Pisa (Italy)
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>

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

On Tue, Dec 6, 2011 at 8:38 AM, Juergen Gross
<juergen.gross@ts.fujitsu.com> wrote:
> (XEN) Xen BUG at spinlock.c:47
[snip]
> (XEN) Xen call trace:
> (XEN)    [<ffff82c480124e84>] check_lock+0x44/0x50
> (XEN)    [<ffff82c480124ec1>] _spin_lock+0x11/0x5d
> (XEN)    [<ffff82c48012c085>] xmem_pool_alloc+0x138/0x4d2
> (XEN)    [<ffff82c48012c557>] _xmalloc+0x138/0x230
> (XEN)    [<ffff82c48012c660>] _xzalloc+0x11/0x2d
> (XEN)    [<ffff82c48011f8ab>] sedf_adjust+0x37c/0x9b2
> (XEN)    [<ffff82c480120fec>] sched_adjust+0x5f/0xb7
> (XEN)    [<ffff82c4801037e3>] do_domctl+0xf32/0x1a9f
> (XEN)    [<ffff82c48021f128>] syscall_enter+0xc8/0x122

Hmm, looks like the problem is that we assert that locks must be
called with IRQs enabled all the time, or never. From
xen/common/spinlock.c:

* We partition locks into IRQ-safe (always held with IRQs disabled) and
* IRQ-unsafe (always held with IRQs enabled) types. The convention for
* every lock must be consistently observed else we can deadlock in
* IRQ-context rendezvous functions (a rendezvous which gets every CPU
* into IRQ context before any CPU is released from the rendezvous).

sedf_adj() grabs the private lock with irqs disabled, then calls
sedf_adjust_weights(), which calls xmalloc to allocate some local
scratch space, which ultimately grabs a non-IRQ spinlock.

Not sure the best thing to do here...

-George

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

On Tue, Dec 6, 2011 at 1:24 PM, George Dunlap
<George.Dunlap@eu.citrix.com> wrote:
> Hey Dario,  sorry for the long delay in responding.
>
Hi, and no problem at all :-)

> Regressions in mid-patch series are bad because it can mess up bisection.
>
I strongly agree. I didn't think much about that for this series since
the mechanism
I was trying to fix was (especially for sedf) already broken in
various ways but,
indeed, patches could be better split.

> I think a better way of breaking it down would be:
> * Add scheduler global locks, and have them called in the pluggable
> scheduler *_adjust() function.  This is redundant, but shouldn't be
> harmful.
> * Atomically remove both the pause and the lock in schedule.c, and add
> the scheduling locks around the per-vcpu EDOM_INFO variables in
> sched_sedf.c, all in one patch.  This makes the patch bigger, but it
> avoids a regression.
>
Ok, I'll go for this.

> The two things are related anwyay: the reason
> you now need scheduling locks around EDOM_INFO variables is because
> you're getting rid of the pausing and the lock in schedule.c.
>
Yeah, what I was trying to do was leaving a clear footstep about why
pausing was going and, yes, also making patches smaller, but again, I
see your point and will follow your advice. :-)

Thanks and Regards,
Dario

--
<<This happens because I choose it to happen!>> (Raistlin Majere)
----------------------------------------------------------------------
Dario Faggioli, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
PhD Candidate, ReTiS Lab, Scuola Superiore Sant'Anna, Pisa  (Italy)

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