| Message ID | 1554887407-17965-1-git-send-email-devesh.sharma@broadcom.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Jason Gunthorpe |
| Headers | show |
| Series | RDMA/ocrdma: remove use of idr use pci bdf instead | expand |
On Wed, Apr 10, 2019 at 05:10:07AM -0400, Devesh Sharma wrote: > removing the use of IDR variable just to name the > function ids. Using the PCI_FUNC(pdev->devfn) instead > to create the device name, associated resources and > to print driver into at various places. > > Reported-by: Matthew Wilcox <willy@infradead.org> > Signed-off-by: Devesh Sharma <devesh.sharma@broadcom.com> > --- > drivers/infiniband/hw/ocrdma/ocrdma_main.c | 13 ++----------- > 1 file changed, 2 insertions(+), 11 deletions(-) I'm going to apply this so we can progress on the IDR conversion, but please send a patch fixing this driver to not call pr_err/warn/etc Drivers should be calling dev_warn/etc and not trying to print their own device name. As is this driver has broken logging in the presense of device rename. However, it is worth waiting until Gal's patch is merged to add ib specific printing functions is merged and use those functions instead. https://patchwork.kernel.org/patch/10874849/ Hopefully Gal will resend his patch soon, we can apply it.. Jason
On Fri, Apr 12, 2019 at 7:37 PM Jason Gunthorpe <jgg@mellanox.com> wrote: > > On Wed, Apr 10, 2019 at 05:10:07AM -0400, Devesh Sharma wrote: > > removing the use of IDR variable just to name the > > function ids. Using the PCI_FUNC(pdev->devfn) instead > > to create the device name, associated resources and > > to print driver into at various places. > > > > Reported-by: Matthew Wilcox <willy@infradead.org> > > Signed-off-by: Devesh Sharma <devesh.sharma@broadcom.com> > > --- > > drivers/infiniband/hw/ocrdma/ocrdma_main.c | 13 ++----------- > > 1 file changed, 2 insertions(+), 11 deletions(-) > > I'm going to apply this so we can progress on the IDR conversion, but > please send a patch fixing this driver to not call pr_err/warn/etc > > Drivers should be calling dev_warn/etc and not trying to print their > own device name. As is this driver has broken logging in the presense > of device rename. > > However, it is worth waiting until Gal's patch is merged to add ib specific > printing functions is merged and use those functions instead. Okay sure, I will keep track of it. > > https://patchwork.kernel.org/patch/10874849/ > > Hopefully Gal will resend his patch soon, we can apply it.. > > Jason
On 12-Apr-19 18:59, Devesh Sharma wrote: > On Fri, Apr 12, 2019 at 7:37 PM Jason Gunthorpe <jgg@mellanox.com> wrote: >> >> On Wed, Apr 10, 2019 at 05:10:07AM -0400, Devesh Sharma wrote: >>> removing the use of IDR variable just to name the >>> function ids. Using the PCI_FUNC(pdev->devfn) instead >>> to create the device name, associated resources and >>> to print driver into at various places. >>> >>> Reported-by: Matthew Wilcox <willy@infradead.org> >>> Signed-off-by: Devesh Sharma <devesh.sharma@broadcom.com> >>> --- >>> drivers/infiniband/hw/ocrdma/ocrdma_main.c | 13 ++----------- >>> 1 file changed, 2 insertions(+), 11 deletions(-) >> >> I'm going to apply this so we can progress on the IDR conversion, but >> please send a patch fixing this driver to not call pr_err/warn/etc >> >> Drivers should be calling dev_warn/etc and not trying to print their >> own device name. As is this driver has broken logging in the presense >> of device rename. >> >> However, it is worth waiting until Gal's patch is merged to add ib specific >> printing functions is merged and use those functions instead. > Okay sure, I will keep track of it. >> >> https://patchwork.kernel.org/patch/10874849/ >> >> Hopefully Gal will resend his patch soon, we can apply it.. >> >> Jason I'll resubmit it in the next few days.
diff --git a/drivers/infiniband/hw/ocrdma/ocrdma_main.c b/drivers/infiniband/hw/ocrdma/ocrdma_main.c index 34d3d59..8bc2f97 100644 --- a/drivers/infiniband/hw/ocrdma/ocrdma_main.c +++ b/drivers/infiniband/hw/ocrdma/ocrdma_main.c @@ -62,8 +62,6 @@ MODULE_AUTHOR("Emulex Corporation"); MODULE_LICENSE("Dual BSD/GPL"); -static DEFINE_IDR(ocrdma_dev_id); - void ocrdma_get_guid(struct ocrdma_dev *dev, u8 *guid) { u8 mac_addr[6]; @@ -310,13 +308,10 @@ static struct ocrdma_dev *ocrdma_add(struct be_dev_info *dev_info) } dev->mbx_cmd = kzalloc(sizeof(struct ocrdma_mqe_emb_cmd), GFP_KERNEL); if (!dev->mbx_cmd) - goto idr_err; + goto init_err; memcpy(&dev->nic_info, dev_info, sizeof(*dev_info)); - dev->id = idr_alloc(&ocrdma_dev_id, NULL, 0, 0, GFP_KERNEL); - if (dev->id < 0) - goto idr_err; - + dev->id = PCI_FUNC(dev->nic_info.pdev->devfn); status = ocrdma_init_hw(dev); if (status) goto init_err; @@ -353,8 +348,6 @@ static struct ocrdma_dev *ocrdma_add(struct be_dev_info *dev_info) ocrdma_free_resources(dev); ocrdma_cleanup_hw(dev); init_err: - idr_remove(&ocrdma_dev_id, dev->id); -idr_err: kfree(dev->mbx_cmd); ib_dealloc_device(&dev->ibdev); pr_err("%s() leaving. ret=%d\n", __func__, status); @@ -364,7 +357,6 @@ static struct ocrdma_dev *ocrdma_add(struct be_dev_info *dev_info) static void ocrdma_remove_free(struct ocrdma_dev *dev) { - idr_remove(&ocrdma_dev_id, dev->id); kfree(dev->mbx_cmd); ib_dealloc_device(&dev->ibdev); } @@ -469,7 +461,6 @@ static void __exit ocrdma_exit_module(void) { be_roce_unregister_driver(&ocrdma_drv); ocrdma_rem_debugfs(); - idr_destroy(&ocrdma_dev_id); } module_init(ocrdma_init_module);
removing the use of IDR variable just to name the function ids. Using the PCI_FUNC(pdev->devfn) instead to create the device name, associated resources and to print driver into at various places. Reported-by: Matthew Wilcox <willy@infradead.org> Signed-off-by: Devesh Sharma <devesh.sharma@broadcom.com> --- drivers/infiniband/hw/ocrdma/ocrdma_main.c | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-)