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