Date: Wed, 12 Nov 2008 08:09:23 -0800 From: "Murty, Ravi" <ravi.murty@intel.com> To: John Baldwin <jhb@freebsd.org>, "freebsd-hackers@freebsd.org" <freebsd-hackers@freebsd.org> Cc: "jeff@freebsd.org" <jeff@freebsd.org> Subject: RE: Typo in ULE in FreeBSD 8.0 -- that's not really a bug Message-ID: <6D5D25EA3941074EB7734E51B16687040AD518AC@orsmsx506.amr.corp.intel.com> In-Reply-To: <200811111137.55731.jhb@freebsd.org> References: <6D5D25EA3941074EB7734E51B16687040AD02A77@orsmsx506.amr.corp.intel.com> <200811111137.55731.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Yes, that's what I was thinking. Just look at what's running on the remote = CPU. Thanks Jeff and John. Ravi -----Original Message----- From: John Baldwin [mailto:jhb@freebsd.org]=20 Sent: Tuesday, November 11, 2008 8:38 AM To: freebsd-hackers@freebsd.org Cc: Murty, Ravi; jeff@freebsd.org Subject: Re: Typo in ULE in FreeBSD 8.0 -- that's not really a bug On Monday 10 November 2008 12:57:44 pm Murty, Ravi wrote: > Hello All, >=20 > I have been playing with ULE in 8.0 and while staring at tdq_notify notic= ed=20 an interesting (and what seems like a typo) problem. > The intention of the function is obvious, send an IPI to notify the remot= e=20 CPU of some new piece of work. In the case where there is no IPI currently= =20 pending on the target CPU and this thread should be preempting what's runni= ng=20 there, the code checks in td (passed in as a parameter) is the IDLE thread= =20 (TDF_IDLETD). If so, it checks the state and sees if idle is RUNNING and if= =20 so figures it will notice this new work and we don't really need to send an= =20 expensive IPI. However, why would td (parameter) ever be the IDLE thread? I= t=20 almost seems like this check will always fail and we end up sending a hard= =20 IPI to the target CPU which works, but may not be needed. May be we wanted = to=20 use PCPU->curthread instead of td? I think you are correct. Something like this might fix it: Index: sched_ule.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /usr/cvs/src/sys/kern/sched_ule.c,v retrieving revision 1.246 diff -u -r1.246 sched_ule.c --- sched_ule.c 19 Jul 2008 05:13:47 -0000 1.246 +++ sched_ule.c 11 Nov 2008 16:36:25 -0000 @@ -942,7 +942,7 @@ static void tdq_notify(struct tdq *tdq, struct thread *td) { - int cpri; + struct thread *ctd; int pri; int cpu; =20 @@ -950,10 +950,10 @@ return; cpu =3D td->td_sched->ts_cpu; pri =3D td->td_priority; - cpri =3D pcpu_find(cpu)->pc_curthread->td_priority; - if (!sched_shouldpreempt(pri, cpri, 1)) + ctd =3D pcpu_find(cpu)->pc_curthread; + if (!sched_shouldpreempt(pri, ctd->td_priority, 1)) return; - if (TD_IS_IDLETHREAD(td)) { + if (TD_IS_IDLETHREAD(ctd)) { /* * If the idle thread is still 'running' it's probably * waiting on us to release the tdq spinlock already. No --=20 John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?6D5D25EA3941074EB7734E51B16687040AD518AC>