Typo in ULE in FreeBSD 8.0 -- that's not really a bug
Murty, Ravi
ravi.murty at intel.com
Wed Nov 12 08:09:44 PST 2008
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 at freebsd.org]
Sent: Tuesday, November 11, 2008 8:38 AM
To: freebsd-hackers at freebsd.org
Cc: Murty, Ravi; jeff at 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,
>
> I have been playing with ULE in 8.0 and while staring at tdq_notify noticed
an interesting (and what seems like a typo) problem.
> The intention of the function is obvious, send an IPI to notify the remote
CPU of some new piece of work. In the case where there is no IPI currently
pending on the target CPU and this thread should be preempting what's running
there, the code checks in td (passed in as a parameter) is the IDLE thread
(TDF_IDLETD). If so, it checks the state and sees if idle is RUNNING and if
so figures it will notice this new work and we don't really need to send an
expensive IPI. However, why would td (parameter) ever be the IDLE thread? It
almost seems like this check will always fail and we end up sending a hard
IPI to the target CPU which works, but may not be needed. May be we wanted to
use PCPU->curthread instead of td?
I think you are correct. Something like this might fix it:
Index: sched_ule.c
===================================================================
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;
@@ -950,10 +950,10 @@
return;
cpu = td->td_sched->ts_cpu;
pri = td->td_priority;
- cpri = pcpu_find(cpu)->pc_curthread->td_priority;
- if (!sched_shouldpreempt(pri, cpri, 1))
+ ctd = 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
--
John Baldwin
More information about the freebsd-hackers
mailing list