| Message ID | 20200826164214.31792-10-snelson@pensando.io |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | ionic memory usage rework | expand |
On Wed, 26 Aug 2020 09:42:11 -0700 Shannon Nelson wrote: > + mutex_lock(&lif->queue_lock); > + netif_device_detach(lif->netdev); > + ionic_stop_queues(lif); > + ionic_txrx_deinit(lif); > > + err = ionic_txrx_init(lif); > + if (err) > + goto err_out; > + > + /* don't start the queues until we have link */ > + if (netif_carrier_ok(netdev)) { > + err = ionic_start_queues(lif); > + if (err) > + goto err_out; > + } > + > +err_out: > + netif_device_attach(lif->netdev); > + mutex_unlock(&lif->queue_lock); Looks a little racy, since the link state is changed before queue_lock is taken: if (!netif_carrier_ok(netdev)) { u32 link_speed; ionic_port_identify(lif->ionic); link_speed = le32_to_cpu(lif->info->status.link_speed); netdev_info(netdev, "Link up - %d Gbps\n", link_speed / 1000); netif_carrier_on(netdev); } if (lif->netdev->flags & IFF_UP && netif_running(lif->netdev)) \ { mutex_lock(&lif->queue_lock); ionic_start_queues(lif); mutex_unlock(&lif->queue_lock); }
On 8/26/20 2:09 PM, Jakub Kicinski wrote: > On Wed, 26 Aug 2020 09:42:11 -0700 Shannon Nelson wrote: >> + mutex_lock(&lif->queue_lock); >> + netif_device_detach(lif->netdev); >> + ionic_stop_queues(lif); >> + ionic_txrx_deinit(lif); >> >> + err = ionic_txrx_init(lif); >> + if (err) >> + goto err_out; >> + >> + /* don't start the queues until we have link */ >> + if (netif_carrier_ok(netdev)) { >> + err = ionic_start_queues(lif); >> + if (err) >> + goto err_out; >> + } >> + >> +err_out: >> + netif_device_attach(lif->netdev); >> + mutex_unlock(&lif->queue_lock); > Looks a little racy, since the link state is changed before queue_lock > is taken: > > if (!netif_carrier_ok(netdev)) { u32 link_speed; > ionic_port_identify(lif->ionic); > link_speed = le32_to_cpu(lif->info->status.link_speed); > netdev_info(netdev, "Link up - %d Gbps\n", > link_speed / 1000); > netif_carrier_on(netdev); > } > > if (lif->netdev->flags & IFF_UP && netif_running(lif->netdev)) \ > { > mutex_lock(&lif->queue_lock); > ionic_start_queues(lif); > mutex_unlock(&lif->queue_lock); > } Yeah, that would probably be better served to just call ionic_link_status_check_request() here and let it do the job. sln
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c index b77827e9355c..d82ae717bc6c 100644 --- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c +++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c @@ -36,6 +36,8 @@ static void ionic_lif_handle_fw_down(struct ionic_lif *lif); static void ionic_lif_handle_fw_up(struct ionic_lif *lif); static void ionic_lif_set_netdev_info(struct ionic_lif *lif); +static void ionic_txrx_deinit(struct ionic_lif *lif); +static int ionic_txrx_init(struct ionic_lif *lif); static int ionic_start_queues(struct ionic_lif *lif); static void ionic_stop_queues(struct ionic_lif *lif); static void ionic_lif_queue_identify(struct ionic_lif *lif); @@ -593,6 +595,17 @@ static int ionic_qcqs_alloc(struct ionic_lif *lif) return err; } +static void ionic_qcq_sanitize(struct ionic_qcq *qcq) +{ + qcq->q.tail_idx = 0; + qcq->q.head_idx = 0; + qcq->cq.tail_idx = 0; + qcq->cq.done_color = 1; + memset(qcq->q_base, 0, qcq->q_size); + memset(qcq->cq_base, 0, qcq->cq_size); + memset(qcq->sg_base, 0, qcq->sg_size); +} + static int ionic_lif_txq_init(struct ionic_lif *lif, struct ionic_qcq *qcq) { struct device *dev = lif->ionic->dev; @@ -632,9 +645,7 @@ static int ionic_lif_txq_init(struct ionic_lif *lif, struct ionic_qcq *qcq) dev_dbg(dev, "txq_init.ver %d\n", ctx.cmd.q_init.ver); dev_dbg(dev, "txq_init.intr_index %d\n", ctx.cmd.q_init.intr_index); - q->tail_idx = 0; - q->head_idx = 0; - cq->tail_idx = 0; + ionic_qcq_sanitize(qcq); err = ionic_adminq_post_wait(lif, &ctx); if (err) @@ -689,9 +700,7 @@ static int ionic_lif_rxq_init(struct ionic_lif *lif, struct ionic_qcq *qcq) dev_dbg(dev, "rxq_init.ver %d\n", ctx.cmd.q_init.ver); dev_dbg(dev, "rxq_init.intr_index %d\n", ctx.cmd.q_init.intr_index); - q->tail_idx = 0; - q->head_idx = 0; - cq->tail_idx = 0; + ionic_qcq_sanitize(qcq); err = ionic_adminq_post_wait(lif, &ctx); if (err) @@ -1326,8 +1335,30 @@ static int ionic_change_mtu(struct net_device *netdev, int new_mtu) return err; netdev->mtu = new_mtu; - err = ionic_reset_queues(lif, NULL, NULL); + /* if we're not running, nothing more to do */ + if (!netif_running(lif->netdev)) + return 0; + + /* stop and reinit the queues */ + mutex_lock(&lif->queue_lock); + netif_device_detach(lif->netdev); + ionic_stop_queues(lif); + ionic_txrx_deinit(lif); + err = ionic_txrx_init(lif); + if (err) + goto err_out; + + /* don't start the queues until we have link */ + if (netif_carrier_ok(netdev)) { + err = ionic_start_queues(lif); + if (err) + goto err_out; + } + +err_out: + netif_device_attach(lif->netdev); + mutex_unlock(&lif->queue_lock); return err; }
We really don't need to tear down and rebuild the whole queue structure when changing the MTU; we can simply stop the queues, clean and refill, then restart the queues. Signed-off-by: Shannon Nelson <snelson@pensando.io> --- .../net/ethernet/pensando/ionic/ionic_lif.c | 45 ++++++++++++++++--- 1 file changed, 38 insertions(+), 7 deletions(-)