diff mbox series

[v3,02/13] scsi: fnic: Add and use fnic number

Message ID 20231114223221.633719-1-kartilak@cisco.com (mailing list archive)
State Accepted
Headers show
Series Introduce support for multiqueue (MQ) in fnic | expand

Commit Message

Karan Tilak Kumar (kartilak) Nov. 14, 2023, 10:32 p.m. UTC 
Add fnic_num in fnic.h to identify fnic in a multi-fnic environment.
Increment and set the fnic number during driver load in fnic_probe.
Replace the host number with fnic number in debugfs.

Reviewed-by: Sesidhar Baddela <sebaddel@cisco.com>
Reviewed-by: Arulprabhu Ponnusamy <arulponn@cisco.com>
Signed-off-by: Karan Tilak Kumar <kartilak@cisco.com>
---
 drivers/scsi/fnic/fnic.h         | 1 +
 drivers/scsi/fnic/fnic_debugfs.c | 2 +-
 drivers/scsi/fnic/fnic_main.c    | 6 +++++-
 3 files changed, 7 insertions(+), 2 deletions(-)

Comments

Martin K. Petersen Nov. 25, 2023, 1:22 a.m. UTC | #1 
Hi Karan,

> Add fnic_num in fnic.h to identify fnic in a multi-fnic environment.
> Increment and set the fnic number during driver load in fnic_probe.
> Replace the host number with fnic number in debugfs.

I agree with Hannes. Everything else in the stack will be using the host
number. Your change will make it harder to correlate a SCSI error
message to an fnic driver instance.

If you absolutely need an instance number I suggest you add it as an
orthogonal value instead replacing the host. Also, we typically use an
idr for enumerating things like this.
Karan Tilak Kumar (kartilak) Nov. 27, 2023, 8:17 p.m. UTC | #2 
On Friday, November 24, 2023 5:22 PM, Martin K. Petersen <martin.petersen@oracle.com> wrote:
>
>
> Hi Karan,
>
> > Add fnic_num in fnic.h to identify fnic in a multi-fnic environment.
> > Increment and set the fnic number during driver load in fnic_probe.
> > Replace the host number with fnic number in debugfs.
>
> I agree with Hannes. Everything else in the stack will be using the host number. Your change will make it harder to correlate a SCSI error message to an fnic driver instance.
>
> If you absolutely need an instance number I suggest you add it as an orthogonal value instead replacing the host. Also, we typically use an idr for enumerating things like this.
>

Thanks for your review and comments, Martin.

I understand the issue. I'll take your suggestion about adding the fnic instance number as an orthogonal value instead of replacing the host number.
Could you please help me understand what is meant by "idr"? How can I use it?

Regards,
Karan
Karan Tilak Kumar (kartilak) Nov. 29, 2023, 5:26 p.m. UTC | #3 
On Monday, November 27, 2023 12:18 PM, Karan Tilak Kumar (kartilak) wrote:
>
> On Friday, November 24, 2023 5:22 PM, Martin K. Petersen <martin.petersen@oracle.com> wrote:
> >
> >
> > Hi Karan,
> >
> > > Add fnic_num in fnic.h to identify fnic in a multi-fnic environment.
> > > Increment and set the fnic number during driver load in fnic_probe.
> > > Replace the host number with fnic number in debugfs.
> >
> > I agree with Hannes. Everything else in the stack will be using the host number. Your change will make it harder to correlate a SCSI error message to an fnic driver instance.
> >
> > If you absolutely need an instance number I suggest you add it as an orthogonal value instead replacing the host. Also, we typically use an idr for enumerating things like this.
> >
>
> Thanks for your review and comments, Martin.
>
> I understand the issue. I'll take your suggestion about adding the fnic instance number as an orthogonal value instead of replacing the host number.
> Could you please help me understand what is meant by "idr"? How can I use it?
>

I found it in the Linux documentation.  
I'll make suitable changes and re-submit the patch set as V4.

Regards,
Karan
diff mbox series

Patch

diff --git a/drivers/scsi/fnic/fnic.h b/drivers/scsi/fnic/fnic.h
index 93c68931a593..c6c549c633b1 100644
--- a/drivers/scsi/fnic/fnic.h
+++ b/drivers/scsi/fnic/fnic.h
@@ -216,6 +216,7 @@  struct fnic_event {
 
 /* Per-instance private data structure */
 struct fnic {
+	int fnic_num;
 	struct fc_lport *lport;
 	struct fcoe_ctlr ctlr;		/* FIP FCoE controller structure */
 	struct vnic_dev_bar bar0;
diff --git a/drivers/scsi/fnic/fnic_debugfs.c b/drivers/scsi/fnic/fnic_debugfs.c
index c4d9ed0d7d75..fac617672868 100644
--- a/drivers/scsi/fnic/fnic_debugfs.c
+++ b/drivers/scsi/fnic/fnic_debugfs.c
@@ -676,7 +676,7 @@  void fnic_stats_debugfs_init(struct fnic *fnic)
 {
 	char name[16];
 
-	snprintf(name, sizeof(name), "host%d", fnic->lport->host->host_no);
+	snprintf(name, sizeof(name), "fnic%d", fnic->fnic_num);
 
 	fnic->fnic_stats_debugfs_host = debugfs_create_dir(name,
 						fnic_stats_debugfs_root);
diff --git a/drivers/scsi/fnic/fnic_main.c b/drivers/scsi/fnic/fnic_main.c
index 984bc5fc55e2..ea7b1ba27ac7 100644
--- a/drivers/scsi/fnic/fnic_main.c
+++ b/drivers/scsi/fnic/fnic_main.c
@@ -84,6 +84,9 @@  static struct libfc_function_template fnic_transport_template = {
 	.exch_mgr_reset = fnic_exch_mgr_reset
 };
 
+
+atomic_t fnic_num;
+
 static int fnic_slave_alloc(struct scsi_device *sdev)
 {
 	struct fc_rport *rport = starget_to_rport(scsi_target(sdev));
@@ -587,6 +590,7 @@  static int fnic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	int i;
 	unsigned long flags;
 
+	atomic_inc(&fnic_num);
 	/*
 	 * Allocate SCSI Host and set up association between host,
 	 * local port, and fnic
@@ -608,7 +612,7 @@  static int fnic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		 host->host_no);
 
 	host->transportt = fnic_fc_transport;
-
+	fnic->fnic_num = atomic_read(&fnic_num);
 	fnic_stats_debugfs_init(fnic);
 
 	/* Setup PCI resources */