Re: [PATCH v6 net-next 02/10] qed/qede: make devlink survive recovery
From: Jesse Brandeburg <hidden>
Date: 2020-08-21 18:18:03
Igor Russkikh wrote:
Devlink instance lifecycle was linked to qed_dev object, that caused devlink to be recreated on each recovery. Changing it by making higher level driver (qede) responsible for its life. This way devlink now survives recoveries. qede now stores devlink structure pointer as a part of its device object, devlink private data contains a linkage structure, qed_devlink. Signed-off-by: Igor Russkikh <redacted> Signed-off-by: Alexander Lobakin <redacted> Signed-off-by: Michal Kalderon <redacted>
<snip>
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/qlogic/qede/qede_main.c b/drivers/net/ethernet/qlogic/qede/qede_main.c index 140a392a81bb..93071d41afe4 100644 --- a/drivers/net/ethernet/qlogic/qede/qede_main.c +++ b/drivers/net/ethernet/qlogic/qede/qede_main.c@@ -1170,10 +1170,23 @@ static int __qede_probe(struct pci_dev *pdev, u32 dp_module, u8 dp_level, rc = -ENOMEM; goto err2; } + + edev->devlink = qed_ops->common->devlink_register(cdev); + if (IS_ERR(edev->devlink)) { + DP_NOTICE(edev, "Cannot register devlink\n"); + edev->devlink = NULL; + /* Go on, we can live without devlink */ + } } else { struct net_device *ndev = pci_get_drvdata(pdev); edev = netdev_priv(ndev); + + if (edev && edev->devlink) { + struct qed_devlink *qdl = devlink_priv(edev->devlink); + + qdl->cdev = cdev; + } edev->cdev = cdev; memset(&edev->stats, 0, sizeof(edev->stats)); memcpy(&edev->dev_info, &dev_info, sizeof(dev_info));
same comment as against old version:
cppcheck notes that the edev check here before the && is either
unnecessary, or the original code had a possible null pointer
dereference. I figure the code should just be
if (edev->devlink) {
And I recommend that you try to run cppcheck --enable=all on your code,
it finds several style violations and a few other null pointer checks
that can be fixed up.