On Fri, May 22, 2015 at 15:52:25 +0800, Wang Yufei wrote:

From: Zhang Bo <oscar.zhangbo@huawei.com>

When we change system clock to years ago, a certain CPU may use up 100% cputime. The reason is that in function virEventPollCalculateTimeout(), we assign the unsigned long long result to an INT variable, *timeout = then - now; // timeout is INT, and then/now are long long if (*timeout < 0) *timeout = 0; there's a chance that variable @then minus variable @now may be a very large number that overflows INT value expression, then *timeout will be negative and be assigned to 0. Next the 'poll' in function virEventPollRunOnce() will get into an 'endless' while loop there. thus, the cpu that virEventPollRunOnce() thread runs on will go up to 100%.

Although as we discussed before in https://www.redhat.com/archives/libvir-list/2015-May/msg00400.html it should be prohibited to set-time while other applications are running, but it does seems to have no harm to make the codes more robust.

Signed-off-by: Wang Yufei <james.wangyufei@huawei.com> Signed-off-by: Zhang Bo <oscar.zhangbo@huawei.com> --- src/util/vireventpoll.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/util/vireventpoll.c b/src/util/vireventpoll.c index ffda206..9c9e7af 100644 --- a/src/util/vireventpoll.c +++ b/src/util/vireventpoll.c @@ -357,9 +357,12 @@ static int virEventPollCalculateTimeout(int *timeout) return -1;

EVENT_DEBUG("Schedule timeout then=%llu now=%llu", then, now); - *timeout = then - now; - if (*timeout < 0) + if (then <= now) { *timeout = 0; + } else { + *timeout = (int) (then - now);

This still won't fix the overflow issue since the same implicit typecast would be done without the explicit one. You probably should clamp the value to INT_MAX if you want to be safe.

+ *timeout = (*timeout > 0) ? (*timeout) : (*timeout)*(-1);

This would denote that timeout overflowed, hence you did not fix it at first.

+ } } else { *timeout = -1; }

I'm not discussing the previous comments done by DanPB though. Peter