Hi Minyard,

I test this patch, it works.

Thanks.

On 2017/7/30 10:20, miny...@acm.org wrote:
From: Corey Minyard <cminy...@mvista.com>

When I set the timeout to a specific value such as 500ms, the timeout
event will not happen in time due to the overflow in function
check_msg_timeout:
...
        ent->timeout -= timeout_period;
        if (ent->timeout > 0)
                return;
...

The type of timeout_period is long, but ent->timeout is unsigned long.
This patch makes the type consistent.

Reported-by: Weilong Chen <chenweil...@huawei.com>
Signed-off-by: Corey Minyard <cminy...@mvista.com>
---
I like to keep things consistent (though I obviously messed up here)
and keep variables that should be positive unsigned.

But you are right, there is a bug here and some inconsistency.
This patch changes timeout_period to be unsigned and fixes the
check.  Can you try this out?

 drivers/char/ipmi/ipmi_msghandler.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/char/ipmi/ipmi_msghandler.c 
b/drivers/char/ipmi/ipmi_msghandler.c
index 810b138..c82d9fd 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -4030,7 +4030,8 @@ smi_from_recv_msg(ipmi_smi_t intf, struct ipmi_recv_msg 
*recv_msg,
 }

 static void check_msg_timeout(ipmi_smi_t intf, struct seq_table *ent,
-                             struct list_head *timeouts, long timeout_period,
+                             struct list_head *timeouts,
+                             unsigned long timeout_period,
                              int slot, unsigned long *flags,
                              unsigned int *waiting_msgs)
 {
@@ -4043,8 +4044,8 @@ static void check_msg_timeout(ipmi_smi_t intf, struct 
seq_table *ent,
        if (!ent->inuse)
                return;

-       ent->timeout -= timeout_period;
-       if (ent->timeout > 0) {
+       if (timeout_period < ent->timeout) {
+               ent->timeout -= timeout_period;
                (*waiting_msgs)++;
                return;
        }
@@ -4110,7 +4111,8 @@ static void check_msg_timeout(ipmi_smi_t intf, struct 
seq_table *ent,
        }
 }

-static unsigned int ipmi_timeout_handler(ipmi_smi_t intf, long timeout_period)
+static unsigned int ipmi_timeout_handler(ipmi_smi_t intf,
+                                        unsigned long timeout_period)
 {
        struct list_head     timeouts;
        struct ipmi_recv_msg *msg, *msg2;


Reply via email to