Messages in this thread | | | Date | Sun, 23 Oct 2005 14:12:51 -0700 | From | Nish Aravamudan <> | Subject | Re: [PATCH 9/9] ipmi: add timer thread |
| |
On 10/23/05, Andrew Morton <akpm@osdl.org> wrote: > Corey Minyard <minyard@acm.org> 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.
Thanks, Nish - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |