Re: Re: [PATCH net-next v12 2/8] hinic3: Fix loose success check in rx buffer filling
Fan Gong <[email protected]> Wed, 5 Aug 2026 10:22:12 +0800
| Newsgroups | gmane.linux.documentation,gmane.linux.kernel,gmane.linux.network |
|---|---|
| Message-ID | <[email protected]> |
> > > > 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..
Thanks for your explaination. We indeed misunderstood your comment, so we
made this change. We will remove it in the next patchset.