Hi, Peter
On 2019/4/15 20:46, Peter Zijlstra wrote:
I write a demo about this, which I described it as overflow.
#cat test.c
//test.c
#include <stdio.h>
#include <stdlib.h>
#include <limits.h>
#include <assert.h>
int main(void)
{
long a = 1048576 * 9144968455305; /* shares = tg_shares * load */
unsigned long b = a;
//unsigned long e = 1048576 * 9144968455305;
printf("LONG_MAX = %ld, 0x%lx\n", LONG_MAX, LONG_MAX);
printf("ULONG_MAX = %lu, 0x%lx\n", ULONG_MAX, ULONG_MAX);
if (b > LONG_MAX)
printf("==overflow!!!==\n"); // OVERFLOW
printf("a = %20ld,0x%016lx\n", a, a);
printf("b = %20lu,0x%016lx\n", b, b);
/* shares /= tg_weight */
printf("a/3 = 0x%016lx\n", (a / 3)); // WRONG
printf("a/3 = 0x%016lx\n", (((unsigned long)a) / 3)); // unsigned
printf("a/3 = 0x%016lx\n", (a / (unsigned long)3)); // unsigned
printf("b/3 = 0x%016lx\n", b / 3); // unsigned
return EXIT_SUCCESS;
}
#./test
LONG_MAX = 9223372036854775807, 0x7fffffffffffffff
ULONG_MAX = 18446744073709551615, 0xffffffffffffffff
==overflow!!!==
a = -8857549630719655936,0x8513a98a48900000
b = 9589194442989895680,0x8513a98a48900000
a/3 = 0xd7068dd8c2daaaab // WRONG
a/3 = 0x2c5be32e18300000
a/3 = 0x2c5be32e18300000
b/3 = 0x2c5be32e18300000
On Sat, Apr 13, 2019 at 03:32:34AM +0000, Cheng Jian wrote:
group_share and group_runnable are tracked as 'unsigned long',
however some functions using them as 'long' which is ultimately
assigned back to 'unsigned long' variables in reweight_entity.
Since there is not scope on using a different and signed type,
this change improves code consistency and avoids further type
conversions. More important, to prevent undefined behavior
caused by overflow.
There is no undefined behaviour due to overflow.UBSAN is broken,
upgrade to GCC8 or later.
.
So, function calc_group_shares will return the wrong value.
```cpp
static long calc_group_shares(struct cfs_rq *cfs_rq)
{
// ......
shares = (tg_shares * load);
if (tg_weight)
shares /= tg_weight;
}
```
The same to calc_group_runnable.
Thanks.
CHENG Jian.
