u64 division: truly the gift that keeps on giving. Thanks Andrew for following
up on these.
Andrew Morton writes:
Ah.
It's rather unclear why that u64 cast is there anyway. We're dealing
with ulongs all over this code. The below will suffice.
This place in particular uses u64 to make sure we don't overflow when left
shifting, since the numbers can get pretty big (and that's somewhat needed due
to the need for high precision when calculating the penalty jiffies). It's ok
if the output after division is an unsigned long, just the intermediate steps
need to have enough precision.
Chris, please take a look?
--- a/mm/memcontrol.c~mm-throttle-allocators-when-failing-reclaim-over-memoryhigh-fix-fix-fix
+++ a/mm/memcontrol.c
@@ -2415,7 +2415,7 @@ void mem_cgroup_handle_over_high(void)
clamped_high = max(high, 1UL);
overage = (u64)(usage - high) << MEMCG_DELAY_PRECISION_SHIFT;
- do_div(overage, clamped_high);
+ overage /= clamped_high;
I think this isn't going to work because left shifting by
MEMCG_DELAY_PRECISION_SHIFT can make the number bigger than ULONG_MAX, which
may cause wraparound -- we need to retain the u64 until we divide.
Maybe div_u64 will satisfy both ARM and i386? ie.
diff --git mm/memcontrol.c mm/memcontrol.c
index 5c7b9facb0eb..e12a47e96154 100644
--- mm/memcontrol.c
+++ mm/memcontrol.c
@@ -2419,8 +2419,8 @@ void mem_cgroup_handle_over_high(void)
*/
clamped_high = max(high, 1UL);
- overage = (u64)(usage - high) << MEMCG_DELAY_PRECISION_SHIFT;
- do_div(overage, clamped_high);
+ overage = div_u64((u64)(usage - high) << MEMCG_DELAY_PRECISION_SHIFT,
+ clamped_high);
penalty_jiffies = ((u64)overage * overage * HZ)
>> (MEMCG_DELAY_PRECISION_SHIFT + MEMCG_DELAY_SCALING_SHIFT);
