Messages in this thread | | | From | Suren Baghdasaryan <> | Date | Tue, 12 Nov 2019 10:33:21 -0800 | Subject | Re: [PATCH] psi:fix divide by zero in psi_update_stats |
| |
On Tue, Nov 12, 2019 at 7:41 AM Johannes Weiner <hannes@cmpxchg.org> wrote: > > On Fri, Nov 08, 2019 at 03:33:24PM +0800, tim wrote: > > In psi_update_stats, it is possible that period has value like > > 0xXXXXXXXX00000000 where the lower 32 bit is 0, then it calls div_u64 which > > truncates u64 period to u32, results in zero divisor. > > Use div64_u64() instead of div_u64() if the divisor is u64 to avoid > > truncation to 32-bit on 64-bit platforms. > > > > Signed-off-by: xiejingfeng <xiejingfeng@linux.alibaba.com> > > This is legit. When we stop the periodic averaging worker due to an > idle CPU, the period after restart can be much longer than the ~4 sec > in the lower 32 bits. See the missed_periods logic in update_averages. > > What is surprising is that you can hit this repeatedly, as the odds > are 1 in 4,294,967,296. An extremely coarse clock source? > > Acked-by: Johannes Weiner <hannes@cmpxchg.org> > > There are several more instances of div_u64 in psi.c. They all look > fine to me except for one in the psi poll() windowing code, where we > divide by the window size, which can be up to 10s. CCing Suren. > > --- > From 009cece5f37a38f4baeb1bebdcb432ac9ae66ef8 Mon Sep 17 00:00:00 2001 > From: Johannes Weiner <hannes@cmpxchg.org> > Date: Tue, 12 Nov 2019 10:35:26 -0500 > Subject: [PATCH] psi: fix a division error in psi poll() > > The psi window size is a u64 an can be up to 10 seconds right now, > which exceeds the lower 32 bits of the variable. But div_u64 is meant > only for 32-bit divisors. Use div64_u64 for the 64-bit divisor. > > Signed-off-by: Johannes Weiner <hannes@cmpxchg.org> > --- > kernel/sched/psi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/kernel/sched/psi.c b/kernel/sched/psi.c > index 517e3719027e..84af7aa158bf 100644 > --- a/kernel/sched/psi.c > +++ b/kernel/sched/psi.c > @@ -481,7 +481,7 @@ static u64 window_update(struct psi_window *win, u64 now, u64 value) > u32 remaining; > > remaining = win->size - elapsed; > - growth += div_u64(win->prev_growth * remaining, win->size); > + growth += div64_u64(win->prev_growth * remaining, win->size); > } > > return growth; > -- > 2.24.0 >
Reviewed-by: Suren Baghdasaryan <surenb@google.com>
| |