lkml.org 
[lkml]   [2005]   [Oct]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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/

\
 
 \ /
  Last update: 2005-10-23 23:15    [W:0.164 / U:0.044 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site