Re: [PATCH 1/1] ipmi: setting mod_timer for read_event_msg buffer cmd
> > No, I did not see final else in ipmi_thread ever getting hit. > > I am looking at ipmi_thread_busy_wait(), > by default the below condition never gets set unitl I explicetly set > "kipmid_max_busy_us" to some value. > if (smi_info->intf_num < num_max_busy_us) > > This means by default "max_busy_us" is always 0. Hence Ill basically end up > only doing this > if (max_busy_us == 0 || smi_result != SI_SM_CALL_WITH_DELAY) > ipmi_si_set_not_busy(busy_until); > > That probably explains why I never hit the else condition in ipmi_thread. > > I see ipmi_start_timer_if_necessary() getting called from ipmi_thread() after > setting "kipmid_max_busy_us" > Ill set "kipmid_max_busy_us=300" and run the stress. I am hoping that we will > not see the issue in this case. > I don't see the issue after this change. (I am going to try again just to be sure) I am trying to figure out how to hit ipmi_start_timer_if_necessary() in default mode...! Thanks, G > > Thanks, > G > > > >> Thanks, >> >> -corey >> >>> >>> >>> Thanks, >>> G >>> >>> -Original Message- >>> From: Corey Minyard [mailto:tcminy...@gmail.com] On Behalf Of Corey Minyard >>> Sent: Tuesday, December 03, 2013 2:34 AM >>> To: Gowda, Srinivas G >>> Cc: tcminy...@gmail.com; linux-kernel@vger.kernel.org; openi...@mvista.com >>> Subject: Re: [PATCH 1/1] ipmi: setting mod_timer for read_event_msg buffer >>> cmd On 12/02/2013 08:49 AM, srinivas_g_go...@dell.com wrote: >>>> Thanks for the patch Corey. >>>> I am afraid that the system does not have interrupts enabled. It uses >>>> polling mode. >>>> >>>> When the error is seen, I know for a fact that in function >>>> ipmi_thread() smi_result is SI_SM_CALL_WITH_DELAY, I have some logs where >>>> in busy_wait always reads as 1. Not sure if it was ever set to 0. (ill >>>> check this again). >>>> Ill anyway run the test using the patch that you have shared. >>>> >>>> b/w would it harm if we were to do to something like this ? >>> Unfortunately, that would start the timer unnecessarily. You don't want to >>> start timers unnecessarily in the kernel or the power management police >>> will come after you. >>> The patch I sent did have this call in the non-idle portion of the kernel >>> thread and that should have done the same thing. Plus, if you are using >>> the kernel thread, it's going to run periodically and should kick things >>> off again if they get stuck. I'm suspicious now that something else is >>> going on. >>> -corey >>>> Signed-off-by: Srinivas Gowda >>>> --- >>>> drivers/char/ipmi/ipmi_si_intf.c | 1 + >>>> 1 file changed, 1 insertion(+) >>>> >>>> diff --git a/drivers/char/ipmi/ipmi_si_intf.c >>>> b/drivers/char/ipmi/ipmi_si_intf.c >>>> index 15e4a60..e23484f 100644 >>>> --- a/drivers/char/ipmi/ipmi_si_intf.c >>>> +++ b/drivers/char/ipmi/ipmi_si_intf.c >>>> @@ -1008,6 +1008,7 @@ static int ipmi_thread(void *data) >>>>spin_unlock_irqrestore(&(smi_info->si_lock), flags); >>>>busy_wait = ipmi_thread_busy_wait(smi_result, smi_info, >>>> _until); >>>> + ipmi_start_timer_if_necessary(smi_info); >>>>if (smi_result == SI_SM_CALL_WITHOUT_DELAY) >>>>; /* do nothing */ >>>>else if (smi_result == SI_SM_CALL_WITH_DELAY && >>>> busy_wait) >> > > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] ipmi: setting mod_timer for read_event_msg buffer cmd
No, I did not see final else in ipmi_thread ever getting hit. I am looking at ipmi_thread_busy_wait(), by default the below condition never gets set unitl I explicetly set kipmid_max_busy_us to some value. if (smi_info-intf_num num_max_busy_us) This means by default max_busy_us is always 0. Hence Ill basically end up only doing this if (max_busy_us == 0 || smi_result != SI_SM_CALL_WITH_DELAY) ipmi_si_set_not_busy(busy_until); That probably explains why I never hit the else condition in ipmi_thread. I see ipmi_start_timer_if_necessary() getting called from ipmi_thread() after setting kipmid_max_busy_us Ill set kipmid_max_busy_us=300 and run the stress. I am hoping that we will not see the issue in this case. I don't see the issue after this change. (I am going to try again just to be sure) I am trying to figure out how to hit ipmi_start_timer_if_necessary() in default mode...! Thanks, G Thanks, G Thanks, -corey Thanks, G -Original Message- From: Corey Minyard [mailto:tcminy...@gmail.com] On Behalf Of Corey Minyard Sent: Tuesday, December 03, 2013 2:34 AM To: Gowda, Srinivas G Cc: tcminy...@gmail.com; linux-kernel@vger.kernel.org; openi...@mvista.com Subject: Re: [PATCH 1/1] ipmi: setting mod_timer for read_event_msg buffer cmd On 12/02/2013 08:49 AM, srinivas_g_go...@dell.com wrote: Thanks for the patch Corey. I am afraid that the system does not have interrupts enabled. It uses polling mode. When the error is seen, I know for a fact that in function ipmi_thread() smi_result is SI_SM_CALL_WITH_DELAY, I have some logs where in busy_wait always reads as 1. Not sure if it was ever set to 0. (ill check this again). Ill anyway run the test using the patch that you have shared. b/w would it harm if we were to do to something like this ? Unfortunately, that would start the timer unnecessarily. You don't want to start timers unnecessarily in the kernel or the power management police will come after you. The patch I sent did have this call in the non-idle portion of the kernel thread and that should have done the same thing. Plus, if you are using the kernel thread, it's going to run periodically and should kick things off again if they get stuck. I'm suspicious now that something else is going on. -corey Signed-off-by: Srinivas Gowda srinivas_g_go...@dell.com --- drivers/char/ipmi/ipmi_si_intf.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c index 15e4a60..e23484f 100644 --- a/drivers/char/ipmi/ipmi_si_intf.c +++ b/drivers/char/ipmi/ipmi_si_intf.c @@ -1008,6 +1008,7 @@ static int ipmi_thread(void *data) spin_unlock_irqrestore((smi_info-si_lock), flags); busy_wait = ipmi_thread_busy_wait(smi_result, smi_info, busy_until); + ipmi_start_timer_if_necessary(smi_info); if (smi_result == SI_SM_CALL_WITHOUT_DELAY) ; /* do nothing */ else if (smi_result == SI_SM_CALL_WITH_DELAY busy_wait) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] ipmi: setting mod_timer for read_event_msg buffer cmd
On 12/03/2013 10:40 PM, Corey Minyard wrote: > On 12/03/2013 12:18 AM, srinivas_g_go...@dell.com wrote: >> Dell - Internal Use - Confidential >> >> Hi Corey, >>> Unfortunately, that would start the timer unnecessarily. You don't want to >>> start timers unnecessarily in the kernel or the power management police >>> will come after you. >>I still see the issue after applying the patch. >>I have one question, for commands such as IPMI_READ_EVENT_MSG_BUFFER_CMD >> in smi_event_handler() that are invoked by the driver itself where do you >> expect the timer to be set ?? > > The timer should be done by the calling function. Since the timer is > actually used to time out complete operations, you don't want to reset > it from interrupts or the kthread. The patch I added would start the > timer if it wasn't running and the event returned something non-idle. > > So what should happen is smi_event_handler() starts the event message > buffer read, goes back to the top of smi_event_handler() and handles the > new message. It should return something besides idle in that case. And > thus the timer should be started. > >>> The patch I sent did have this call in the non-idle portion of the kernel >>> thread and that should have done the same thing. Plus, if you are using >>> the kernel thread, it's going to run periodically and should kick things >>> off again if they get stuck. I'm suspicious now that something else is >>> going on. >> ipmi_thread() is getting invoked when the issue is seen, but I keep >> hitting this condition >> else if (smi_result == SI_SM_CALL_WITH_DELAY && busy_wait) >> Since the timer is not armed for IPMI_READ_EVENT_MSG_BUFFER_CMD, >> smi_event_handler calls with time value 0, which doesn't help when OBF is >> stuck. >> We keep running around this loop since we never hit the kcs error states. > > Maybe the timer start function needs to be called from that else clause, > but it didn't seem so to me. It wouldn't hurt, I suppose. But the > event handler should eventually return something non-idle and the timer > start function should get called. Is the ipmi thread just stuck using > 100% CPU? Yes > Does the final else clause ever get hit? Maybe the bug is in > ipmi_thread_busy_wait(). > No, I did not see final else in ipmi_thread ever getting hit. I am looking at ipmi_thread_busy_wait(), by default the below condition never gets set unitl I explicetly set "kipmid_max_busy_us" to some value. if (smi_info->intf_num < num_max_busy_us) This means by default "max_busy_us" is always 0. Hence Ill basically end up only doing this if (max_busy_us == 0 || smi_result != SI_SM_CALL_WITH_DELAY) ipmi_si_set_not_busy(busy_until); That probably explains why I never hit the else condition in ipmi_thread. I see ipmi_start_timer_if_necessary() getting called from ipmi_thread() after setting "kipmid_max_busy_us" Ill set "kipmid_max_busy_us=300" and run the stress. I am hoping that we will not see the issue in this case. Thanks, G > Thanks, > > -corey > >> >> >> Thanks, >> G >> >> -----Original Message- >> From: Corey Minyard [mailto:tcminy...@gmail.com] On Behalf Of Corey Minyard >> Sent: Tuesday, December 03, 2013 2:34 AM >> To: Gowda, Srinivas G >> Cc: tcminy...@gmail.com; linux-kernel@vger.kernel.org; openi...@mvista.com >> Subject: Re: [PATCH 1/1] ipmi: setting mod_timer for read_event_msg buffer >> cmd On 12/02/2013 08:49 AM, srinivas_g_go...@dell.com wrote: >>> Thanks for the patch Corey. >>> I am afraid that the system does not have interrupts enabled. It uses >>> polling mode. >>> >>> When the error is seen, I know for a fact that in function >>> ipmi_thread() smi_result is SI_SM_CALL_WITH_DELAY, I have some logs where >>> in busy_wait always reads as 1. Not sure if it was ever set to 0. (ill >>> check this again). >>> Ill anyway run the test using the patch that you have shared. >>> >>> b/w would it harm if we were to do to something like this ? >> Unfortunately, that would start the timer unnecessarily. You don't want to >> start timers unnecessarily in the kernel or the power management police will >> come after you. >> The patch I sent did have this call in the non-idle portion of the kernel >> thread and that should have done the same thing. Plus, if you are using the >> kernel thread, it's going to run periodically and should kick things off >> again if they get stuck. I'm suspicious now that something
Re: [PATCH 1/1] ipmi: setting mod_timer for read_event_msg buffer cmd
On 12/03/2013 10:40 PM, Corey Minyard wrote: On 12/03/2013 12:18 AM, srinivas_g_go...@dell.com wrote: Dell - Internal Use - Confidential Hi Corey, Unfortunately, that would start the timer unnecessarily. You don't want to start timers unnecessarily in the kernel or the power management police will come after you. I still see the issue after applying the patch. I have one question, for commands such as IPMI_READ_EVENT_MSG_BUFFER_CMD in smi_event_handler() that are invoked by the driver itself where do you expect the timer to be set ?? The timer should be done by the calling function. Since the timer is actually used to time out complete operations, you don't want to reset it from interrupts or the kthread. The patch I added would start the timer if it wasn't running and the event returned something non-idle. So what should happen is smi_event_handler() starts the event message buffer read, goes back to the top of smi_event_handler() and handles the new message. It should return something besides idle in that case. And thus the timer should be started. The patch I sent did have this call in the non-idle portion of the kernel thread and that should have done the same thing. Plus, if you are using the kernel thread, it's going to run periodically and should kick things off again if they get stuck. I'm suspicious now that something else is going on. ipmi_thread() is getting invoked when the issue is seen, but I keep hitting this condition else if (smi_result == SI_SM_CALL_WITH_DELAY busy_wait) Since the timer is not armed for IPMI_READ_EVENT_MSG_BUFFER_CMD, smi_event_handler calls with time value 0, which doesn't help when OBF is stuck. We keep running around this loop since we never hit the kcs error states. Maybe the timer start function needs to be called from that else clause, but it didn't seem so to me. It wouldn't hurt, I suppose. But the event handler should eventually return something non-idle and the timer start function should get called. Is the ipmi thread just stuck using 100% CPU? Yes Does the final else clause ever get hit? Maybe the bug is in ipmi_thread_busy_wait(). No, I did not see final else in ipmi_thread ever getting hit. I am looking at ipmi_thread_busy_wait(), by default the below condition never gets set unitl I explicetly set kipmid_max_busy_us to some value. if (smi_info-intf_num num_max_busy_us) This means by default max_busy_us is always 0. Hence Ill basically end up only doing this if (max_busy_us == 0 || smi_result != SI_SM_CALL_WITH_DELAY) ipmi_si_set_not_busy(busy_until); That probably explains why I never hit the else condition in ipmi_thread. I see ipmi_start_timer_if_necessary() getting called from ipmi_thread() after setting kipmid_max_busy_us Ill set kipmid_max_busy_us=300 and run the stress. I am hoping that we will not see the issue in this case. Thanks, G Thanks, -corey Thanks, G -Original Message- From: Corey Minyard [mailto:tcminy...@gmail.com] On Behalf Of Corey Minyard Sent: Tuesday, December 03, 2013 2:34 AM To: Gowda, Srinivas G Cc: tcminy...@gmail.com; linux-kernel@vger.kernel.org; openi...@mvista.com Subject: Re: [PATCH 1/1] ipmi: setting mod_timer for read_event_msg buffer cmd On 12/02/2013 08:49 AM, srinivas_g_go...@dell.com wrote: Thanks for the patch Corey. I am afraid that the system does not have interrupts enabled. It uses polling mode. When the error is seen, I know for a fact that in function ipmi_thread() smi_result is SI_SM_CALL_WITH_DELAY, I have some logs where in busy_wait always reads as 1. Not sure if it was ever set to 0. (ill check this again). Ill anyway run the test using the patch that you have shared. b/w would it harm if we were to do to something like this ? Unfortunately, that would start the timer unnecessarily. You don't want to start timers unnecessarily in the kernel or the power management police will come after you. The patch I sent did have this call in the non-idle portion of the kernel thread and that should have done the same thing. Plus, if you are using the kernel thread, it's going to run periodically and should kick things off again if they get stuck. I'm suspicious now that something else is going on. -corey Signed-off-by: Srinivas Gowda srinivas_g_go...@dell.com --- drivers/char/ipmi/ipmi_si_intf.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c index 15e4a60..e23484f 100644 --- a/drivers/char/ipmi/ipmi_si_intf.c +++ b/drivers/char/ipmi/ipmi_si_intf.c @@ -1008,6 +1008,7 @@ static int ipmi_thread(void *data) spin_unlock_irqrestore((smi_info-si_lock), flags); busy_wait = ipmi_thread_busy_wait(smi_result, smi_info, busy_until); + ipmi_start_timer_if_necessary
Re: [PATCH 1/1] ipmi: setting mod_timer for read_event_msg buffer cmd
On 12/03/2013 12:18 AM, srinivas_g_go...@dell.com wrote: > Dell - Internal Use - Confidential > > Hi Corey, >> Unfortunately, that would start the timer unnecessarily. You don't want to >> start timers unnecessarily in the kernel or the power management police will >> come after you. >I still see the issue after applying the patch. >I have one question, for commands such as IPMI_READ_EVENT_MSG_BUFFER_CMD > in smi_event_handler() that are invoked by the driver itself where do you > expect the timer to be set ?? The timer should be done by the calling function. Since the timer is actually used to time out complete operations, you don't want to reset it from interrupts or the kthread. The patch I added would start the timer if it wasn't running and the event returned something non-idle. So what should happen is smi_event_handler() starts the event message buffer read, goes back to the top of smi_event_handler() and handles the new message. It should return something besides idle in that case. And thus the timer should be started. >> The patch I sent did have this call in the non-idle portion of the kernel >> thread and that should have done the same thing. Plus, if you are using the >> kernel thread, it's going to run periodically and should kick things off >> again if they get stuck. I'm suspicious now that something else is going on. > ipmi_thread() is getting invoked when the issue is seen, but I keep hitting > this condition > else if (smi_result == SI_SM_CALL_WITH_DELAY && busy_wait) > Since the timer is not armed for IPMI_READ_EVENT_MSG_BUFFER_CMD, > smi_event_handler calls with time value 0, which doesn't help when OBF is > stuck. > We keep running around this loop since we never hit the kcs error states. Maybe the timer start function needs to be called from that else clause, but it didn't seem so to me. It wouldn't hurt, I suppose. But the event handler should eventually return something non-idle and the timer start function should get called. Is the ipmi thread just stuck using 100% CPU? Does the final else clause ever get hit? Maybe the bug is in ipmi_thread_busy_wait(). Thanks, -corey > > > Thanks, > G > > -Original Message- > From: Corey Minyard [mailto:tcminy...@gmail.com] On Behalf Of Corey Minyard > Sent: Tuesday, December 03, 2013 2:34 AM > To: Gowda, Srinivas G > Cc: tcminy...@gmail.com; linux-kernel@vger.kernel.org; openi...@mvista.com > Subject: Re: [PATCH 1/1] ipmi: setting mod_timer for read_event_msg buffer > cmd On 12/02/2013 08:49 AM, srinivas_g_go...@dell.com wrote: >> Thanks for the patch Corey. >> I am afraid that the system does not have interrupts enabled. It uses >> polling mode. >> >> When the error is seen, I know for a fact that in function >> ipmi_thread() smi_result is SI_SM_CALL_WITH_DELAY, I have some logs where in >> busy_wait always reads as 1. Not sure if it was ever set to 0. (ill check >> this again). >> Ill anyway run the test using the patch that you have shared. >> >> b/w would it harm if we were to do to something like this ? > Unfortunately, that would start the timer unnecessarily. You don't want to > start timers unnecessarily in the kernel or the power management police will > come after you. > The patch I sent did have this call in the non-idle portion of the kernel > thread and that should have done the same thing. Plus, if you are using the > kernel thread, it's going to run periodically and should kick things off > again if they get stuck. I'm suspicious now that something else is going on. > -corey >> Signed-off-by: Srinivas Gowda >> --- >> drivers/char/ipmi/ipmi_si_intf.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/char/ipmi/ipmi_si_intf.c >> b/drivers/char/ipmi/ipmi_si_intf.c >> index 15e4a60..e23484f 100644 >> --- a/drivers/char/ipmi/ipmi_si_intf.c >> +++ b/drivers/char/ipmi/ipmi_si_intf.c >> @@ -1008,6 +1008,7 @@ static int ipmi_thread(void *data) >>spin_unlock_irqrestore(&(smi_info->si_lock), flags); >>busy_wait = ipmi_thread_busy_wait(smi_result, smi_info, >> _until); >> + ipmi_start_timer_if_necessary(smi_info); >>if (smi_result == SI_SM_CALL_WITHOUT_DELAY) >>; /* do nothing */ >>else if (smi_result == SI_SM_CALL_WITH_DELAY && >> busy_wait) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] ipmi: setting mod_timer for read_event_msg buffer cmd
On 12/03/2013 12:18 AM, srinivas_g_go...@dell.com wrote: Dell - Internal Use - Confidential Hi Corey, Unfortunately, that would start the timer unnecessarily. You don't want to start timers unnecessarily in the kernel or the power management police will come after you. I still see the issue after applying the patch. I have one question, for commands such as IPMI_READ_EVENT_MSG_BUFFER_CMD in smi_event_handler() that are invoked by the driver itself where do you expect the timer to be set ?? The timer should be done by the calling function. Since the timer is actually used to time out complete operations, you don't want to reset it from interrupts or the kthread. The patch I added would start the timer if it wasn't running and the event returned something non-idle. So what should happen is smi_event_handler() starts the event message buffer read, goes back to the top of smi_event_handler() and handles the new message. It should return something besides idle in that case. And thus the timer should be started. The patch I sent did have this call in the non-idle portion of the kernel thread and that should have done the same thing. Plus, if you are using the kernel thread, it's going to run periodically and should kick things off again if they get stuck. I'm suspicious now that something else is going on. ipmi_thread() is getting invoked when the issue is seen, but I keep hitting this condition else if (smi_result == SI_SM_CALL_WITH_DELAY busy_wait) Since the timer is not armed for IPMI_READ_EVENT_MSG_BUFFER_CMD, smi_event_handler calls with time value 0, which doesn't help when OBF is stuck. We keep running around this loop since we never hit the kcs error states. Maybe the timer start function needs to be called from that else clause, but it didn't seem so to me. It wouldn't hurt, I suppose. But the event handler should eventually return something non-idle and the timer start function should get called. Is the ipmi thread just stuck using 100% CPU? Does the final else clause ever get hit? Maybe the bug is in ipmi_thread_busy_wait(). Thanks, -corey Thanks, G -Original Message- From: Corey Minyard [mailto:tcminy...@gmail.com] On Behalf Of Corey Minyard Sent: Tuesday, December 03, 2013 2:34 AM To: Gowda, Srinivas G Cc: tcminy...@gmail.com; linux-kernel@vger.kernel.org; openi...@mvista.com Subject: Re: [PATCH 1/1] ipmi: setting mod_timer for read_event_msg buffer cmd On 12/02/2013 08:49 AM, srinivas_g_go...@dell.com wrote: Thanks for the patch Corey. I am afraid that the system does not have interrupts enabled. It uses polling mode. When the error is seen, I know for a fact that in function ipmi_thread() smi_result is SI_SM_CALL_WITH_DELAY, I have some logs where in busy_wait always reads as 1. Not sure if it was ever set to 0. (ill check this again). Ill anyway run the test using the patch that you have shared. b/w would it harm if we were to do to something like this ? Unfortunately, that would start the timer unnecessarily. You don't want to start timers unnecessarily in the kernel or the power management police will come after you. The patch I sent did have this call in the non-idle portion of the kernel thread and that should have done the same thing. Plus, if you are using the kernel thread, it's going to run periodically and should kick things off again if they get stuck. I'm suspicious now that something else is going on. -corey Signed-off-by: Srinivas Gowda srinivas_g_go...@dell.com --- drivers/char/ipmi/ipmi_si_intf.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c index 15e4a60..e23484f 100644 --- a/drivers/char/ipmi/ipmi_si_intf.c +++ b/drivers/char/ipmi/ipmi_si_intf.c @@ -1008,6 +1008,7 @@ static int ipmi_thread(void *data) spin_unlock_irqrestore((smi_info-si_lock), flags); busy_wait = ipmi_thread_busy_wait(smi_result, smi_info, busy_until); + ipmi_start_timer_if_necessary(smi_info); if (smi_result == SI_SM_CALL_WITHOUT_DELAY) ; /* do nothing */ else if (smi_result == SI_SM_CALL_WITH_DELAY busy_wait) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/1] ipmi: setting mod_timer for read_event_msg buffer cmd
Dell - Internal Use - Confidential Hi Corey, > Unfortunately, that would start the timer unnecessarily. You don't want to > start timers unnecessarily in the kernel or the power management police will > come after you. I still see the issue after applying the patch. I have one question, for commands such as IPMI_READ_EVENT_MSG_BUFFER_CMD in smi_event_handler() that are invoked by the driver itself where do you expect the timer to be set ?? > The patch I sent did have this call in the non-idle portion of the kernel > thread and that should have done the same thing. Plus, if you are using the > kernel thread, it's going to run periodically and should kick things off > again if they get stuck. I'm suspicious now that something else is going on. ipmi_thread() is getting invoked when the issue is seen, but I keep hitting this condition else if (smi_result == SI_SM_CALL_WITH_DELAY && busy_wait) Since the timer is not armed for IPMI_READ_EVENT_MSG_BUFFER_CMD, smi_event_handler calls with time value 0, which doesn't help when OBF is stuck. We keep running around this loop since we never hit the kcs error states. Thanks, G -Original Message- From: Corey Minyard [mailto:tcminy...@gmail.com] On Behalf Of Corey Minyard Sent: Tuesday, December 03, 2013 2:34 AM To: Gowda, Srinivas G Cc: tcminy...@gmail.com; linux-kernel@vger.kernel.org; openi...@mvista.com Subject: Re: [PATCH 1/1] ipmi: setting mod_timer for read_event_msg buffer cmd On 12/02/2013 08:49 AM, srinivas_g_go...@dell.com wrote: > Thanks for the patch Corey. > I am afraid that the system does not have interrupts enabled. It uses polling > mode. > > When the error is seen, I know for a fact that in function > ipmi_thread() smi_result is SI_SM_CALL_WITH_DELAY, I have some logs where in > busy_wait always reads as 1. Not sure if it was ever set to 0. (ill check > this again). > Ill anyway run the test using the patch that you have shared. > > b/w would it harm if we were to do to something like this ? Unfortunately, that would start the timer unnecessarily. You don't want to start timers unnecessarily in the kernel or the power management police will come after you. The patch I sent did have this call in the non-idle portion of the kernel thread and that should have done the same thing. Plus, if you are using the kernel thread, it's going to run periodically and should kick things off again if they get stuck. I'm suspicious now that something else is going on. -corey > > Signed-off-by: Srinivas Gowda > --- > drivers/char/ipmi/ipmi_si_intf.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/char/ipmi/ipmi_si_intf.c > b/drivers/char/ipmi/ipmi_si_intf.c > index 15e4a60..e23484f 100644 > --- a/drivers/char/ipmi/ipmi_si_intf.c > +++ b/drivers/char/ipmi/ipmi_si_intf.c > @@ -1008,6 +1008,7 @@ static int ipmi_thread(void *data) > spin_unlock_irqrestore(&(smi_info->si_lock), flags); > busy_wait = ipmi_thread_busy_wait(smi_result, smi_info, > _until); > + ipmi_start_timer_if_necessary(smi_info); > if (smi_result == SI_SM_CALL_WITHOUT_DELAY) > ; /* do nothing */ > else if (smi_result == SI_SM_CALL_WITH_DELAY && >busy_wait) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] ipmi: setting mod_timer for read_event_msg buffer cmd
On 12/02/2013 08:49 AM, srinivas_g_go...@dell.com wrote: > Thanks for the patch Corey. > I am afraid that the system does not have interrupts enabled. It uses polling > mode. > > When the error is seen, I know for a fact that in function ipmi_thread() > smi_result is SI_SM_CALL_WITH_DELAY, > I have some logs where in busy_wait always reads as 1. Not sure if it was > ever set to 0. (ill check this again). > Ill anyway run the test using the patch that you have shared. > > b/w would it harm if we were to do to something like this ? Unfortunately, that would start the timer unnecessarily. You don't want to start timers unnecessarily in the kernel or the power management police will come after you. The patch I sent did have this call in the non-idle portion of the kernel thread and that should have done the same thing. Plus, if you are using the kernel thread, it's going to run periodically and should kick things off again if they get stuck. I'm suspicious now that something else is going on. -corey > > Signed-off-by: Srinivas Gowda > --- > drivers/char/ipmi/ipmi_si_intf.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/char/ipmi/ipmi_si_intf.c > b/drivers/char/ipmi/ipmi_si_intf.c > index 15e4a60..e23484f 100644 > --- a/drivers/char/ipmi/ipmi_si_intf.c > +++ b/drivers/char/ipmi/ipmi_si_intf.c > @@ -1008,6 +1008,7 @@ static int ipmi_thread(void *data) > spin_unlock_irqrestore(&(smi_info->si_lock), flags); > busy_wait = ipmi_thread_busy_wait(smi_result, smi_info, > _until); > + ipmi_start_timer_if_necessary(smi_info); > if (smi_result == SI_SM_CALL_WITHOUT_DELAY) > ; /* do nothing */ > else if (smi_result == SI_SM_CALL_WITH_DELAY && busy_wait) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] ipmi: setting mod_timer for read_event_msg buffer cmd
Thanks for the patch Corey. I am afraid that the system does not have interrupts enabled. It uses polling mode. When the error is seen, I know for a fact that in function ipmi_thread() smi_result is SI_SM_CALL_WITH_DELAY, I have some logs where in busy_wait always reads as 1. Not sure if it was ever set to 0. (ill check this again). Ill anyway run the test using the patch that you have shared. b/w would it harm if we were to do to something like this ? Signed-off-by: Srinivas Gowda --- drivers/char/ipmi/ipmi_si_intf.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c index 15e4a60..e23484f 100644 --- a/drivers/char/ipmi/ipmi_si_intf.c +++ b/drivers/char/ipmi/ipmi_si_intf.c @@ -1008,6 +1008,7 @@ static int ipmi_thread(void *data) spin_unlock_irqrestore(&(smi_info->si_lock), flags); busy_wait = ipmi_thread_busy_wait(smi_result, smi_info, _until); + ipmi_start_timer_if_necessary(smi_info); if (smi_result == SI_SM_CALL_WITHOUT_DELAY) ; /* do nothing */ else if (smi_result == SI_SM_CALL_WITH_DELAY && busy_wait) -- 1.8.1.2 Thanks, G On 11/30/2013 05:49 AM, Corey Minyard wrote: > On 11/27/2013 04:34 AM, srinivas_g_go...@dell.com wrote: >> >> *Dell - Internal Use - Confidential * >> >> I hit a bug during one of our stress tests, Here is the issue that I >> am looking at. >> >> We have IPMI_READ_EVENT_MSG_BUFFER_CMD getting invoked from >> smi_event_handler. >> >> In case we hit error scenario, say "OBF not ready in time" we do not >> have smi_timeout driving the interface. >> >> Seems like the timer is not armed when we invoke >> IPMI_READ_EVENT_MSG_BUFFER_CMD from smi_event_handler. >> >> For the proposed patch I checked the return value of mod_timer just >> before smi_info->handlers->start_transaction, that returns 0 !!! >> >> gWithout smi_timeout handler getting called periodically, if the BMC >> fails to set OBF flag during the msg transaction of >> IPMI_READ_EVENT_MSG_BUFFER_CMD, >> >> the driver just keeps looping until the flag is set. Ideally we would >> want BMC to set the flag, but in case it doesn’t we do not want the >> driver to loop indefinitely rather hit KCS_ERROR states. >> >> To summarize, we do not have timer set to invoke smi_timeout() when we >> call IPMI_READ_EVENT_MSG_BUFFER_CMD from smi_event_handler. >> >> Do you feel there is a better way to fix it or a bug elsewhere…! >> > > Ok, I think I know what is happening, and I think I have a fix. I'm > betting that you have interrupts on this, and > I found a situation where if an interrupt came in at a certain time, it > wouldn't start the timer. The attached patch should fix the problem. > > Can you try this out? > > Thanks for the detailed description. > > -corey -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] ipmi: setting mod_timer for read_event_msg buffer cmd
Thanks for the patch Corey. I am afraid that the system does not have interrupts enabled. It uses polling mode. When the error is seen, I know for a fact that in function ipmi_thread() smi_result is SI_SM_CALL_WITH_DELAY, I have some logs where in busy_wait always reads as 1. Not sure if it was ever set to 0. (ill check this again). Ill anyway run the test using the patch that you have shared. b/w would it harm if we were to do to something like this ? Signed-off-by: Srinivas Gowda srinivas_g_go...@dell.com --- drivers/char/ipmi/ipmi_si_intf.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c index 15e4a60..e23484f 100644 --- a/drivers/char/ipmi/ipmi_si_intf.c +++ b/drivers/char/ipmi/ipmi_si_intf.c @@ -1008,6 +1008,7 @@ static int ipmi_thread(void *data) spin_unlock_irqrestore((smi_info-si_lock), flags); busy_wait = ipmi_thread_busy_wait(smi_result, smi_info, busy_until); + ipmi_start_timer_if_necessary(smi_info); if (smi_result == SI_SM_CALL_WITHOUT_DELAY) ; /* do nothing */ else if (smi_result == SI_SM_CALL_WITH_DELAY busy_wait) -- 1.8.1.2 Thanks, G On 11/30/2013 05:49 AM, Corey Minyard wrote: On 11/27/2013 04:34 AM, srinivas_g_go...@dell.com wrote: *Dell - Internal Use - Confidential * I hit a bug during one of our stress tests, Here is the issue that I am looking at. We have IPMI_READ_EVENT_MSG_BUFFER_CMD getting invoked from smi_event_handler. In case we hit error scenario, say OBF not ready in time we do not have smi_timeout driving the interface. Seems like the timer is not armed when we invoke IPMI_READ_EVENT_MSG_BUFFER_CMD from smi_event_handler. For the proposed patch I checked the return value of mod_timer just before smi_info-handlers-start_transaction, that returns 0 !!! gWithout smi_timeout handler getting called periodically, if the BMC fails to set OBF flag during the msg transaction of IPMI_READ_EVENT_MSG_BUFFER_CMD, the driver just keeps looping until the flag is set. Ideally we would want BMC to set the flag, but in case it doesn’t we do not want the driver to loop indefinitely rather hit KCS_ERROR states. To summarize, we do not have timer set to invoke smi_timeout() when we call IPMI_READ_EVENT_MSG_BUFFER_CMD from smi_event_handler. Do you feel there is a better way to fix it or a bug elsewhere…! Ok, I think I know what is happening, and I think I have a fix. I'm betting that you have interrupts on this, and I found a situation where if an interrupt came in at a certain time, it wouldn't start the timer. The attached patch should fix the problem. Can you try this out? Thanks for the detailed description. -corey -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] ipmi: setting mod_timer for read_event_msg buffer cmd
On 12/02/2013 08:49 AM, srinivas_g_go...@dell.com wrote: Thanks for the patch Corey. I am afraid that the system does not have interrupts enabled. It uses polling mode. When the error is seen, I know for a fact that in function ipmi_thread() smi_result is SI_SM_CALL_WITH_DELAY, I have some logs where in busy_wait always reads as 1. Not sure if it was ever set to 0. (ill check this again). Ill anyway run the test using the patch that you have shared. b/w would it harm if we were to do to something like this ? Unfortunately, that would start the timer unnecessarily. You don't want to start timers unnecessarily in the kernel or the power management police will come after you. The patch I sent did have this call in the non-idle portion of the kernel thread and that should have done the same thing. Plus, if you are using the kernel thread, it's going to run periodically and should kick things off again if they get stuck. I'm suspicious now that something else is going on. -corey Signed-off-by: Srinivas Gowda srinivas_g_go...@dell.com --- drivers/char/ipmi/ipmi_si_intf.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c index 15e4a60..e23484f 100644 --- a/drivers/char/ipmi/ipmi_si_intf.c +++ b/drivers/char/ipmi/ipmi_si_intf.c @@ -1008,6 +1008,7 @@ static int ipmi_thread(void *data) spin_unlock_irqrestore((smi_info-si_lock), flags); busy_wait = ipmi_thread_busy_wait(smi_result, smi_info, busy_until); + ipmi_start_timer_if_necessary(smi_info); if (smi_result == SI_SM_CALL_WITHOUT_DELAY) ; /* do nothing */ else if (smi_result == SI_SM_CALL_WITH_DELAY busy_wait) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/1] ipmi: setting mod_timer for read_event_msg buffer cmd
Dell - Internal Use - Confidential Hi Corey, Unfortunately, that would start the timer unnecessarily. You don't want to start timers unnecessarily in the kernel or the power management police will come after you. I still see the issue after applying the patch. I have one question, for commands such as IPMI_READ_EVENT_MSG_BUFFER_CMD in smi_event_handler() that are invoked by the driver itself where do you expect the timer to be set ?? The patch I sent did have this call in the non-idle portion of the kernel thread and that should have done the same thing. Plus, if you are using the kernel thread, it's going to run periodically and should kick things off again if they get stuck. I'm suspicious now that something else is going on. ipmi_thread() is getting invoked when the issue is seen, but I keep hitting this condition else if (smi_result == SI_SM_CALL_WITH_DELAY busy_wait) Since the timer is not armed for IPMI_READ_EVENT_MSG_BUFFER_CMD, smi_event_handler calls with time value 0, which doesn't help when OBF is stuck. We keep running around this loop since we never hit the kcs error states. Thanks, G -Original Message- From: Corey Minyard [mailto:tcminy...@gmail.com] On Behalf Of Corey Minyard Sent: Tuesday, December 03, 2013 2:34 AM To: Gowda, Srinivas G Cc: tcminy...@gmail.com; linux-kernel@vger.kernel.org; openi...@mvista.com Subject: Re: [PATCH 1/1] ipmi: setting mod_timer for read_event_msg buffer cmd On 12/02/2013 08:49 AM, srinivas_g_go...@dell.com wrote: Thanks for the patch Corey. I am afraid that the system does not have interrupts enabled. It uses polling mode. When the error is seen, I know for a fact that in function ipmi_thread() smi_result is SI_SM_CALL_WITH_DELAY, I have some logs where in busy_wait always reads as 1. Not sure if it was ever set to 0. (ill check this again). Ill anyway run the test using the patch that you have shared. b/w would it harm if we were to do to something like this ? Unfortunately, that would start the timer unnecessarily. You don't want to start timers unnecessarily in the kernel or the power management police will come after you. The patch I sent did have this call in the non-idle portion of the kernel thread and that should have done the same thing. Plus, if you are using the kernel thread, it's going to run periodically and should kick things off again if they get stuck. I'm suspicious now that something else is going on. -corey Signed-off-by: Srinivas Gowda srinivas_g_go...@dell.com --- drivers/char/ipmi/ipmi_si_intf.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c index 15e4a60..e23484f 100644 --- a/drivers/char/ipmi/ipmi_si_intf.c +++ b/drivers/char/ipmi/ipmi_si_intf.c @@ -1008,6 +1008,7 @@ static int ipmi_thread(void *data) spin_unlock_irqrestore((smi_info-si_lock), flags); busy_wait = ipmi_thread_busy_wait(smi_result, smi_info, busy_until); + ipmi_start_timer_if_necessary(smi_info); if (smi_result == SI_SM_CALL_WITHOUT_DELAY) ; /* do nothing */ else if (smi_result == SI_SM_CALL_WITH_DELAY busy_wait) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] ipmi: setting mod_timer for read_event_msg buffer cmd
On 11/27/2013 04:34 AM, srinivas_g_go...@dell.com wrote:
>
> *Dell - Internal Use - Confidential *
>
> I hit a bug during one of our stress tests, Here is the issue that I
> am looking at.
>
> We have IPMI_READ_EVENT_MSG_BUFFER_CMD getting invoked from
> smi_event_handler.
>
> In case we hit error scenario, say "OBF not ready in time" we do not
> have smi_timeout driving the interface.
>
> Seems like the timer is not armed when we invoke
> IPMI_READ_EVENT_MSG_BUFFER_CMD from smi_event_handler.
>
> For the proposed patch I checked the return value of mod_timer just
> before smi_info->handlers->start_transaction, that returns 0 !!!
>
> gWithout smi_timeout handler getting called periodically, if the BMC
> fails to set OBF flag during the msg transaction of
> IPMI_READ_EVENT_MSG_BUFFER_CMD,
>
> the driver just keeps looping until the flag is set. Ideally we would
> want BMC to set the flag, but in case it doesn’t we do not want the
> driver to loop indefinitely rather hit KCS_ERROR states.
>
> To summarize, we do not have timer set to invoke smi_timeout() when we
> call IPMI_READ_EVENT_MSG_BUFFER_CMD from smi_event_handler.
>
> Do you feel there is a better way to fix it or a bug elsewhere…!
>
Ok, I think I know what is happening, and I think I have a fix. I'm
betting that you have interrupts on this, and
I found a situation where if an interrupt came in at a certain time, it
wouldn't start the timer. The attached patch should fix the problem.
Can you try this out?
Thanks for the detailed description.
-corey
>From a3dcd3ce65c4072acd707a000ba30ff917d52ef3 Mon Sep 17 00:00:00 2001
From: Corey Minyard
Date: Fri, 29 Nov 2013 18:13:45 -0600
Subject: [PATCH] IPMI: Start the timer properly in certain situations
If using interrupt, or in some cases with the IPMI thread, the
IPMI timer would not be started to time out operations, resulting
in the driver operation hanging.
Signed-off-by: Corey Minyard
---
drivers/char/ipmi/ipmi_si_intf.c | 17 ++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
index 68c5ef5..a8dfd94 100644
--- a/drivers/char/ipmi/ipmi_si_intf.c
+++ b/drivers/char/ipmi/ipmi_si_intf.c
@@ -981,6 +981,13 @@ static int ipmi_thread_busy_wait(enum si_sm_result smi_result,
return 1;
}
+static void ipmi_start_timer_if_necessary(struct smi_info *smi_info)
+{
+ if (!timer_pending(_info->si_timer)) {
+ mod_timer(_info->si_timer, jiffies + SI_TIMEOUT_JIFFIES);
+ smi_info->last_timeout_jiffies = jiffies;
+ }
+}
/*
* A busy-waiting loop for speeding up IPMI operation.
@@ -1014,8 +1021,10 @@ static int ipmi_thread(void *data)
schedule();
else if (smi_result == SI_SM_IDLE)
schedule_timeout_interruptible(100);
- else
+ else {
+ ipmi_start_timer_if_necessary(smi_info);
schedule_timeout_interruptible(1);
+ }
}
return 0;
}
@@ -1118,7 +1127,8 @@ static irqreturn_t si_irq_handler(int irq, void *data)
do_gettimeofday();
printk(KERN_DEBUG "**Interrupt: %d.%9.9d\n", t.tv_sec, t.tv_usec);
#endif
- smi_event_handler(smi_info, 0);
+ if (smi_event_handler(smi_info, 0) != SI_SM_IDLE)
+ ipmi_start_timer_if_necessary(smi_info);
spin_unlock_irqrestore(&(smi_info->si_lock), flags);
return IRQ_HANDLED;
}
@@ -1979,7 +1989,8 @@ static u32 ipmi_acpi_gpe(acpi_handle gpe_device,
do_gettimeofday();
printk("**ACPI_GPE: %d.%9.9d\n", t.tv_sec, t.tv_usec);
#endif
- smi_event_handler(smi_info, 0);
+ if (smi_event_handler(smi_info, 0) != SI_SM_IDLE)
+ ipmi_start_timer_if_necessary(smi_info);
spin_unlock_irqrestore(&(smi_info->si_lock), flags);
return ACPI_INTERRUPT_HANDLED;
--
1.8.3.1
Re: [PATCH 1/1] ipmi: setting mod_timer for read_event_msg buffer cmd
On 11/27/2013 04:34 AM, srinivas_g_go...@dell.com wrote:
*Dell - Internal Use - Confidential *
I hit a bug during one of our stress tests, Here is the issue that I
am looking at.
We have IPMI_READ_EVENT_MSG_BUFFER_CMD getting invoked from
smi_event_handler.
In case we hit error scenario, say OBF not ready in time we do not
have smi_timeout driving the interface.
Seems like the timer is not armed when we invoke
IPMI_READ_EVENT_MSG_BUFFER_CMD from smi_event_handler.
For the proposed patch I checked the return value of mod_timer just
before smi_info-handlers-start_transaction, that returns 0 !!!
gWithout smi_timeout handler getting called periodically, if the BMC
fails to set OBF flag during the msg transaction of
IPMI_READ_EVENT_MSG_BUFFER_CMD,
the driver just keeps looping until the flag is set. Ideally we would
want BMC to set the flag, but in case it doesn’t we do not want the
driver to loop indefinitely rather hit KCS_ERROR states.
To summarize, we do not have timer set to invoke smi_timeout() when we
call IPMI_READ_EVENT_MSG_BUFFER_CMD from smi_event_handler.
Do you feel there is a better way to fix it or a bug elsewhere…!
Ok, I think I know what is happening, and I think I have a fix. I'm
betting that you have interrupts on this, and
I found a situation where if an interrupt came in at a certain time, it
wouldn't start the timer. The attached patch should fix the problem.
Can you try this out?
Thanks for the detailed description.
-corey
From a3dcd3ce65c4072acd707a000ba30ff917d52ef3 Mon Sep 17 00:00:00 2001
From: Corey Minyard cminy...@mvista.com
Date: Fri, 29 Nov 2013 18:13:45 -0600
Subject: [PATCH] IPMI: Start the timer properly in certain situations
If using interrupt, or in some cases with the IPMI thread, the
IPMI timer would not be started to time out operations, resulting
in the driver operation hanging.
Signed-off-by: Corey Minyard cminy...@mvista.com
---
drivers/char/ipmi/ipmi_si_intf.c | 17 ++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
index 68c5ef5..a8dfd94 100644
--- a/drivers/char/ipmi/ipmi_si_intf.c
+++ b/drivers/char/ipmi/ipmi_si_intf.c
@@ -981,6 +981,13 @@ static int ipmi_thread_busy_wait(enum si_sm_result smi_result,
return 1;
}
+static void ipmi_start_timer_if_necessary(struct smi_info *smi_info)
+{
+ if (!timer_pending(smi_info-si_timer)) {
+ mod_timer(smi_info-si_timer, jiffies + SI_TIMEOUT_JIFFIES);
+ smi_info-last_timeout_jiffies = jiffies;
+ }
+}
/*
* A busy-waiting loop for speeding up IPMI operation.
@@ -1014,8 +1021,10 @@ static int ipmi_thread(void *data)
schedule();
else if (smi_result == SI_SM_IDLE)
schedule_timeout_interruptible(100);
- else
+ else {
+ ipmi_start_timer_if_necessary(smi_info);
schedule_timeout_interruptible(1);
+ }
}
return 0;
}
@@ -1118,7 +1127,8 @@ static irqreturn_t si_irq_handler(int irq, void *data)
do_gettimeofday(t);
printk(KERN_DEBUG **Interrupt: %d.%9.9d\n, t.tv_sec, t.tv_usec);
#endif
- smi_event_handler(smi_info, 0);
+ if (smi_event_handler(smi_info, 0) != SI_SM_IDLE)
+ ipmi_start_timer_if_necessary(smi_info);
spin_unlock_irqrestore((smi_info-si_lock), flags);
return IRQ_HANDLED;
}
@@ -1979,7 +1989,8 @@ static u32 ipmi_acpi_gpe(acpi_handle gpe_device,
do_gettimeofday(t);
printk(**ACPI_GPE: %d.%9.9d\n, t.tv_sec, t.tv_usec);
#endif
- smi_event_handler(smi_info, 0);
+ if (smi_event_handler(smi_info, 0) != SI_SM_IDLE)
+ ipmi_start_timer_if_necessary(smi_info);
spin_unlock_irqrestore((smi_info-si_lock), flags);
return ACPI_INTERRUPT_HANDLED;
--
1.8.3.1
Re: [PATCH 1/1] ipmi: setting mod_timer for read_event_msg buffer cmd
On 11/25/2013 03:51 AM, srinivas_g_go...@dell.com wrote: > Setting up mod_timer() for IPMI_READ_EVENT_MSG_BUFFER_CMD. > Driver stalls in case we hit error cases for IPMI_READ_EVENT_MSG_BUFFER_CMD. > > Signed-off-by: Srinivas Gowda > --- > drivers/char/ipmi/ipmi_si_intf.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/char/ipmi/ipmi_si_intf.c > b/drivers/char/ipmi/ipmi_si_intf.c > index 15e4a60..affcc52 100644 > --- a/drivers/char/ipmi/ipmi_si_intf.c > +++ b/drivers/char/ipmi/ipmi_si_intf.c > @@ -843,6 +843,9 @@ static enum si_sm_result smi_event_handler(struct > smi_info *smi_info, > smi_info->curr_msg->data[1] = IPMI_READ_EVENT_MSG_BUFFER_CMD; > smi_info->curr_msg->data_size = 2; > > + smi_info->last_timeout_jiffies = jiffies; > + mod_timer(_info->si_timer, (jiffies + SI_TIMEOUT_JIFFIES)); > + This change is not correct and will mess up timing. That code is called from a number of places and that's where the timer modification should be done, if any are required. I can't imagine how this change would make a difference, but if it does, it points to a bug elsewhere in the code (or possibly in the BMC), not here. -corey > smi_info->handlers->start_transaction( > smi_info->si_sm, > smi_info->curr_msg->data, -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] ipmi: setting mod_timer for read_event_msg buffer cmd
On 11/25/2013 03:51 AM, srinivas_g_go...@dell.com wrote: Setting up mod_timer() for IPMI_READ_EVENT_MSG_BUFFER_CMD. Driver stalls in case we hit error cases for IPMI_READ_EVENT_MSG_BUFFER_CMD. Signed-off-by: Srinivas Gowda srinivas_g_go...@dell.com --- drivers/char/ipmi/ipmi_si_intf.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c index 15e4a60..affcc52 100644 --- a/drivers/char/ipmi/ipmi_si_intf.c +++ b/drivers/char/ipmi/ipmi_si_intf.c @@ -843,6 +843,9 @@ static enum si_sm_result smi_event_handler(struct smi_info *smi_info, smi_info-curr_msg-data[1] = IPMI_READ_EVENT_MSG_BUFFER_CMD; smi_info-curr_msg-data_size = 2; + smi_info-last_timeout_jiffies = jiffies; + mod_timer(smi_info-si_timer, (jiffies + SI_TIMEOUT_JIFFIES)); + This change is not correct and will mess up timing. That code is called from a number of places and that's where the timer modification should be done, if any are required. I can't imagine how this change would make a difference, but if it does, it points to a bug elsewhere in the code (or possibly in the BMC), not here. -corey smi_info-handlers-start_transaction( smi_info-si_sm, smi_info-curr_msg-data, -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
