| Message ID | 20230202013002.34358-4-shannon.nelson@amd.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | ionic: code maintenance | expand |
On Wed, Feb 01, 2023 at 05:29:59PM -0800, Shannon Nelson wrote: > Make sure there are qcqs to clean before trying to swap resources > or clean their interrupt assignments. > > Fixes: 101b40a0171f ("ionic: change queue count with no reset") > Signed-off-by: Shannon Nelson <shannon.nelson@amd.com> > --- > .../net/ethernet/pensando/ionic/ionic_lif.c | 27 ++++++++++++++++--- > 1 file changed, 24 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c > index 8499165b1563..c08d0762212c 100644 > --- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c > +++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c > @@ -2741,6 +2741,14 @@ int ionic_reconfigure_queues(struct ionic_lif *lif, > sg_desc_sz = sizeof(struct ionic_txq_sg_desc); > > for (i = 0; i < qparam->nxqs; i++) { > + /* If missing, short placeholder qcq needed for swap */ > + if (!lif->txqcqs[i]) { > + flags = IONIC_QCQ_F_TX_STATS | IONIC_QCQ_F_SG; > + err = ionic_qcq_alloc(lif, IONIC_QTYPE_TXQ, i, "tx", flags, > + 4, desc_sz, comp_sz, sg_desc_sz, > + lif->kern_pid, &lif->txqcqs[i]); You are not checking return value, so you don't really need to store returned value in err variable. Thanks
On Wed, 1 Feb 2023 17:29:59 -0800 Shannon Nelson wrote: > Make sure there are qcqs to clean before trying to swap resources > or clean their interrupt assignments. ... Otherwise $what-may-happen Bug fixes should come with an explanation of what the user-visible misbehavior is
On 2/2/23 10:05 AM, Jakub Kicinski wrote: > On Wed, 1 Feb 2023 17:29:59 -0800 Shannon Nelson wrote: >> Make sure there are qcqs to clean before trying to swap resources >> or clean their interrupt assignments. > > ... Otherwise $what-may-happen > > Bug fixes should come with an explanation of what the user-visible > misbehavior is I can add some text here and post a v2. Would you prefer I repost some of these as net-next rather than net as Leon was suggesting, or keep this patchset together for v2? I have a couple other larger net-next patches getting ready as well... sln
On Thu, 2 Feb 2023 12:06:52 -0800 Shannon Nelson wrote: > On 2/2/23 10:05 AM, Jakub Kicinski wrote: > > On Wed, 1 Feb 2023 17:29:59 -0800 Shannon Nelson wrote: > >> Make sure there are qcqs to clean before trying to swap resources > >> or clean their interrupt assignments. > > > > ... Otherwise $what-may-happen > > > > Bug fixes should come with an explanation of what the user-visible > > misbehavior is > > I can add some text here and post a v2. > > Would you prefer I repost some of these as net-next rather than net as > Leon was suggesting, or keep this patchset together for v2? I have a > couple other larger net-next patches getting ready as well... I'm not sure what the user impact of the fixes is, but at a glance splitting this into separate series makes most sense. We merge net into net-next every Thu, so if there is a dependency the wait should not be too long.
On 2/2/23 12:46 PM, Jakub Kicinski wrote: > On Thu, 2 Feb 2023 12:06:52 -0800 Shannon Nelson wrote: >> On 2/2/23 10:05 AM, Jakub Kicinski wrote: >>> On Wed, 1 Feb 2023 17:29:59 -0800 Shannon Nelson wrote: >>>> Make sure there are qcqs to clean before trying to swap resources >>>> or clean their interrupt assignments. >>> >>> ... Otherwise $what-may-happen >>> >>> Bug fixes should come with an explanation of what the user-visible >>> misbehavior is >> >> I can add some text here and post a v2. >> >> Would you prefer I repost some of these as net-next rather than net as >> Leon was suggesting, or keep this patchset together for v2? I have a >> couple other larger net-next patches getting ready as well... > > I'm not sure what the user impact of the fixes is, but at a glance > splitting this into separate series makes most sense. We merge net > into net-next every Thu, so if there is a dependency the wait should > not be too long. Thanks. I'll resubmit 3 of these for net shortly, and pull the others into the net-next list and send after I rebase tonight or tomorrow. sln
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c index 8499165b1563..c08d0762212c 100644 --- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c +++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c @@ -2741,6 +2741,14 @@ int ionic_reconfigure_queues(struct ionic_lif *lif, sg_desc_sz = sizeof(struct ionic_txq_sg_desc); for (i = 0; i < qparam->nxqs; i++) { + /* If missing, short placeholder qcq needed for swap */ + if (!lif->txqcqs[i]) { + flags = IONIC_QCQ_F_TX_STATS | IONIC_QCQ_F_SG; + err = ionic_qcq_alloc(lif, IONIC_QTYPE_TXQ, i, "tx", flags, + 4, desc_sz, comp_sz, sg_desc_sz, + lif->kern_pid, &lif->txqcqs[i]); + } + flags = lif->txqcqs[i]->flags & ~IONIC_QCQ_F_INTR; err = ionic_qcq_alloc(lif, IONIC_QTYPE_TXQ, i, "tx", flags, num_desc, desc_sz, comp_sz, sg_desc_sz, @@ -2760,6 +2768,14 @@ int ionic_reconfigure_queues(struct ionic_lif *lif, comp_sz *= 2; for (i = 0; i < qparam->nxqs; i++) { + /* If missing, short placeholder qcq needed for swap */ + if (!lif->rxqcqs[i]) { + flags = IONIC_QCQ_F_RX_STATS | IONIC_QCQ_F_SG; + err = ionic_qcq_alloc(lif, IONIC_QTYPE_RXQ, i, "rx", flags, + 4, desc_sz, comp_sz, sg_desc_sz, + lif->kern_pid, &lif->rxqcqs[i]); + } + flags = lif->rxqcqs[i]->flags & ~IONIC_QCQ_F_INTR; err = ionic_qcq_alloc(lif, IONIC_QTYPE_RXQ, i, "rx", flags, num_desc, desc_sz, comp_sz, sg_desc_sz, @@ -2809,10 +2825,15 @@ int ionic_reconfigure_queues(struct ionic_lif *lif, lif->tx_coalesce_hw = lif->rx_coalesce_hw; } - /* clear existing interrupt assignments */ + /* Clear existing interrupt assignments. We check for NULL here + * because we're checking the whole array for potential qcqs, not + * just those qcqs that have just been set up. + */ for (i = 0; i < lif->ionic->ntxqs_per_lif; i++) { - ionic_qcq_intr_free(lif, lif->txqcqs[i]); - ionic_qcq_intr_free(lif, lif->rxqcqs[i]); + if (lif->txqcqs[i]) + ionic_qcq_intr_free(lif, lif->txqcqs[i]); + if (lif->rxqcqs[i]) + ionic_qcq_intr_free(lif, lif->rxqcqs[i]); } /* re-assign the interrupts */
Make sure there are qcqs to clean before trying to swap resources or clean their interrupt assignments. Fixes: 101b40a0171f ("ionic: change queue count with no reset") Signed-off-by: Shannon Nelson <shannon.nelson@amd.com> --- .../net/ethernet/pensando/ionic/ionic_lif.c | 27 ++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-)