>-----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. M >Jason
