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

[Tao Liu <thomas.liu@ucloud.cn>]

On Mon, Jun 27, 2022 at 11:57:30AM +0300, Leon Romanovsky wrote:
> 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?
> 
Yes, I will. Thanks for your comment.

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