On Wed, Jun 26, 2024 at 04:19:17AM +0800, Zhu Yanjun wrote:
> 在 2024/6/25 13:00, Anand Khoje 写道:
> >
> > On 6/25/24 02:11, Jesse Brandeburg wrote:
> > > On 6/24/2024 8:33 AM, Anand Khoje wrote:
> > >
> > > > --- a/drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c
> > > > +++ b/drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c
> > > > @@ -608,6 +608,7 @@ enum {
> > > > RELEASE_ALL_PAGES_MASK = 0x4000,
> > > > };
> > > > +#define MAX_RECLAIM_NPAGES -50000
> > > Can you please explain why this is negative? There doesn't seem to be
> > > any reason mentioned in the commit message or code.
> > >
> > > At the very least it's super confusing to have a MAX be negative, and at
> > > worst it's a bug. I don't have any other context on this code besides
> > > this patch, so an explanation would be helpful.
> > >
> > >
> > >
> > Hi Jesse,
> >
> > The way Mellanox ConnectX5 driver handles 'release of allocated pages
> > from HCA' or 'allocation of pages to HCA', is by sending an event to the
> > host. This event will have number of pages in it. If the number is
> > positive, that indicates HCA is requesting that number of pages to be
> > allocated. And if that number is negative, it is the HCA indicating that
> > that number of pages can be reclaimed by the host.
> >
> > In this patch we are restricting the maximum number of pages that can be
> > reclaimed to be 50000 (effectively this would be -50000 as it is
> > reclaim). This limit is based on the capability of the firmware as it
> > cannot release more than 50000 back to the host in one go.
> >
> > I hope that explains.
>
> To be honest, I am also obvious why this MACRO is defined as a negative
> number. From the above, I can understand why. I think, perhaps many people
> also wonder why it is defined as a negative. IMO, it is better that you put
> the above explanations into the source code as comments.
> When users check the source code, from the comments, users will know why it
> is defined as a negative number.
I see no problem with adding a comment to the code, but I think that it
won't help anyone. The whole reclaim/give page logic inside the mlx5
driver is written with the assumption that the number of pages is
negative for reclaim and positive for give.
Thanks
>
> Thanks a lot.
> Zhu Yanjun
>
> >
> > Thanks,
> >
> > Anand
> >
>
>
