Re: [PATCH rdma-next] RDMA/hfi1: Use PCI-ID as an identification in debugfs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/11/2018 1:27 PM, Leon Romanovsky wrote:
On Tue, Dec 11, 2018 at 12:21:53PM -0500, Dennis Dalessandro wrote:
On 12/11/2018 8:09 AM, Leon Romanovsky wrote:
On Tue, Dec 11, 2018 at 07:03:46AM -0500, Dennis Dalessandro wrote:
On 12/11/2018 6:36 AM, Yuval Shaia wrote:
On Tue, Dec 11, 2018 at 12:06:30PM +0200, Leon Romanovsky wrote:
From: Leon Romanovsky <leonro@xxxxxxxxxxxx>

Update hfi1 to use PCI-IDs as an identification in debugfs, instead
of device name which can be changed after user executes device rename.

Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
---
    drivers/infiniband/hw/hfi1/debugfs.c | 11 ++++++-----
    1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/hw/hfi1/debugfs.c b/drivers/infiniband/hw/hfi1/debugfs.c
index 0a557795563c..54b579d8ec07 100644
--- a/drivers/infiniband/hw/hfi1/debugfs.c
+++ b/drivers/infiniband/hw/hfi1/debugfs.c
@@ -1166,23 +1166,24 @@ void hfi1_dbg_ibdev_init(struct hfi1_ibdev *ibd)
    	char name[sizeof("port0counters") + 1];
    	char link[10];
    	struct hfi1_devdata *dd = dd_from_dev(ibd);
+	struct pci_dev *pdev = dd->pcidev;
    	struct hfi1_pportdata *ppd;
    	int unit = dd->unit;
    	int i, j;

    	if (!hfi1_dbg_root)
    		return;
-	snprintf(name, sizeof(name), "%s_%d", class_name(), unit);
+
    	snprintf(link, sizeof(link), "%d", unit);
-	ibd->hfi1_ibdev_dbg = debugfs_create_dir(name, hfi1_dbg_root);
+	ibd->hfi1_ibdev_dbg = debugfs_create_dir(pci_name(pdev), hfi1_dbg_root);
    	if (!ibd->hfi1_ibdev_dbg) {
-		pr_warn("create of %s failed\n", name);
+		pr_warn("create of %s failed\n", pci_name(pdev));
    		return;
    	}
    	ibd->hfi1_ibdev_link =
-		debugfs_create_symlink(link, hfi1_dbg_root, name);
+		debugfs_create_symlink(link, hfi1_dbg_root, pci_name(pdev));
    	if (!ibd->hfi1_ibdev_link) {
-		pr_warn("create of %s symlink failed\n", name);
+		pr_warn("create of %s symlink failed\n", pci_name(pdev));
    		return;
    	}
    	DEBUGFS_SEQ_FILE_CREATE(opcode_stats, ibd->hfi1_ibdev_dbg, ibd);

Reviewed-by: Yuval Shaia <yuval.shaia@xxxxxxxxxx>


Nak for now. This isn't quite what we have in mind. Patch is on the way,
going through review and testing. Will post soon to for-next.

Awesome, can you shed the light and summarize why this patch isn't good
fit for you?

The patch is very similar, our version, which is part of a bigger series,
doesn't use local variable and changes the pr_warn() calls to dd_dev_warn().
I can go ahead and post our version, or you could make a v2 to fix the
pr_warns, up to you. The local variable vs non isn't a big concern here,
meaning I'm fine either way.

I tried to localize and minimize my changes, so it is better to go with
your more comprehensive variant.

During review an issue was raised that this impacts a number of user space scripts/tools and there is a desire to not break the existing ABI and instead maintain a separate name space here. Considering device rename applies to the ib verbs device not necessarily what we have in debugfs which applies to the HW.

-Denny





[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux