Re: [PATCH net-next v12 2/8] hinic3: Fix loose success check in rx buffer filling

Jakub Kicinski <[email protected]> Mon, 3 Aug 2026 16:13:14 -0700
Newsgroups org.kernel.vger.netdev,org.kernel.vger.linux-doc,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
On Wed, 29 Jul 2026 15:44:50 +0800 Fan Gong wrote:
> > > diff --git a/drivers/net/ethernet/huawei/hinic3/hinic3_rx.c b/drivers/net/ethernet/huawei/hinic3/hinic3_rx.c
> > > index 309ab5901379..389b1c2158be 100644
> > > --- a/drivers/net/ethernet/huawei/hinic3/hinic3_rx.c
> > > +++ b/drivers/net/ethernet/huawei/hinic3/hinic3_rx.c
> > > @@ -541,7 +541,7 @@ int hinic3_configure_rxqs(struct net_device *netdev, u16 num_rq,
> > >   		rq_associate_cqes(rxq);
> > >
> > >   		pkts = hinic3_rx_fill_buffers(rxq);
> > > -		if (!pkts) {
> > > +		if (pkts < rxq->q_mask) {
> > >   			netdev_err(netdev, "Failed to fill Rx buffer\n");
> > >   			return -ENOMEM;
> > >   		}  
> >
> > Given the subject, curious if a partially populated ring result in any
> > functional failure? if so, then perhaps failure mode can be explained
> > and this specific change be sent to net tree?  
> 
> Thanks for the comment. It fixes a previous review comment of the patchset
> as the following quotes. To see the full review, visit
> https://lore.kernel.org/netdev/[email protected]/

You misunderstood the comment there.
The concern was not about this function.
The concern is that hinic3_change_channel_settings() in the proposed
patch may fail after already tearing down the old configuration.
Driver should allocate memory and other resources for the queue
_before_ freeing the old memory. This limits the downtime and prevents
failed reconfiguration from bringing the device down.

IOW, when I said "this function may fail" I meant that you can't call
function which may fail after stopping the device and freeing mem.
Not that there is anything wrong with the function failing as it does..