On Tue, Sep 25, 2018 at 11:28:24AM +0000, Ruhl, Michael J wrote: > >-----Original Message----- > >From: Jason Gunthorpe [mailto:jgg@xxxxxxxxxxxx] > >Sent: Monday, September 24, 2018 5:09 PM > >To: Ruhl, Michael J <michael.j.ruhl@xxxxxxxxx> > >Cc: linux-rdma@xxxxxxxxxxxxxxx; Doug Ledford <dledford@xxxxxxxxxx>; Selvin > >Xavier <selvin.xavier@xxxxxxxxxxxx>; Devesh Sharma > ><devesh.sharma@xxxxxxxxxxxx>; Somnath Kotur > ><somnath.kotur@xxxxxxxxxxxx>; Sriharsha Basavapatna > ><sriharsha.basavapatna@xxxxxxxxxxxx>; Steve Wise <swise@xxxxxxxxxxx>; > >Marciniszyn, Mike <mike.marciniszyn@xxxxxxxxx>; Dalessandro, Dennis > ><dennis.dalessandro@xxxxxxxxx>; Lijun Ou <oulijun@xxxxxxxxxx>; Wei > >Hu(Xavier) <xavier.huwei@xxxxxxxxxx>; Latif, Faisal <faisal.latif@xxxxxxxxx>; > >Saleem, Shiraz <shiraz.saleem@xxxxxxxxx>; Yishai Hadas > ><yishaih@xxxxxxxxxxxx>; Leon Romanovsky <leon@xxxxxxxxxx>; Michal > >Kalderon <Michal.Kalderon@xxxxxxxxxx>; Ariel Elior <Ariel.Elior@xxxxxxxxxx>; > >Christian Benvenuti <benve@xxxxxxxxx>; Adit Ranadive <aditr@xxxxxxxxxx>; > >VMware PV-Drivers <pv-drivers@xxxxxxxxxx>; Bart Van Assche > ><bvanassche@xxxxxxx> > >Subject: Re: [PATCH 1/6] RDMA: Fully setup the device name in ib_register_device > > > >On Mon, Sep 24, 2018 at 09:01:32PM +0000, Ruhl, Michael J wrote: > >> >From: Jason Gunthorpe <jgg@xxxxxxxxxxxx> > >> > > >> >The current code has two copies of the device name, ibdev->dev and > >> >dev_name(&ibdev->dev), and they are setup at different times, which is > >> >very confusing. > >> > > >> >Set them both up at the same time and make dev_name() the lead name, > >> >which > >> >is the proper use of the driver core APIs. To make it very clear that the > >> >name is not valid until registration pass it in to the > >> >ib_register_device() call rather than messing with ibdev->name directly. > >> > > >> >Also the reorganization now checks that dev_name is unique even if it does > >> >not contain a %. > >> > > >> >Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxxxx> > >> > drivers/infiniband/hw/hfi1/init.c | 1 - > >> > drivers/infiniband/hw/hfi1/verbs.c | 4 ++- > >> > drivers/infiniband/hw/qib/qib_init.c | 1 - > >> > drivers/infiniband/hw/qib/qib_verbs.c | 4 ++- > >> >diff --git a/drivers/infiniband/hw/hfi1/init.c > >> >b/drivers/infiniband/hw/hfi1/init.c > >> >index 1e770a1337793e..e87ff7a544eb1a 100644 > >> >+++ b/drivers/infiniband/hw/hfi1/init.c > >> >@@ -1313,7 +1313,6 @@ static struct hfi1_devdata > >*hfi1_alloc_devdata(struct > >> >pci_dev *pdev, > >> > "Could not allocate unit ID: error %d\n", -ret); > >> > goto bail; > >> > } > >> >- rvt_set_ibdev_name(&dd->verbs_dev.rdi, "%s_%d", class_name(), > >> >dd->unit); > >> > >> If you remove this call, the HFI (and probably the QIB) dd_dev_xxx() macros will > >> not work correctly until we call ib_register(). Since we do a lot of work before > >> that, this will lose our ability to print out the device name. > > > >What prints exactly? > > > >From what I can see the hfi driver uses macros like this: > > > >#define dd_dev_warn(dd, fmt, ...) \ > > dev_warn(&(dd)->pcidev->dev, "%s: " fmt, \ > > You missed the important part of this macro: > > #define dd_dev_warn(dd, fmt, ...) \ > dev_warn(&(dd)->pcidev->dev, "%s: " fmt, \ > rvt_get_ibdev_name(&(dd)->verbs_dev.rdi), ##__VA_ARGS__) > > The unit name is retrieved with the rvt_get_ibdev_name() function. > > >Which uses the physical device dev for printing, not the IB device. > > > >Generally speaking, drivers are supposed to use the physical device > >for printing until registration completes, then they should use the ib > >device. > > Yah. Need to ponder the best way to clean this up. Convert to use dev_name and set respective init_name. Thanks > > M > > >Jason
Attachment:
signature.asc
Description: PGP signature
