Re: [PATCH RFC net] linux/dim: Fix divide 0 in RDMA DIM.

[Leon Romanovsky <leonro@nvidia.com>]

On Mon, Jun 27, 2022 at 04:45:14PM +0800, Tao Liu wrote:
> On Mon, Jun 27, 2022 at 10:42:02AM +0300, Leon Romanovsky wrote:
> > On Thu, Jun 23, 2022 at 04:58:58PM +0800, Tao Liu wrote:
> > > We hit a divide 0 error in ofed 5.1.2.3.7.1. But dim.c and
> > > rdma_dim.c seem same as upstream.

<...>

> > > Fixes: f4915455dcf0 ("linux/dim: Implement RDMA adaptive moderation (DIM)")
> > > Signed-off-by: Tao Liu <thomas.liu@ucloud.cn>
> > > ---
> > >  lib/dim/rdma_dim.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > 
> > I think that this change will be better as it won't change
> > decision order in rdma_dim_stats_compare()
> > 
> > diff --git a/include/linux/dim.h b/include/linux/dim.h
> > index b698266d0035..69ae238ec2dc 100644
> > --- a/include/linux/dim.h
> > +++ b/include/linux/dim.h
> > @@ -21,7 +21,7 @@
> >   * We consider 10% difference as significant.
> >   */
> >  #define IS_SIGNIFICANT_DIFF(val, ref) \
> > -       (((100UL * abs((val) - (ref))) / (ref)) > 10)
> > +       (ref && (((100UL * abs((val) - (ref))) / (ref)) > 10))
> >  
> >  /*
> >   * Calculate the gap between two values.
> > 
> > 
> Reviewed code in net_dim_stats_compare() and rdma_dim_stats_compare(), the
> crash point is the only place not covered 0 condition. So it maybe not
> need to change the macro.

Change in the macro ensures that we check cqe_ratio only when it is
needed.

Can you please resubmit?

Thanks

> 
> But I am not familiar with the algorithm, and not sure what is the right
> return value.
> > > 
> > > diff --git a/lib/dim/rdma_dim.c b/lib/dim/rdma_dim.c
> > > index 15462d54758d..a657b106343c 100644
> > > --- a/lib/dim/rdma_dim.c
> > > +++ b/lib/dim/rdma_dim.c
> > > @@ -34,6 +34,9 @@ static int rdma_dim_stats_compare(struct dim_stats *curr,
> > >  		return (curr->cpms > prev->cpms) ? DIM_STATS_BETTER :
> > >  						DIM_STATS_WORSE;
> > >  
> > > +	if (!prev->cpe_ratio)
> > > +		return DIM_STATS_SAME;
> > > +
> > >  	if (IS_SIGNIFICANT_DIFF(curr->cpe_ratio, prev->cpe_ratio))
> > >  		return (curr->cpe_ratio > prev->cpe_ratio) ? DIM_STATS_BETTER :
> > >  						DIM_STATS_WORSE;
> > > -- 
> > > 2.30.1 (Apple Git-130)
> > > 
> >