Re: mmotm 2019-07-24-21-39 uploaded (mm/memcontrol)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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);



[Index of Archives]     [Linux Kernel]     [Linux USB Development]     [Yosemite News]     [Linux SCSI]

  Powered by Linux