From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH 08/24] xen: tracing: add trace records for schedule and rate-limiting. Date: Thu, 18 Aug 2016 11:41:54 +0200 Message-ID: <1471513314.6806.63.camel@citrix.com> References: <147145358844.25877.7490417583264534196.stgit@Solace.fritz.box> <147145430952.25877.10937525630692282780.stgit@Solace.fritz.box> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============2760735618110779276==" Return-path: Received: from mail6.bemta6.messagelabs.com ([193.109.254.103]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1baJpq-0004ZW-Or for xen-devel@lists.xenproject.org; Thu, 18 Aug 2016 09:42:02 +0000 In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: Meng Xu Cc: George Dunlap , "xen-devel@lists.xenproject.org" , Anshul Makkar List-Id: xen-devel@lists.xenproject.org --===============2760735618110779276== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="=-8BNQumjm2jGGKF0nPN/9" --=-8BNQumjm2jGGKF0nPN/9 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, 2016-08-17 at 20:57 -0400, Meng Xu wrote: > On Wed, Aug 17, 2016 at 1:18 PM, Dario Faggioli > wrote: > >=C2=A0 > > diff --git a/xen/common/sched_rt.c b/xen/common/sched_rt.c > > index 41c61a7..903dbd8 100644 > > --- a/xen/common/sched_rt.c > > +++ b/xen/common/sched_rt.c > >=C2=A0 > > @@ -1035,6 +1036,20 @@ rt_schedule(const struct scheduler *ops, > > s_time_t now, bool_t tasklet_work_sched > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0struct rt_vcpu *snext =3D NULL; > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0struct task_slice ret =3D { .migrated =3D= 0 }; > >=20 > > +=C2=A0=C2=A0=C2=A0=C2=A0/* TRACE */ > > +=C2=A0=C2=A0=C2=A0=C2=A0{ > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0struct __packed { > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0unsigned cpu:17, tasklet:8, tickled:4, idle:4; > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0} d; > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0d.cpu =3D cpu; > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0d.tasklet =3D tasklet_= work_scheduled; > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0d.tickled =3D cpumask_= test_cpu(cpu, &prv->tickled); > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0d.idle =3D is_idle_vcp= u(current); > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0__trace_var(TRC_RTDS_S= CHEDULE, 1, > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0sizeof(d), > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0(unsigned char *)&d); > > +=C2=A0=C2=A0=C2=A0=C2=A0} > > 1) IMHO, the trace should be wrapped by the if ( > unlikely(tb_init_done) ) {} statement as done in sched_credit2.c. > Otherwise, we always enable the tracing which may hurt the > performance, I think. >=20 You're right. Actually, in order to follow suite from sched_rt.c, I think using trace_var() instead of __trace_var() is what we want. Then, I think it will be a good thing, at some point, to convert all these /*TRACE*/ blocks to extrapolate the tb_init_done check and make it guard the trace record marshalling, like I did a few patches ago for Credit2.... but that's another patch. This is a cut-&-paste mistake, good job noticing it. :-) > 2) Why does the cpu field here use 17 bits instead of 16 bits as used > in credit2? > This may be a typo I guess (since you are trying to align the > structure to 32 bits I guess ;-) )? > Wow... 17.. I must have been drunk when writing that! :-O > In addition, I'm wondering if uint16 is better than unsigned? I'm not > that confident if unsigned type will always have 16 bits on all types > of machines? >=20 Yeah, well, TBH, all this bitfields and packing, etc., is something I truly hate. But I think in this case unsigned is fine. 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) --=-8BNQumjm2jGGKF0nPN/9 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 iQIcBAABCAAGBQJXtYLiAAoJEBZCeImluHPu/3YQALaF2aBEp/O3tlHwJHD9MSCt 9dZLOuzYRt+ad9JkedpMk7Q336KX9eg6/gu7SReRBPYEt4638gL526tcsFSx9Qs8 PimsY09q+CTTUXk1BSq5B0ShKm9T2NpIJOPsjqf0dfvNlbuo+MVY1qP6MgDbofYZ MPTkpLwTZ0Zm11VyLMQbHMN9aOTHxkTIFfI/3GWNbtvUvSJDIhz30hkozcqLv76g of15Pi4FvIibhRqkhCJeHtuomn2twzv8zG0ZpMbJJM96dKRlWQIioUIEDG4e5qQN PQ+L9CzOTxr2eZ/ZElm8Xu1AiIfeOrda9nJIeOCpx5XAhjde6EZ18d7nSv7hGQrl 2g/3UOEmaC8I3pANdKIjW4yoolVNzB4q0f6/W8Zi1PPj+3O2jz2t/2j0emirfqkw z6ugv2yG3BB9aTsWxgi+8Ws8loEVJRtq94zkaGOqSBwE9IVGCXTN4AuXjasqlUvK IB7ZIXF4rzv4sckTO6SmGKCgwWsYszMEw/UGH0RkdNvpchtoGr0p++fhZqswbDNv p/DJhYtB0MHCY8KCBtb/y42Hv6gu1W1nfb4nRmbqA+6IasqJ5XjnJQ/qpF3e5WZT N2To1+G1nTdbxqSL/FK0VkyAMXcA+elXD7uWOQpbjEWolQ2pE8oLwwFzaKp5wttt yM3sQiKGzZSl0sNDVwu2 =6h7c -----END PGP SIGNATURE----- --=-8BNQumjm2jGGKF0nPN/9-- --===============2760735618110779276== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwczovL2xpc3RzLnhlbi5v cmcveGVuLWRldmVsCg== --===============2760735618110779276==--