From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH 1/3] xen: timers: don't miss a timer event because of stop_timer() Date: Wed, 27 Sep 2017 15:39:49 +0200 Message-ID: <1506519589.17428.8.camel@citrix.com> References: <150549814200.30442.14095065559616595274.stgit@Solace.fritz.box> <150549847722.30442.110948122518687861.stgit@Solace.fritz.box> <59CA8789020000780017FDDC@prv-mh.provo.novell.com> <1506449460.27663.37.camel@citrix.com> <59CB7B5D0200007800180006@prv-mh.provo.novell.com> <1506507507.17428.1.camel@citrix.com> <59CB9A02020000780018008D@prv-mh.provo.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============6490866736664370441==" Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dxCZX-0005VG-Ly for xen-devel@lists.xenproject.org; Wed, 27 Sep 2017 13:40:19 +0000 In-Reply-To: <59CB9A02020000780018008D@prv-mh.provo.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: Jan Beulich Cc: Stefano Stabellini , George Dunlap , Andrew Cooper , TimDeegan , Julien Grall , xen-devel@lists.xenproject.org List-Id: xen-devel@lists.xenproject.org --===============6490866736664370441== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="=-6tm9Tqc+vW1cbI4T0Lbg" --=-6tm9Tqc+vW1cbI4T0Lbg Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, 2017-09-27 at 04:30 -0600, Jan Beulich wrote: > > > > On 27.09.17 at 12:18, wrote: >=20 > > And that is because the following happens: > > - the CPU wants to go idle > > - sched_tick_suspend() > > =C2=A0=C2=A0=C2=A0=C2=A0rcu_idle_timer_start() > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0set_timer(RCU_idle_timer) > > - the CPU goes idle > > =C2=A0 ... ... ... > > - RCU_idle_timer's IRQ arrives > > - the CPU wakes up > > - raise_softirq(TIMER_SOFTIRQ) > > - sched_tick_resume() > > =C2=A0=C2=A0=C2=A0=C2=A0rcu_idle_timer_stop() > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0stop_timer(RCU_idle_timer) > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0deactivate_timer(RCU_id= le_timer) > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0remove_entr= y(RCU_idle_timer) // timer out of heap/list > > - do_softirq() (we are inside idle_loop()) > > - softirq_handlers[TIMER_SOFTIRQ]() > > - timer_softirq_action() > > =C2=A0=C2=A0=C2=A0=C2=A0// but the timer is not in the heap/list! >=20 > But this is an extremely special case, not something likely to > happen anywhere else. Hence I wonder whether it wouldn't > be better to handle the special case in a special way, rather > than making generic code fit the special case. > Well, yes. As said, this "new" timer is the first, and for now the only, that follow this pattern. And I also agree that this is not something we must expect to see to happen much more (if at all). Still, I continue to think that with a timer already expired, its IRQ already delivered and handled and the relative TIMER_SOFTIRQ already risen, we should arrange for the timer handler to run, in the general case. > Or wait - > wouldn't all you need be to avoid calling stop_timer() in the > call tree above, if the timer's expiry has passed (suitably > explained in a comment)? >=20 Yes. For the reason stated above, I addressed the problem at the generic code level. If that doesn't fly, I'll do like this. I had thought about that, and although I haven't tried, I think it works for this case. Thanks and Regards, Dario --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-6tm9Tqc+vW1cbI4T0Lbg 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 v2 iQIcBAABCAAGBQJZy6olAAoJEBZCeImluHPuv0YP/3ZVaTvW0aRe9XreYlwWZrnd g9iVFAI6TZkn92hKyIo6Z6QAlo/h7HhIr/8sCE5wr9576AUPNZilTIzK5GOXH31c LVS4RlYDT6OQQ2XkOvBlbPP/O5SHrFhD+Z1Dhx9F3OhFvCE/B3bqM2WUQxDOrsiF St1KDjliUq1N4+3IaHTMj72KlVcldAzV8TWFyZcoPbwwPBPNS4gpWqgU5oyS1Ozo qIpwPxLoAbK8+s9sI/OqAoXeHgzWktUskR288T5+cz9V1HgPkWeJuYSCw2YC/Ok9 ZopBIa1rdncJWeKUJmlqr2FbJSZZQacWbOlnZ+K+FJPeRwlQ3hqsyBCGjhX4TD9q OliKgxv58IdwlpwALLVAgMyAucincdch9f+78dRztvxkDUE0WZ2NLW9E5Ph0GD6N 3tEWy2iDXxQ+ICl8Zgd7Fi11k2Du37LWDahYGCM+1Dn7NE9k/t9+XUcnemfPkXUG gR4Y1UMa0uULcInScEceGh7UcYlS0QUvKsZ5ydSZNrYHeaXSueioRmVcAQWRDO79 tCCWjiDY256sSGdIW2n3RTNcOhyYzCMBrpudiHlVxThWsTboCIwsCgON+osyhDHG hnf3Rp0CN8GBAYXloqzDV7n3lqy5BraN3tQ+iX9Odc+f1YxPHmR+FwjggzIfrnR/ /v1KyhEqYo+ag5G4JMbI =cnXr -----END PGP SIGNATURE----- --=-6tm9Tqc+vW1cbI4T0Lbg-- --===============6490866736664370441== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwczovL2xpc3RzLnhlbi5v cmcveGVuLWRldmVsCg== --===============6490866736664370441==--