From: Dario Faggioli <dario.faggioli@citrix.com>
To: Andrew Cooper <andrew.cooper3@citrix.com>,
xen-devel@lists.xenproject.org
Cc: George Dunlap <george.dunlap@eu.citrix.com>,
Meng Xu <mengxu@cis.upenn.edu>
Subject: Re: [PATCH 3/9] xen: sched: make locking for {insert, remove}_vcpu consistent
Date: Tue, 29 Sep 2015 23:56:24 +0200 [thread overview]
Message-ID: <1443563784.3276.141.camel@citrix.com> (raw)
In-Reply-To: <1443562845.3276.134.camel@citrix.com>
[-- Attachment #1.1: Type: text/plain, Size: 3870 bytes --]
On Tue, 2015-09-29 at 23:40 +0200, Dario Faggioli wrote:
> On Tue, 2015-09-29 at 18:31 +0100, Andrew Cooper wrote:
> > On 29/09/15 17:55, Dario Faggioli wrote:
> > Is the use of _lock_irq() here to cover another issue expecting
> > interrupts to be disabled, or could it be replaced with a plain
> > spin_lock()?
> >
> As said, it is probably the case that spin_lock() would be ok, here
> as
> well as elsewhere. This is being done like this in this patch for
> consistency, as that is what happens **everywhere** else in
> scheduling
> code. In fact, I haven't tried, but it may well be the case that,
> converting only one (or a subset) of locks to non _irq* variants,
> we'd
> make check_lock() complain.
>
And now I have just checked, and that indeed looks to be the case.
I.e., I changed the spin_lock_irq() being touched in this patch to
spin_lock(), and here's what I see (very early, during Xen's boot):
(XEN) Xen BUG at spinlock.c:48
(XEN) ----[ Xen-4.7-unstable x86_64 debug=y Not tainted ]----
(XEN) CPU: 0
(XEN) RIP: e008:[<ffff82d08012df6e>] check_lock+0x3d/0x41
(XEN) RFLAGS: 0000000000010046 CONTEXT: hypervisor
(XEN) rax: 0000000000000000 rbx: ffff82d08033cd68 rcx: 0000000000000001
(XEN) rdx: 0000000000000000 rsi: 0000000000000001 rdi: ffff82d08033cd70
(XEN) rbp: ffff8300dbaf7e40 rsp: ffff8300dbaf7e40 r8: 0000000000000001
(XEN) r9: 0000000000000001 r10: 0000000000000001 r11: 0000000000000004
(XEN) r12: ffff82d0803272a0 r13: ffff83031fa29000 r14: ffff82d08033cd60
(XEN) r15: ffff82d08033cd68 cr0: 000000008005003b cr4: 00000000000026e0
(XEN) cr3: 00000000dbaa3000 cr2: 0000000000000000
(XEN) ds: 0000 es: 0000 fs: 0000 gs: 0000 ss: 0000 cs: e008
(XEN) Xen stack trace from rsp=ffff8300dbaf7e40:
(XEN) ffff8300dbaf7e58 ffff82d08012df9d 0000000000000282 ffff8300dbaf7e70
(XEN) ffff82d08012e000 ffff8300db8f4000 ffff8300dbaf7ec0 ffff82d08012b6e3
(XEN) ffff82d080189be6 ffff8300dbaf7eb8 ffff82d08018a008 ffff8300db8f4000
(XEN) ffff8300dbaf7f18 ffff83031fa29000 0000000000000001 ffff82d080291d20
(XEN) ffff8300dbaf7ee0 ffff82d08010642e 00000000000002f8 ffff8300dbaf0000
(XEN) ffff8300dbaf7ef0 ffff82d080106579 ffff8300dbaf7f10 ffff82d0801895b2
(XEN) ffff8300dbaf0000 ffff8300dbaf7f18 ffff8300dbaf7fb8 0080026200000002
(XEN) 0000000000000000 00000000000dbaf6 ffff8300dbaf6000 ffff8300dbaf0000
(XEN) ffff8300dbaf0000 ffff8300dbaf7f18 ffff83031fa29000 0000000000000001
(XEN) ffff82d080291d20 ffff8300dbaf7f78 ffff82d080184130 ffff8300dbaf7f88
(XEN) ffff82d08018546a ffff8300dbaf7f98 ffff82d080185491 ffff8300dbaf7fb8
(XEN) ffff82d0802c763f ffff82d0802e66c0 ffff83031fabb610 ffff82d0802f7f08
(XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000
(XEN) 0000000000000000 ffff8300dbb3f000 0000000000000000 0000000000000000
(XEN) Xen call trace:
(XEN) [<ffff82d08012df6e>] check_lock+0x3d/0x41
(XEN) [<ffff82d08012df9d>] _spin_lock+0x11/0x52
(XEN) [<ffff82d08012e000>] _spin_lock_irqsave+0xd/0x13
(XEN) [<ffff82d08012b6e3>] vcpu_wake+0x36/0x3ce
(XEN) [<ffff82d08010642e>] domain_unpause+0x38/0x48
(XEN) [<ffff82d080106579>] domain_unpause_by_systemcontroller+0x2c/0x3b
(XEN) [<ffff82d0801895b2>] init_done+0x1d/0x144
(XEN)
(XEN)
(XEN) ****************************************
(XEN) Panic on CPU 0:
(XEN) Xen BUG at spinlock.c:48
(XEN) ****************************************
So, really, (trying to)do the spinlock conversion in its dedicated
series is the way to go, IMO.
Regards,
Dario
--
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
next prev parent reply other threads:[~2015-09-29 21:56 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-09-29 16:55 [PATCH 0/9] xen: sched: improve (a lot! :-D) Credit2 runqueue handling Dario Faggioli
2015-09-29 16:55 ` [PATCH 1/9] xen: sched: fix an 'off by one \t' in credit2 debug dump Dario Faggioli
2015-10-01 5:22 ` Juergen Gross
2015-10-08 14:09 ` George Dunlap
2015-09-29 16:55 ` [PATCH 2/9] xen: sched: improve scope and placement of credit2 boot parameters Dario Faggioli
2015-10-01 5:23 ` Juergen Gross
2015-10-01 7:51 ` Jan Beulich
2015-10-01 8:17 ` Dario Faggioli
2015-09-29 16:55 ` [PATCH 3/9] xen: sched: make locking for {insert, remove}_vcpu consistent Dario Faggioli
2015-09-29 17:31 ` Andrew Cooper
2015-09-29 21:40 ` Dario Faggioli
2015-09-29 21:56 ` Dario Faggioli [this message]
2015-09-30 9:00 ` Andrew Cooper
2015-10-08 14:58 ` George Dunlap
2015-10-08 15:20 ` Andrew Cooper
2015-10-08 16:46 ` George Dunlap
2015-10-08 17:23 ` Andrew Cooper
2015-10-08 20:44 ` Dario Faggioli
2015-10-12 9:44 ` George Dunlap
2015-10-08 20:39 ` Dario Faggioli
2015-10-09 13:05 ` Andrew Cooper
2015-10-09 16:56 ` Dario Faggioli
2015-10-01 8:03 ` Jan Beulich
2015-10-01 11:59 ` Dario Faggioli
2015-09-29 16:55 ` [PATCH 4/9] xen: sched: add .init_pdata hook to the scheduler interface Dario Faggioli
2015-10-01 5:21 ` Juergen Gross
2015-10-01 6:33 ` Dario Faggioli
2015-10-01 7:43 ` Juergen Gross
2015-10-01 9:32 ` Andrew Cooper
2015-10-01 9:40 ` Dario Faggioli
2015-10-01 8:17 ` Jan Beulich
2015-10-01 9:26 ` Dario Faggioli
2015-10-01 10:12 ` Jan Beulich
2015-10-01 10:35 ` Dario Faggioli
2015-10-01 10:47 ` Jan Beulich
2015-09-29 16:56 ` [PATCH 5/9] xen: sched: make implementing .alloc_pdata optional Dario Faggioli
2015-10-01 5:28 ` Juergen Gross
2015-10-01 6:35 ` Dario Faggioli
2015-10-01 7:49 ` Jan Beulich
2015-10-01 8:13 ` Dario Faggioli
2015-09-29 16:56 ` [PATCH 6/9] xen: sched: implement .init_pdata in all schedulers Dario Faggioli
2015-09-29 16:56 ` [PATCH 7/9] xen: sched: fix per-socket runqueue creation in credit2 Dario Faggioli
2015-09-29 16:56 ` [PATCH 8/9] xen: sched: allow for choosing credit2 runqueues configuration at boot Dario Faggioli
2015-10-01 5:48 ` Juergen Gross
2015-10-01 7:23 ` Dario Faggioli
2015-10-01 7:46 ` Juergen Gross
2015-09-29 16:56 ` [PATCH 9/9] xen: sched: per-core runqueues as default in credit2 Dario Faggioli
2015-10-01 5:48 ` Juergen Gross
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1443563784.3276.141.camel@citrix.com \
--to=dario.faggioli@citrix.com \
--cc=andrew.cooper3@citrix.com \
--cc=george.dunlap@eu.citrix.com \
--cc=mengxu@cis.upenn.edu \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).