Re: [PATCH net-next v3] net: mvneta: Use min macro

[Simon Horman]

On Sun, Sep 01, 2024 at 10:52:38AM +0000, David Laight wrote:
> From: Yan Zhen
> > Sent: 30 August 2024 02:04
> > To: marcin.s.wojtas@xxxxxxxxx; davem@xxxxxxxxxxxxx; edumazet@xxxxxxxxxx; kuba@xxxxxxxxxx;
> > 
> > Using the real macro is usually more intuitive and readable,
> > When the original file is guaranteed to contain the minmax.h header file
> > and compile correctly.
> > 
> > Signed-off-by: Yan Zhen <yanzhen@xxxxxxxx>
> > ---
> > 
> > Changes in v3:
> > - Rewrite the subject.
> > 
> >  drivers/net/ethernet/marvell/mvneta.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
> > index d72b2d5f96db..08d277165f40 100644
> > --- a/drivers/net/ethernet/marvell/mvneta.c
> > +++ b/drivers/net/ethernet/marvell/mvneta.c
> > @@ -4750,8 +4750,7 @@ mvneta_ethtool_set_ringparam(struct net_device *dev,
> > 
> >  	if ((ring->rx_pending == 0) || (ring->tx_pending == 0))
> >  		return -EINVAL;
> > -	pp->rx_ring_size = ring->rx_pending < MVNETA_MAX_RXD ?
> > -		ring->rx_pending : MVNETA_MAX_RXD;
> > +	pp->rx_ring_size = umin(ring->rx_pending, MVNETA_MAX_RXD);
> 
> Why did you use umin() instead of min() ?

Possibly because I mistakenly advised it is appropriate, sorry about that.
Given your explanation elsewhere [1], I now agree min() is appropriate.

[1] https://lore.kernel.org/netdev/20240901171150.GA23170@xxxxxxxxxx/T/#mebc52fc11de13eff8a610e3a63c5d1026d527492

> >  	pp->tx_ring_size = clamp_t(u16, ring->tx_pending,
> >  				   MVNETA_MAX_SKB_DESCS * 2, MVNETA_MAX_TXD);
> 
> Hmmm how about a patch to fix the bug in that line?
> A typical example of the complete misuse of the '_t' variants.
> The fact that the LHS is u16 doesn't mean that it is anyway
> correct to cast the RHS value to u16.
> In this case if someone tries to set the ring size to 64k they'll