Hi all, On Sat, 27 Jul 2019 11:16:08 +0100 Chris Down <chris@xxxxxxxxxxxxxx> wrote: > > 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); I have applied this to the akpm-current tree in linux-next today. -- Cheers, Stephen Rothwell
Attachment:
pgpxFn6SMh_4s.pgp
Description: OpenPGP digital signature
