[PATCH 7/12] IPMI: add poll delay

[PATCH 7/12] IPMI: add poll delay

[Corey Minyard]

Make sure to delay a little in the IPMI poll routine so we can pass in
a timeout time and thus time things out.

Signed-off-by: Corey Minyard <minyard@acm.org>

Index: linux-2.6.19-rc6/drivers/char/ipmi/ipmi_si_intf.c
===================================================================
--- linux-2.6.19-rc6.orig/drivers/char/ipmi/ipmi_si_intf.c
+++ linux-2.6.19-rc6/drivers/char/ipmi/ipmi_si_intf.c
@@ -807,7 +807,12 @@ static void poll(void *send_info)
 {
 	struct smi_info *smi_info = send_info;
 
-	smi_event_handler(smi_info, 0);
+	/*
+	 * Make sure there is some delay in the poll loop so we can
+	 * drive time forward and timeout things.
+	 */
+	udelay(10);
+	smi_event_handler(smi_info, 10);
 }
 
 static void request_events(void *send_info)

Re: [PATCH 7/12] IPMI: add poll delay

[Andrew Morton]

On Fri, 1 Dec 2006 22:35:20 -0600
Corey Minyard <minyard@acm.org> wrote:

> Make sure to delay a little in the IPMI poll routine so we can pass in
> a timeout time and thus time things out.
> 
> Signed-off-by: Corey Minyard <minyard@acm.org>
> 
> Index: linux-2.6.19-rc6/drivers/char/ipmi/ipmi_si_intf.c
> ===================================================================
> --- linux-2.6.19-rc6.orig/drivers/char/ipmi/ipmi_si_intf.c
> +++ linux-2.6.19-rc6/drivers/char/ipmi/ipmi_si_intf.c
> @@ -807,7 +807,12 @@ static void poll(void *send_info)
>  {
>  	struct smi_info *smi_info = send_info;
>  
> -	smi_event_handler(smi_info, 0);
> +	/*
> +	 * Make sure there is some delay in the poll loop so we can
> +	 * drive time forward and timeout things.
> +	 */
> +	udelay(10);
> +	smi_event_handler(smi_info, 10);
>  }

I don't understand what this patch is doing.  It looks fishy.  More
details, please?

Re: [PATCH 7/12] IPMI: add poll delay

[Corey Minyard]

Andrew Morton wrote:
> On Fri, 1 Dec 2006 22:35:20 -0600
> Corey Minyard <minyard@acm.org> wrote:
>
>   
>> Make sure to delay a little in the IPMI poll routine so we can pass in
>> a timeout time and thus time things out.
>>
>> Signed-off-by: Corey Minyard <minyard@acm.org>
>>
>> Index: linux-2.6.19-rc6/drivers/char/ipmi/ipmi_si_intf.c
>> ===================================================================
>> --- linux-2.6.19-rc6.orig/drivers/char/ipmi/ipmi_si_intf.c
>> +++ linux-2.6.19-rc6/drivers/char/ipmi/ipmi_si_intf.c
>> @@ -807,7 +807,12 @@ static void poll(void *send_info)
>>  {
>>  	struct smi_info *smi_info = send_info;
>>  
>> -	smi_event_handler(smi_info, 0);
>> +	/*
>> +	 * Make sure there is some delay in the poll loop so we can
>> +	 * drive time forward and timeout things.
>> +	 */
>> +	udelay(10);
>> +	smi_event_handler(smi_info, 10);
>>  }
>>     
>
> I don't understand what this patch is doing.  It looks fishy.  More
> details, please?
>   
Yeah, it does look a little fishy.  This is a poll routine that is only
called at panic
time; it is used to force things to happen in the driver without
scheduling or
timers.  The driver does this so it can set watchdog parameters and store
panic information in the event log at panic time.

Without this change, if something goes wrong in the BMC the driver will
never
time out the operation since it doesn't see time being driven forward. 
So this
makes sure the driver sees time advancing as it should.

-Corey

Re: [PATCH 7/12] IPMI: add poll delay

[Jesper Juhl]

On 05/12/06, Corey Minyard <minyard@acm.org> wrote:
> Andrew Morton wrote:
> > On Fri, 1 Dec 2006 22:35:20 -0600
> > Corey Minyard <minyard@acm.org> wrote:
> >
> >
> >> Make sure to delay a little in the IPMI poll routine so we can pass in
> >> a timeout time and thus time things out.
> >>
> >> Signed-off-by: Corey Minyard <minyard@acm.org>
> >>
> >> Index: linux-2.6.19-rc6/drivers/char/ipmi/ipmi_si_intf.c
> >> ===================================================================
> >> --- linux-2.6.19-rc6.orig/drivers/char/ipmi/ipmi_si_intf.c
> >> +++ linux-2.6.19-rc6/drivers/char/ipmi/ipmi_si_intf.c
> >> @@ -807,7 +807,12 @@ static void poll(void *send_info)
> >>  {
> >>      struct smi_info *smi_info = send_info;
> >>
> >> -    smi_event_handler(smi_info, 0);
> >> +    /*
> >> +     * Make sure there is some delay in the poll loop so we can
> >> +     * drive time forward and timeout things.
> >> +     */
> >> +    udelay(10);
> >> +    smi_event_handler(smi_info, 10);
> >>  }
> >>
> >
> > I don't understand what this patch is doing.  It looks fishy.  More
> > details, please?
> >
> Yeah, it does look a little fishy.  This is a poll routine that is only
> called at panic
> time; it is used to force things to happen in the driver without
> scheduling or
> timers.  The driver does this so it can set watchdog parameters and store
> panic information in the event log at panic time.
>
> Without this change, if something goes wrong in the BMC the driver will
> never
> time out the operation since it doesn't see time being driven forward.
> So this
> makes sure the driver sees time advancing as it should.
>

Hmm, I wonder if this could explain why some of my IBM servers become
unreachable via IPMI after a kernel crash.

-- 
Jesper Juhl <jesper.juhl@gmail.com>
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please      http://www.expita.com/nomime.html