From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH 3/9] xen: sched: make locking for {insert, remove}_vcpu consistent Date: Tue, 29 Sep 2015 23:56:24 +0200 Message-ID: <1443563784.3276.141.camel@citrix.com> References: <20150929164726.17589.96920.stgit@Solace.station> <20150929165549.17589.76223.stgit@Solace.station> <560ACAD5.8040405@citrix.com> <1443562845.3276.134.camel@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============5761355263106274779==" Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Zh2sx-0005LG-3m for xen-devel@lists.xenproject.org; Tue, 29 Sep 2015 21:56:31 +0000 In-Reply-To: <1443562845.3276.134.camel@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Andrew Cooper , xen-devel@lists.xenproject.org Cc: George Dunlap , Meng Xu List-Id: xen-devel@lists.xenproject.org --===============5761355263106274779== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-yQArQ3eW27TOnS7eE8KU" --=-yQArQ3eW27TOnS7eE8KU Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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()? > >=20 > 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. >=20 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=3Dy Not tainted ]---- (XEN) CPU: 0 (XEN) RIP: e008:[] 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=3Dffff8300dbaf7e40: (XEN) ffff8300dbaf7e58 ffff82d08012df9d 0000000000000282 ffff8300dbaf7e7= 0 (XEN) ffff82d08012e000 ffff8300db8f4000 ffff8300dbaf7ec0 ffff82d08012b6e= 3 (XEN) ffff82d080189be6 ffff8300dbaf7eb8 ffff82d08018a008 ffff8300db8f400= 0 (XEN) ffff8300dbaf7f18 ffff83031fa29000 0000000000000001 ffff82d080291d2= 0 (XEN) ffff8300dbaf7ee0 ffff82d08010642e 00000000000002f8 ffff8300dbaf000= 0 (XEN) ffff8300dbaf7ef0 ffff82d080106579 ffff8300dbaf7f10 ffff82d0801895b= 2 (XEN) ffff8300dbaf0000 ffff8300dbaf7f18 ffff8300dbaf7fb8 008002620000000= 2 (XEN) 0000000000000000 00000000000dbaf6 ffff8300dbaf6000 ffff8300dbaf000= 0 (XEN) ffff8300dbaf0000 ffff8300dbaf7f18 ffff83031fa29000 000000000000000= 1 (XEN) ffff82d080291d20 ffff8300dbaf7f78 ffff82d080184130 ffff8300dbaf7f8= 8 (XEN) ffff82d08018546a ffff8300dbaf7f98 ffff82d080185491 ffff8300dbaf7fb= 8 (XEN) ffff82d0802c763f ffff82d0802e66c0 ffff83031fabb610 ffff82d0802f7f0= 8 (XEN) 0000000000000000 0000000000000000 0000000000000000 000000000000000= 0 (XEN) 0000000000000000 ffff8300dbb3f000 0000000000000000 000000000000000= 0 (XEN) Xen call trace: (XEN) [] check_lock+0x3d/0x41 (XEN) [] _spin_lock+0x11/0x52 (XEN) [] _spin_lock_irqsave+0xd/0x13 (XEN) [] vcpu_wake+0x36/0x3ce (XEN) [] domain_unpause+0x38/0x48 (XEN) [] domain_unpause_by_systemcontroller+0x2c/0x3b (XEN) [] init_done+0x1d/0x144 (XEN)=20 (XEN)=20 (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 --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-yQArQ3eW27TOnS7eE8KU Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iEYEABECAAYFAlYLCQgACgkQk4XaBE3IOsR1owCfXIvHRLWf/w2izaCX9gEoUXFp 8P4AoJcX6moIONM76Ht2L9NJ3oeiJuLd =u7fY -----END PGP SIGNATURE----- --=-yQArQ3eW27TOnS7eE8KU-- --===============5761355263106274779== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============5761355263106274779==--