Nish Aravamudan wrote:
On 10/23/05, Andrew Morton <[email protected]> wrote:
Corey Minyard <[email protected]> wrote:
We must poll for responses to commands when interrupts aren't in use.
The default poll interval is based on using a kernel timer, which
varies with HZ. For character-based interfaces like KCS and SMIC
though, that can be way too slow (>15 minutes to flash a new firmware
with KCS, >20 seconds to retrieve the sensor list).
This creates a low-priority kernel thread to poll more often. If the
state machine is idle, so is the kernel thread. But if there's an
active command, it polls quite rapidly. This decrease a firmware
flash time from 15 minutes to 1.5 minutes, and the sensor list time to
4.5 seconds, on a Dell PowerEdge x8x system.
The timer-based polling remains, to ensure some amount of
responsiveness even under high user process CPU load.
Checking for a stopped timer at rmmod now uses atomics and
del_timer_sync() to ensure safe stoppage.
...
+static int ipmi_thread(void *data)
+{
+ struct smi_info *smi_info = data;
+ unsigned long flags, last=1;
+ enum si_sm_result smi_result;
+
+ daemonize("kipmi%d", smi_info->intf_num);
+ allow_signal(SIGKILL);
+ set_user_nice(current, 19);
+ while (!atomic_read(&smi_info->stop_operation)) {
+ schedule_timeout(last);
+ spin_lock_irqsave(&(smi_info->si_lock), flags);
+ smi_result=smi_event_handler(smi_info, 0);
+ spin_unlock_irqrestore(&(smi_info->si_lock), flags);
+ if (smi_result == SI_SM_CALL_WITHOUT_DELAY)
+ last = 0;
+ else if (smi_result == SI_SM_CALL_WITH_DELAY) {
+ udelay(1);
+ last = 0;
+ }
+ else {
+ /* System is idle; go to sleep */
+ last = 1;
+ current->state = TASK_INTERRUPTIBLE;
+ }
+ }
+ smi_info->thread_pid = 0;
+ complete_and_exit(&(smi_info->exiting), 0);
+ return 0;
+}
<snip>
The first call to schedule_timeout() here will not actually sleep at all,
due to it being in state TASK_RUNNING. Is that deliberate?
Also, this thread can exit in state TASK_INTERUPTIBLE. That's not a bug
per-se, but apparently it'll spit a warning in some of the patches which
Ingo is working on. I don't know why, but I'm sure there's a good reason
;)
You beat me to this one, Andrew! :) Both issue can be avoided by using
schedule_timeout_interruptible().
Additionally, I think the last variable is simply being used to switch
between a 0 and 1 jiffy sleep (took me a while to figure that out in
GMail sadly -- any chance the variable could be renamed?). In the
current implementaion of schedule_timeout(), these will result in the
same behavior, expiring the timer at the next timer interrupt (the
next jiffy increment is the first time we'll notice we had a timer in
the past to expire). Not sure if that's the intent and perhaps just a
means to indicate what is desired (a sleep will still occur, even
though a udelay() has already in the loop, for instance), but wanted
to make sure.
I think it would be VERY nice if we could eliminate calls to sleep for NIL time. Sooner or later
these are going to bite us very badly.
In the above code, the handling of the "task state" is, well, funny. If last is 0, it is not
modified implying that:
if (last)
schedule_timeout(last);
might be a better rendering of the intended function.
--
George Anzinger [email protected]
HRT (High-res-timers): http://sourceforge.net/projects/high-res-timers/
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
