> -----Original Message----- > From: Heiner Kallweit <[email protected]> > Sent: Wednesday, December 2, 2020 11:55 PM > To: Kiyanovski, Arthur <[email protected]>; [email protected]; > [email protected] > Cc: Woodhouse, David <[email protected]>; Machulsky, Zorik > <[email protected]>; Matushevsky, Alexander <[email protected]>; > Bshara, Saeed <[email protected]>; Wilson, Matt <[email protected]>; > Liguori, Anthony <[email protected]>; Bshara, Nafea > <[email protected]>; Tzalik, Guy <[email protected]>; Belgazal, > Netanel <[email protected]>; Saidi, Ali <[email protected]>; > Herrenschmidt, Benjamin <[email protected]>; Dagan, Noam > <[email protected]>; Agroskin, Shay <[email protected]>; Jubran, > Samih <[email protected]> > Subject: RE: [EXTERNAL] [PATCH V3 net-next 1/9] net: ena: use constant > value for net_device allocation > > CAUTION: This email originated from outside of the organization. Do not click > links or open attachments unless you can confirm the sender and know the > content is safe. > > > > Am 02.12.2020 um 21:03 schrieb [email protected]: > > From: Arthur Kiyanovski <[email protected]> > > > > The patch changes the maximum number of RX/TX queues it advertises to > > the kernel (via alloc_etherdev_mq()) from a value received from the > > device to a constant value which is the minimum between 128 and the > > number of CPUs in the system. > > > > By allocating the net_device struct with a constant number of queues, > > the driver is able to allocate it at a much earlier stage, before > > calling any ena_com functions. This would allow to make all log prints > > in ena_com to use netdev_* log functions instead or current pr_* ones. > > > > Did you test this? Usually using netdev_* before the net_device is registered > results in quite ugly messages. Therefore there's a number of patches doing > the opposite, replacing netdev_* with dev_* before register_netdev(). See > e.g. > 22148df0d0bd ("r8169: don't use netif_info et al before net_device has been > registered")
Thanks for your comment. Yes we did test it. Please see the discussion which led to this patch in a previous thread here: https://www.mail-archive.com/[email protected]/msg353590.html > > Signed-off-by: Shay Agroskin <[email protected]> > > Signed-off-by: Arthur Kiyanovski <[email protected]> > > --- > > drivers/net/ethernet/amazon/ena/ena_netdev.c | 46 > > ++++++++++---------- > > 1 file changed, 23 insertions(+), 23 deletions(-) > > > > diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c > > b/drivers/net/ethernet/amazon/ena/ena_netdev.c > > index df1884d57d1a..985dea1870b5 100644 > > --- a/drivers/net/ethernet/amazon/ena/ena_netdev.c > > +++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c > > @@ -29,6 +29,8 @@ MODULE_LICENSE("GPL"); > > /* Time in jiffies before concluding the transmitter is hung. */ > > #define TX_TIMEOUT (5 * HZ) > > > > +#define ENA_MAX_RINGS min_t(unsigned int, > ENA_MAX_NUM_IO_QUEUES, > > +num_possible_cpus()) > > + > > #define ENA_NAPI_BUDGET 64 > > > > #define DEFAULT_MSG_ENABLE (NETIF_MSG_DRV | NETIF_MSG_PROBE > | > > NETIF_MSG_IFUP | \ @@ -4176,18 +4178,34 @@ static int > ena_probe(struct > > pci_dev *pdev, const struct pci_device_id *ent) > > > > ena_dev->dmadev = &pdev->dev; > > > > + netdev = alloc_etherdev_mq(sizeof(struct ena_adapter), > ENA_MAX_RINGS); > > + if (!netdev) { > > + dev_err(&pdev->dev, "alloc_etherdev_mq failed\n"); > > + rc = -ENOMEM; > > + goto err_free_region; > > + } > > + > > + SET_NETDEV_DEV(netdev, &pdev->dev); > > + adapter = netdev_priv(netdev); > > + adapter->ena_dev = ena_dev; > > + adapter->netdev = netdev; > > + adapter->pdev = pdev; > > + adapter->msg_enable = netif_msg_init(debug, > DEFAULT_MSG_ENABLE); > > + > > + pci_set_drvdata(pdev, adapter); > > + > > rc = ena_device_init(ena_dev, pdev, &get_feat_ctx, &wd_state); > > if (rc) { > > dev_err(&pdev->dev, "ENA device init failed\n"); > > if (rc == -ETIME) > > rc = -EPROBE_DEFER; > > - goto err_free_region; > > + goto err_netdev_destroy; > > } > > > > rc = ena_map_llq_mem_bar(pdev, ena_dev, bars); > > if (rc) { > > dev_err(&pdev->dev, "ENA llq bar mapping failed\n"); > > - goto err_free_ena_dev; > > + goto err_device_destroy; > > } > > > > calc_queue_ctx.ena_dev = ena_dev; @@ -4207,26 +4225,8 @@ static > > int ena_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > > goto err_device_destroy; > > } > > > > - /* dev zeroed in init_etherdev */ > > - netdev = alloc_etherdev_mq(sizeof(struct ena_adapter), > max_num_io_queues); > > - if (!netdev) { > > - dev_err(&pdev->dev, "alloc_etherdev_mq failed\n"); > > - rc = -ENOMEM; > > - goto err_device_destroy; > > - } > > - > > - SET_NETDEV_DEV(netdev, &pdev->dev); > > - > > - adapter = netdev_priv(netdev); > > - pci_set_drvdata(pdev, adapter); > > - > > - adapter->ena_dev = ena_dev; > > - adapter->netdev = netdev; > > - adapter->pdev = pdev; > > - > > ena_set_conf_feat_params(adapter, &get_feat_ctx); > > > > - adapter->msg_enable = netif_msg_init(debug, > DEFAULT_MSG_ENABLE); > > adapter->reset_reason = ENA_REGS_RESET_NORMAL; > > > > adapter->requested_tx_ring_size = calc_queue_ctx.tx_queue_size; > > @@ -4257,7 +4257,7 @@ static int ena_probe(struct pci_dev *pdev, const > struct pci_device_id *ent) > > if (rc) { > > dev_err(&pdev->dev, > > "Failed to query interrupt moderation feature\n"); > > - goto err_netdev_destroy; > > + goto err_device_destroy; > > } > > ena_init_io_rings(adapter, > > 0, > > @@ -4335,11 +4335,11 @@ static int ena_probe(struct pci_dev *pdev, > const struct pci_device_id *ent) > > ena_disable_msix(adapter); > > err_worker_destroy: > > del_timer(&adapter->timer_service); > > -err_netdev_destroy: > > - free_netdev(netdev); > > err_device_destroy: > > ena_com_delete_host_info(ena_dev); > > ena_com_admin_destroy(ena_dev); > > +err_netdev_destroy: > > + free_netdev(netdev); > > err_free_region: > > ena_release_bars(ena_dev, pdev); > > err_free_ena_dev: > >
