[PATCH 0/9] lpfc: Update lpfc to revision 12.8.0.1

[PATCH 0/9] lpfc: Update lpfc to revision 12.8.0.1

[James Smart]

Update lpfc to revision 12.8.0.1

Patch set contains several small fixes, code cleanups, a change to
default number of hdwqs at init, and a sync of devloss with the
nvme-fc transport.

The patches were cut against Martin's 5.8/scsi-queue tree

James Smart (9):
  lpfc: Synchronize NVME transport and lpfc driver devloss_tmo
  lpfc: Maintain atomic consistency of queue_claimed flag
  lpfc: Remove re-binding of nvme rport during registration
  lpfc: Fix negation of else clause in lpfc_prep_node_fc4type
  lpfc: Change default queue allocation for reduced memory consumption
  lpfc: Remove unnecessary lockdep_assert_held calls
  lpfc: Fix noderef and address space warnings
  lpfc: Fix MDS Diagnostic Enablement definition
  lpfc: Update lpfc version to 12.8.0.1

 drivers/scsi/lpfc/lpfc.h         |  23 +++++--
 drivers/scsi/lpfc/lpfc_attr.c    | 106 ++++++++++++++++++++++++-------
 drivers/scsi/lpfc/lpfc_ct.c      |   1 -
 drivers/scsi/lpfc/lpfc_debugfs.c |   3 +-
 drivers/scsi/lpfc/lpfc_hbadisc.c |   8 +--
 drivers/scsi/lpfc/lpfc_hw4.h     |   2 +-
 drivers/scsi/lpfc/lpfc_init.c    |  82 ++++++++++--------------
 drivers/scsi/lpfc/lpfc_mbox.c    |   3 +-
 drivers/scsi/lpfc/lpfc_nvme.c    |  33 +---------
 drivers/scsi/lpfc/lpfc_sli.c     |  45 ++++++-------
 drivers/scsi/lpfc/lpfc_sli4.h    |   2 +-
 drivers/scsi/lpfc/lpfc_version.h |   2 +-
 12 files changed, 168 insertions(+), 142 deletions(-)

-- 
2.26.1

[PATCH 1/9] lpfc: Synchronize NVME transport and lpfc driver devloss_tmo

[James Smart]

The driver is not passing it's devloss tmo to the nvme-fc transport when
registering the remote port. Thus devloss tmo for the nvme-fc remote port
will be set to the transport's default. This causes driver actions to be
out of sync with transport actions and out of sync with scsi actions for
perhaps the same remote port.

This is especially notable in the following scenario: while remote port
is attached, devloss is changed globally for lpfc remote ports via lpfc
sysfs parameter. lpfc ties this change in with nvme-fc transport. If the
device disconnects long enough for devloss to expire thus the existing
remote port is deleted, then the remote port is re-discovered, the newly
created remote port will end up set at the transport default, not lpfc's
value.

Fix by setting devloss tmo value when registering the remote port.

Signed-off-by: Dick Kennedy <dick.kennedy@broadcom.com>
Signed-off-by: James Smart <jsmart2021@gmail.com>
---
 drivers/scsi/lpfc/lpfc_nvme.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/lpfc/lpfc_nvme.c b/drivers/scsi/lpfc/lpfc_nvme.c
index 12d2b2775773..43df08aeecf1 100644
--- a/drivers/scsi/lpfc/lpfc_nvme.c
+++ b/drivers/scsi/lpfc/lpfc_nvme.c
@@ -2296,6 +2296,7 @@ lpfc_nvme_register_port(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
 
 	rpinfo.port_name = wwn_to_u64(ndlp->nlp_portname.u.wwn);
 	rpinfo.node_name = wwn_to_u64(ndlp->nlp_nodename.u.wwn);
+	rpinfo.dev_loss_tmo = vport->cfg_devloss_tmo;
 
 	spin_lock_irq(&vport->phba->hbalock);
 	oldrport = lpfc_ndlp_get_nrport(ndlp);
-- 
2.26.1

[PATCH 2/9] lpfc: Maintain atomic consistency of queue_claimed flag

[James Smart]

A previous change introduced the atomic use of queue_claimed flag for
eq's and cq's.  The code works fine, but the clearing of the
queue_claimed flag is not atomic.

Change queue_claimed = 0 into xchg(&queue_claimed, 0) to be consistent
for change under atomicity.

Signed-off-by: Dick Kennedy <dick.kennedy@broadcom.com>
Signed-off-by: James Smart <jsmart2021@gmail.com>
---
 drivers/scsi/lpfc/lpfc_sli.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
index b6fb665e6ec4..9ce37560f4c0 100644
--- a/drivers/scsi/lpfc/lpfc_sli.c
+++ b/drivers/scsi/lpfc/lpfc_sli.c
@@ -538,7 +538,7 @@ lpfc_sli4_process_eq(struct lpfc_hba *phba, struct lpfc_queue *eq,
 	if (count > eq->EQ_max_eqe)
 		eq->EQ_max_eqe = count;
 
-	eq->queue_claimed = 0;
+	xchg(&eq->queue_claimed, 0);
 
 rearm_and_exit:
 	/* Always clear the EQ. */
@@ -13694,7 +13694,7 @@ __lpfc_sli4_process_cq(struct lpfc_hba *phba, struct lpfc_queue *cq,
 				"0369 No entry from completion queue "
 				"qid=%d\n", cq->queue_id);
 
-	cq->queue_claimed = 0;
+	xchg(&cq->queue_claimed, 0);
 
 rearm_and_exit:
 	phba->sli4_hba.sli4_write_cq_db(phba, cq, consumed,
-- 
2.26.1

[PATCH 3/9] lpfc: Remove re-binding of nvme rport during registration

[James Smart]

The lldd rebinds the ndlp with rport during a nvme rport registration
(va nvme_fc_register_remoteport). If rport & ndlp pointers are same as
the previous one, the lldd will re-use the ndlp and rport association
without re-iniitalization. This assumption is incorrect. The lldd should
be ignorant of whether the returned rport pointer is new or not, and
should always assume it is new.

Remove the re-binding code, always assumes that rport pointer received
from transport is a new pointer.

Signed-off-by: Dick Kennedy <dick.kennedy@broadcom.com>
Signed-off-by: James Smart <jsmart2021@gmail.com>
---
 drivers/scsi/lpfc/lpfc_nvme.c | 32 --------------------------------
 1 file changed, 32 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_nvme.c b/drivers/scsi/lpfc/lpfc_nvme.c
index 43df08aeecf1..3121cf37a572 100644
--- a/drivers/scsi/lpfc/lpfc_nvme.c
+++ b/drivers/scsi/lpfc/lpfc_nvme.c
@@ -2322,38 +2322,6 @@ lpfc_nvme_register_port(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
 		spin_unlock_irq(&vport->phba->hbalock);
 		rport = remote_port->private;
 		if (oldrport) {
-			/* New remoteport record does not guarantee valid
-			 * host private memory area.
-			 */
-			if (oldrport == remote_port->private) {
-				/* Same remoteport - ndlp should match.
-				 * Just reuse.
-				 */
-				lpfc_printf_vlog(ndlp->vport, KERN_INFO,
-						 LOG_NVME_DISC,
-						 "6014 Rebind lport to current "
-						 "remoteport x%px wwpn 0x%llx, "
-						 "Data: x%x x%x x%px x%px x%x "
-						 " x%06x\n",
-						 remote_port,
-						 remote_port->port_name,
-						 remote_port->port_id,
-						 remote_port->port_role,
-						 oldrport->ndlp,
-						 ndlp,
-						 ndlp->nlp_type,
-						 ndlp->nlp_DID);
-
-				/* It's a complete rebind only if the driver
-				 * is registering with the same ndlp. Otherwise
-				 * the driver likely executed a node swap
-				 * prior to this registration and the ndlp to
-				 * remoteport binding needs to be redone.
-				 */
-				if (prev_ndlp == ndlp)
-					return 0;
-
-			}
 
 			/* Sever the ndlp<->rport association
 			 * before dropping the ndlp ref from
-- 
2.26.1

[PATCH 4/9] lpfc: Fix negation of else clause in lpfc_prep_node_fc4type

[James Smart]

Implementation of a previous patch added a condition to an if check
that always end up with the if test being true. Execution of the else
clause was inadvertantly negated.  The additional condition check was
incorrect and unnecessary after the other modifications had been done
in that patch.

Remove the check from the if series.

Fixes: b95b21193c85 ("scsi: lpfc: Fix loss of remote port after devloss due to lack of RPIs")
Cc: <stable@vger.kernel.org> # v5.4+
Signed-off-by: Dick Kennedy <dick.kennedy@broadcom.com>
Signed-off-by: James Smart <jsmart2021@gmail.com>
---
 drivers/scsi/lpfc/lpfc_ct.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/scsi/lpfc/lpfc_ct.c b/drivers/scsi/lpfc/lpfc_ct.c
index 2aa578d20f8c..7fce73c39c1c 100644
--- a/drivers/scsi/lpfc/lpfc_ct.c
+++ b/drivers/scsi/lpfc/lpfc_ct.c
@@ -462,7 +462,6 @@ lpfc_prep_node_fc4type(struct lpfc_vport *vport, uint32_t Did, uint8_t fc4_type)
 	struct lpfc_nodelist *ndlp;
 
 	if ((vport->port_type != LPFC_NPIV_PORT) ||
-	    (fc4_type == FC_TYPE_FCP) ||
 	    !(vport->ct_flags & FC_CT_RFF_ID) || !vport->cfg_restrict_login) {
 
 		ndlp = lpfc_setup_disc_node(vport, Did);
-- 
2.26.1

[PATCH 5/9] lpfc: Change default queue allocation for reduced memory consumption

[James Smart]

By default, the driver attempts to allocate a hdwq per logical cpu in
order to provide good cpu affinity. Some systems have extremely high cpu
counts and this can significantly raise memory consumption.

In testing on x86 platforms (non-AMD) it is found that sharing of a hdwq
by a physical cpu and it's HT cpu can occur with little performance
degredation. By sharing the hdwq count can be halved, significantly
reducing the memory overhead.

Change the default behavior of the driver on non-AMD x86 platforms to
share a hdwq by the cpu and its HT cpu.

Signed-off-by: Dick Kennedy <dick.kennedy@broadcom.com>
Signed-off-by: James Smart <jsmart2021@gmail.com>
---
 drivers/scsi/lpfc/lpfc.h      |  23 ++++++--
 drivers/scsi/lpfc/lpfc_attr.c | 106 +++++++++++++++++++++++++++-------
 drivers/scsi/lpfc/lpfc_init.c |  82 +++++++++++---------------
 drivers/scsi/lpfc/lpfc_sli4.h |   2 +-
 4 files changed, 137 insertions(+), 76 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc.h b/drivers/scsi/lpfc/lpfc.h
index 8e2a356911a9..45657a7502f6 100644
--- a/drivers/scsi/lpfc/lpfc.h
+++ b/drivers/scsi/lpfc/lpfc.h
@@ -627,6 +627,19 @@ struct lpfc_ras_fwlog {
 	enum ras_state state;    /* RAS logging running state */
 };
 
+enum lpfc_irq_chann_mode {
+	/* Assign IRQs to all possible cpus that have hardware queues */
+	NORMAL_MODE,
+
+	/* Assign IRQs only to cpus on the same numa node as HBA */
+	NUMA_MODE,
+
+	/* Assign IRQs only on non-hyperthreaded CPUs. This is the
+	 * same as normal_mode, but assign IRQS only on physical CPUs.
+	 */
+	NHT_MODE,
+};
+
 struct lpfc_hba {
 	/* SCSI interface function jump table entries */
 	struct lpfc_io_buf * (*lpfc_get_scsi_buf)
@@ -835,7 +848,6 @@ struct lpfc_hba {
 	uint32_t cfg_fcp_mq_threshold;
 	uint32_t cfg_hdw_queue;
 	uint32_t cfg_irq_chann;
-	uint32_t cfg_irq_numa;
 	uint32_t cfg_suppress_rsp;
 	uint32_t cfg_nvme_oas;
 	uint32_t cfg_nvme_embed_cmd;
@@ -1003,6 +1015,7 @@ struct lpfc_hba {
 	mempool_t *active_rrq_pool;
 
 	struct fc_host_statistics link_stats;
+	enum lpfc_irq_chann_mode irq_chann_mode;
 	enum intr_type_t intr_type;
 	uint32_t intr_mode;
 #define LPFC_INTR_ERROR	0xFFFFFFFF
@@ -1314,19 +1327,19 @@ lpfc_phba_elsring(struct lpfc_hba *phba)
 }
 
 /**
- * lpfc_next_online_numa_cpu - Finds next online CPU on NUMA node
- * @numa_mask: Pointer to phba's numa_mask member.
+ * lpfc_next_online_cpu - Finds next online CPU on cpumask
+ * @mask: Pointer to phba's cpumask member.
  * @start: starting cpu index
  *
  * Note: If no valid cpu found, then nr_cpu_ids is returned.
  *
  **/
 static inline unsigned int
-lpfc_next_online_numa_cpu(const struct cpumask *numa_mask, unsigned int start)
+lpfc_next_online_cpu(const struct cpumask *mask, unsigned int start)
 {
 	unsigned int cpu_it;
 
-	for_each_cpu_wrap(cpu_it, numa_mask, start) {
+	for_each_cpu_wrap(cpu_it, mask, start) {
 		if (cpu_online(cpu_it))
 			break;
 	}
diff --git a/drivers/scsi/lpfc/lpfc_attr.c b/drivers/scsi/lpfc/lpfc_attr.c
index 1354c141d614..2791efa770af 100644
--- a/drivers/scsi/lpfc/lpfc_attr.c
+++ b/drivers/scsi/lpfc/lpfc_attr.c
@@ -5704,17 +5704,69 @@ LPFC_ATTR_R(hdw_queue,
 	    LPFC_HBA_HDWQ_MIN, LPFC_HBA_HDWQ_MAX,
 	    "Set the number of I/O Hardware Queues");
 
-static inline void
-lpfc_assign_default_irq_numa(struct lpfc_hba *phba)
+#if IS_ENABLED(CONFIG_X86)
+/**
+ * lpfc_cpumask_irq_mode_init - initalizes cpumask of phba based on
+ *				irq_chann_mode
+ * @phba: Pointer to HBA context object.
+ **/
+static void
+lpfc_cpumask_irq_mode_init(struct lpfc_hba *phba)
+{
+	unsigned int cpu, first_cpu, numa_node = NUMA_NO_NODE;
+	const struct cpumask *sibling_mask;
+	struct cpumask *aff_mask = &phba->sli4_hba.irq_aff_mask;
+
+	cpumask_clear(aff_mask);
+
+	if (phba->irq_chann_mode == NUMA_MODE) {
+		/* Check if we're a NUMA architecture */
+		numa_node = dev_to_node(&phba->pcidev->dev);
+		if (numa_node == NUMA_NO_NODE) {
+			phba->irq_chann_mode = NORMAL_MODE;
+			return;
+		}
+	}
+
+	for_each_possible_cpu(cpu) {
+		switch (phba->irq_chann_mode) {
+		case NUMA_MODE:
+			if (cpu_to_node(cpu) == numa_node)
+				cpumask_set_cpu(cpu, aff_mask);
+			break;
+		case NHT_MODE:
+			sibling_mask = topology_sibling_cpumask(cpu);
+			first_cpu = cpumask_first(sibling_mask);
+			if (first_cpu < nr_cpu_ids)
+				cpumask_set_cpu(first_cpu, aff_mask);
+			break;
+		default:
+			break;
+		}
+	}
+}
+#endif
+
+static void
+lpfc_assign_default_irq_chann(struct lpfc_hba *phba)
 {
 #if IS_ENABLED(CONFIG_X86)
-	/* If AMD architecture, then default is LPFC_IRQ_CHANN_NUMA */
-	if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
-		phba->cfg_irq_numa = 1;
-	else
-		phba->cfg_irq_numa = 0;
+	switch (boot_cpu_data.x86_vendor) {
+	case X86_VENDOR_AMD:
+		/* If AMD architecture, then default is NUMA_MODE */
+		phba->irq_chann_mode = NUMA_MODE;
+		break;
+	case X86_VENDOR_INTEL:
+		/* If Intel architecture, then default is no hyperthread mode */
+		phba->irq_chann_mode = NHT_MODE;
+		break;
+	default:
+		phba->irq_chann_mode = NORMAL_MODE;
+		break;
+	}
+	lpfc_cpumask_irq_mode_init(phba);
 #else
-	phba->cfg_irq_numa = 0;
+	phba->irq_chann_mode = NORMAL_MODE;
 #endif
 }
 
@@ -5726,6 +5778,7 @@ lpfc_assign_default_irq_numa(struct lpfc_hba *phba)
  *
  *	0		= Configure number of IRQ Channels to:
  *			  if AMD architecture, number of CPUs on HBA's NUMA node
+ *			  if Intel architecture, number of physical CPUs.
  *			  otherwise, number of active CPUs.
  *	[1,256]		= Manually specify how many IRQ Channels to use.
  *
@@ -5751,35 +5804,44 @@ MODULE_PARM_DESC(lpfc_irq_chann, "Set number of interrupt vectors to allocate");
 static int
 lpfc_irq_chann_init(struct lpfc_hba *phba, uint32_t val)
 {
-	const struct cpumask *numa_mask;
+	const struct cpumask *aff_mask;
 
 	if (phba->cfg_use_msi != 2) {
 		lpfc_printf_log(phba, KERN_INFO, LOG_INIT,
 				"8532 use_msi = %u ignoring cfg_irq_numa\n",
 				phba->cfg_use_msi);
-		phba->cfg_irq_numa = 0;
-		phba->cfg_irq_chann = LPFC_IRQ_CHANN_MIN;
+		phba->irq_chann_mode = NORMAL_MODE;
+		phba->cfg_irq_chann = LPFC_IRQ_CHANN_DEF;
 		return 0;
 	}
 
 	/* Check if default setting was passed */
 	if (val == LPFC_IRQ_CHANN_DEF)
-		lpfc_assign_default_irq_numa(phba);
+		lpfc_assign_default_irq_chann(phba);
 
-	if (phba->cfg_irq_numa) {
-		numa_mask = &phba->sli4_hba.numa_mask;
+	if (phba->irq_chann_mode != NORMAL_MODE) {
+		aff_mask = &phba->sli4_hba.irq_aff_mask;
 
-		if (cpumask_empty(numa_mask)) {
+		if (cpumask_empty(aff_mask)) {
 			lpfc_printf_log(phba, KERN_INFO, LOG_INIT,
-					"8533 Could not identify NUMA node, "
-					"ignoring cfg_irq_numa\n");
-			phba->cfg_irq_numa = 0;
-			phba->cfg_irq_chann = LPFC_IRQ_CHANN_MIN;
+					"8533 Could not identify CPUS for "
+					"mode %d, ignoring\n",
+					phba->irq_chann_mode);
+			phba->irq_chann_mode = NORMAL_MODE;
+			phba->cfg_irq_chann = LPFC_IRQ_CHANN_DEF;
 		} else {
-			phba->cfg_irq_chann = cpumask_weight(numa_mask);
+			phba->cfg_irq_chann = cpumask_weight(aff_mask);
+
+			/* If no hyperthread mode, then set hdwq count to
+			 * aff_mask weight as well
+			 */
+			if (phba->irq_chann_mode == NHT_MODE)
+				phba->cfg_hdw_queue = phba->cfg_irq_chann;
+
 			lpfc_printf_log(phba, KERN_INFO, LOG_INIT,
 					"8543 lpfc_irq_chann set to %u "
-					"(numa)\n", phba->cfg_irq_chann);
+					"(mode: %d)\n", phba->cfg_irq_chann,
+					phba->irq_chann_mode);
 		}
 	} else {
 		if (val > LPFC_IRQ_CHANN_MAX) {
@@ -5790,7 +5852,7 @@ lpfc_irq_chann_init(struct lpfc_hba *phba, uint32_t val)
 					val,
 					LPFC_IRQ_CHANN_MIN,
 					LPFC_IRQ_CHANN_MAX);
-			phba->cfg_irq_chann = LPFC_IRQ_CHANN_MIN;
+			phba->cfg_irq_chann = LPFC_IRQ_CHANN_DEF;
 			return -EINVAL;
 		}
 		phba->cfg_irq_chann = val;
diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c
index 4104bdcdbb6f..8b8530351843 100644
--- a/drivers/scsi/lpfc/lpfc_init.c
+++ b/drivers/scsi/lpfc/lpfc_init.c
@@ -6022,29 +6022,6 @@ static void lpfc_log_intr_mode(struct lpfc_hba *phba, uint32_t intr_mode)
 	return;
 }
 
-/**
- * lpfc_cpumask_of_node_init - initalizes cpumask of phba's NUMA node
- * @phba: Pointer to HBA context object.
- *
- **/
-static void
-lpfc_cpumask_of_node_init(struct lpfc_hba *phba)
-{
-	unsigned int cpu, numa_node;
-	struct cpumask *numa_mask = &phba->sli4_hba.numa_mask;
-
-	cpumask_clear(numa_mask);
-
-	/* Check if we're a NUMA architecture */
-	numa_node = dev_to_node(&phba->pcidev->dev);
-	if (numa_node == NUMA_NO_NODE)
-		return;
-
-	for_each_possible_cpu(cpu)
-		if (cpu_to_node(cpu) == numa_node)
-			cpumask_set_cpu(cpu, numa_mask);
-}
-
 /**
  * lpfc_enable_pci_dev - Enable a generic PCI device.
  * @phba: pointer to lpfc hba data structure.
@@ -6483,7 +6460,6 @@ lpfc_sli4_driver_resource_setup(struct lpfc_hba *phba)
 	phba->sli4_hba.num_present_cpu = lpfc_present_cpu;
 	phba->sli4_hba.num_possible_cpu = cpumask_last(cpu_possible_mask) + 1;
 	phba->sli4_hba.curr_disp_cpu = 0;
-	lpfc_cpumask_of_node_init(phba);
 
 	/* Get all the module params for configuring this host */
 	lpfc_get_cfgparam(phba);
@@ -6691,6 +6667,13 @@ lpfc_sli4_driver_resource_setup(struct lpfc_hba *phba)
 #endif
 				/* Not supported for NVMET */
 				phba->cfg_xri_rebalancing = 0;
+				if (phba->irq_chann_mode == NHT_MODE) {
+					phba->cfg_irq_chann =
+						phba->sli4_hba.num_present_cpu;
+					phba->cfg_hdw_queue =
+						phba->sli4_hba.num_present_cpu;
+					phba->irq_chann_mode = NORMAL_MODE;
+				}
 				break;
 			}
 		}
@@ -7032,7 +7015,7 @@ lpfc_sli4_driver_resource_unset(struct lpfc_hba *phba)
 	phba->sli4_hba.num_possible_cpu = 0;
 	phba->sli4_hba.num_present_cpu = 0;
 	phba->sli4_hba.curr_disp_cpu = 0;
-	cpumask_clear(&phba->sli4_hba.numa_mask);
+	cpumask_clear(&phba->sli4_hba.irq_aff_mask);
 
 	/* Free memory allocated for fast-path work queue handles */
 	kfree(phba->sli4_hba.hba_eq_hdl);
@@ -11287,11 +11270,12 @@ lpfc_irq_clear_aff(struct lpfc_hba_eq_hdl *eqhdl)
  * @offline: true, cpu is going offline. false, cpu is coming online.
  *
  * If cpu is going offline, we'll try our best effort to find the next
- * online cpu on the phba's NUMA node and migrate all offlining IRQ affinities.
+ * online cpu on the phba's original_mask and migrate all offlining IRQ
+ * affinities.
  *
- * If cpu is coming online, reaffinitize the IRQ back to the onlineng cpu.
+ * If cpu is coming online, reaffinitize the IRQ back to the onlining cpu.
  *
- * Note: Call only if cfg_irq_numa is enabled, otherwise rely on
+ * Note: Call only if NUMA or NHT mode is enabled, otherwise rely on
  *	 PCI_IRQ_AFFINITY to auto-manage IRQ affinity.
  *
  **/
@@ -11301,14 +11285,14 @@ lpfc_irq_rebalance(struct lpfc_hba *phba, unsigned int cpu, bool offline)
 	struct lpfc_vector_map_info *cpup;
 	struct cpumask *aff_mask;
 	unsigned int cpu_select, cpu_next, idx;
-	const struct cpumask *numa_mask;
+	const struct cpumask *orig_mask;
 
-	if (!phba->cfg_irq_numa)
+	if (phba->irq_chann_mode == NORMAL_MODE)
 		return;
 
-	numa_mask = &phba->sli4_hba.numa_mask;
+	orig_mask = &phba->sli4_hba.irq_aff_mask;
 
-	if (!cpumask_test_cpu(cpu, numa_mask))
+	if (!cpumask_test_cpu(cpu, orig_mask))
 		return;
 
 	cpup = &phba->sli4_hba.cpu_map[cpu];
@@ -11317,9 +11301,9 @@ lpfc_irq_rebalance(struct lpfc_hba *phba, unsigned int cpu, bool offline)
 		return;
 
 	if (offline) {
-		/* Find next online CPU on NUMA node */
-		cpu_next = cpumask_next_wrap(cpu, numa_mask, cpu, true);
-		cpu_select = lpfc_next_online_numa_cpu(numa_mask, cpu_next);
+		/* Find next online CPU on original mask */
+		cpu_next = cpumask_next_wrap(cpu, orig_mask, cpu, true);
+		cpu_select = lpfc_next_online_cpu(orig_mask, cpu_next);
 
 		/* Found a valid CPU */
 		if ((cpu_select < nr_cpu_ids) && (cpu_select != cpu)) {
@@ -11434,7 +11418,7 @@ lpfc_sli4_enable_msix(struct lpfc_hba *phba)
 {
 	int vectors, rc, index;
 	char *name;
-	const struct cpumask *numa_mask = NULL;
+	const struct cpumask *aff_mask = NULL;
 	unsigned int cpu = 0, cpu_cnt = 0, cpu_select = nr_cpu_ids;
 	struct lpfc_hba_eq_hdl *eqhdl;
 	const struct cpumask *maskp;
@@ -11444,16 +11428,18 @@ lpfc_sli4_enable_msix(struct lpfc_hba *phba)
 	/* Set up MSI-X multi-message vectors */
 	vectors = phba->cfg_irq_chann;
 
-	if (phba->cfg_irq_numa) {
-		numa_mask = &phba->sli4_hba.numa_mask;
-		cpu_cnt = cpumask_weight(numa_mask);
+	if (phba->irq_chann_mode != NORMAL_MODE)
+		aff_mask = &phba->sli4_hba.irq_aff_mask;
+
+	if (aff_mask) {
+		cpu_cnt = cpumask_weight(aff_mask);
 		vectors = min(phba->cfg_irq_chann, cpu_cnt);
 
-		/* cpu: iterates over numa_mask including offline or online
-		 * cpu_select: iterates over online numa_mask to set affinity
+		/* cpu: iterates over aff_mask including offline or online
+		 * cpu_select: iterates over online aff_mask to set affinity
 		 */
-		cpu = cpumask_first(numa_mask);
-		cpu_select = lpfc_next_online_numa_cpu(numa_mask, cpu);
+		cpu = cpumask_first(aff_mask);
+		cpu_select = lpfc_next_online_cpu(aff_mask, cpu);
 	} else {
 		flags |= PCI_IRQ_AFFINITY;
 	}
@@ -11487,7 +11473,7 @@ lpfc_sli4_enable_msix(struct lpfc_hba *phba)
 
 		eqhdl->irq = pci_irq_vector(phba->pcidev, index);
 
-		if (phba->cfg_irq_numa) {
+		if (aff_mask) {
 			/* If found a neighboring online cpu, set affinity */
 			if (cpu_select < nr_cpu_ids)
 				lpfc_irq_set_aff(eqhdl, cpu_select);
@@ -11497,11 +11483,11 @@ lpfc_sli4_enable_msix(struct lpfc_hba *phba)
 						LPFC_CPU_FIRST_IRQ,
 						cpu);
 
-			/* Iterate to next offline or online cpu in numa_mask */
-			cpu = cpumask_next(cpu, numa_mask);
+			/* Iterate to next offline or online cpu in aff_mask */
+			cpu = cpumask_next(cpu, aff_mask);
 
-			/* Find next online cpu in numa_mask to set affinity */
-			cpu_select = lpfc_next_online_numa_cpu(numa_mask, cpu);
+			/* Find next online cpu in aff_mask to set affinity */
+			cpu_select = lpfc_next_online_cpu(aff_mask, cpu);
 		} else if (vectors == 1) {
 			cpu = cpumask_first(cpu_present_mask);
 			lpfc_assign_eq_map_info(phba, index, LPFC_CPU_FIRST_IRQ,
diff --git a/drivers/scsi/lpfc/lpfc_sli4.h b/drivers/scsi/lpfc/lpfc_sli4.h
index 8da7429e385a..4decb53d81c3 100644
--- a/drivers/scsi/lpfc/lpfc_sli4.h
+++ b/drivers/scsi/lpfc/lpfc_sli4.h
@@ -920,7 +920,7 @@ struct lpfc_sli4_hba {
 	struct lpfc_vector_map_info *cpu_map;
 	uint16_t num_possible_cpu;
 	uint16_t num_present_cpu;
-	struct cpumask numa_mask;
+	struct cpumask irq_aff_mask;
 	uint16_t curr_disp_cpu;
 	struct lpfc_eq_intr_info __percpu *eq_info;
 #ifdef CONFIG_SCSI_LPFC_DEBUG_FS
-- 
2.26.1

[PATCH 6/9] lpfc: Remove unnecessary lockdep_assert_held calls

[James Smart]

In an audit of lockdep calls in the driver, there are multiple lockdep
checks in successive calling layers. E.g. a routine checks, and then
calls a lower routine that also checks, and so on. Calling sequences
result in many redundant checks.

Refine the code to remove lower-level lockdep checks.
Update comments on the lock, correcting a few places where lock object
in comment was incorrect.

Signed-off-by: Dick Kennedy <dick.kennedy@broadcom.com>
Signed-off-by: James Smart <jsmart2021@gmail.com>
---
 drivers/scsi/lpfc/lpfc_hbadisc.c |  8 +++-----
 drivers/scsi/lpfc/lpfc_sli.c     | 33 +++++++++++++++-----------------
 2 files changed, 18 insertions(+), 23 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_hbadisc.c b/drivers/scsi/lpfc/lpfc_hbadisc.c
index 789eecbf32eb..a8f4ced0fc41 100644
--- a/drivers/scsi/lpfc/lpfc_hbadisc.c
+++ b/drivers/scsi/lpfc/lpfc_hbadisc.c
@@ -1356,14 +1356,14 @@ lpfc_vlan_id_match(uint16_t curr_vlan_id, uint16_t new_vlan_id)
 }
 
 /**
- * lpfc_update_fcf_record - Update driver fcf record
  * __lpfc_update_fcf_record_pri - update the lpfc_fcf_pri record.
  * @phba: pointer to lpfc hba data structure.
  * @fcf_index: Index for the lpfc_fcf_record.
  * @new_fcf_record: pointer to hba fcf record.
  *
  * This routine updates the driver FCF priority record from the new HBA FCF
- * record. This routine is called with the host lock held.
+ * record. The hbalock is asserted held in the code path calling this
+ * routine.
  **/
 static void
 __lpfc_update_fcf_record_pri(struct lpfc_hba *phba, uint16_t fcf_index,
@@ -1372,8 +1372,6 @@ __lpfc_update_fcf_record_pri(struct lpfc_hba *phba, uint16_t fcf_index,
 {
 	struct lpfc_fcf_pri *fcf_pri;
 
-	lockdep_assert_held(&phba->hbalock);
-
 	fcf_pri = &phba->fcf.fcf_pri[fcf_index];
 	fcf_pri->fcf_rec.fcf_index = fcf_index;
 	/* FCF record priority */
@@ -1451,7 +1449,7 @@ lpfc_copy_fcf_record(struct lpfc_fcf_rec *fcf_rec,
  *
  * This routine updates the driver FCF record from the new HBA FCF record
  * together with the address mode, vlan_id, and other informations. This
- * routine is called with the host lock held.
+ * routine is called with the hbalock held.
  **/
 static void
 __lpfc_update_fcf_record(struct lpfc_hba *phba, struct lpfc_fcf_rec *fcf_rec,
diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
index 9ce37560f4c0..df77e75b9f53 100644
--- a/drivers/scsi/lpfc/lpfc_sli.c
+++ b/drivers/scsi/lpfc/lpfc_sli.c
@@ -1248,8 +1248,8 @@ lpfc_sli_get_iocbq(struct lpfc_hba *phba)
  * @phba: Pointer to HBA context object.
  * @iocbq: Pointer to driver iocb object.
  *
- * This function is called with hbalock held to release driver
- * iocb object to the iocb pool. The iotag in the iocb object
+ * This function is called to release the driver iocb object
+ * to the iocb pool. The iotag in the iocb object
  * does not change for each use of the iocb object. This function
  * clears all other fields of the iocb object when it is freed.
  * The sqlq structure that holds the xritag and phys and virtual
@@ -1259,7 +1259,8 @@ lpfc_sli_get_iocbq(struct lpfc_hba *phba)
  * this IO was aborted then the sglq entry it put on the
  * lpfc_abts_els_sgl_list until the CQ_ABORTED_XRI is received. If the
  * IO has good status or fails for any other reason then the sglq
- * entry is added to the free list (lpfc_els_sgl_list).
+ * entry is added to the free list (lpfc_els_sgl_list). The hbalock is
+ *  asserted held in the code path calling this routine.
  **/
 static void
 __lpfc_sli_release_iocbq_s4(struct lpfc_hba *phba, struct lpfc_iocbq *iocbq)
@@ -1269,8 +1270,6 @@ __lpfc_sli_release_iocbq_s4(struct lpfc_hba *phba, struct lpfc_iocbq *iocbq)
 	unsigned long iflag = 0;
 	struct lpfc_sli_ring *pring;
 
-	lockdep_assert_held(&phba->hbalock);
-
 	if (iocbq->sli4_xritag == NO_XRI)
 		sglq = NULL;
 	else
@@ -1333,18 +1332,17 @@ __lpfc_sli_release_iocbq_s4(struct lpfc_hba *phba, struct lpfc_iocbq *iocbq)
  * @phba: Pointer to HBA context object.
  * @iocbq: Pointer to driver iocb object.
  *
- * This function is called with hbalock held to release driver
- * iocb object to the iocb pool. The iotag in the iocb object
- * does not change for each use of the iocb object. This function
- * clears all other fields of the iocb object when it is freed.
+ * This function is called to release the driver iocb object to the
+ * iocb pool. The iotag in the iocb object does not change for each
+ * use of the iocb object. This function clears all other fields of
+ * the iocb object when it is freed. The hbalock is asserted held in
+ * the code path calling this routine.
  **/
 static void
 __lpfc_sli_release_iocbq_s3(struct lpfc_hba *phba, struct lpfc_iocbq *iocbq)
 {
 	size_t start_clean = offsetof(struct lpfc_iocbq, iocb);
 
-	lockdep_assert_held(&phba->hbalock);
-
 	/*
 	 * Clean all volatile data fields, preserve iotag and node struct.
 	 */
@@ -1789,17 +1787,17 @@ lpfc_sli_next_iotag(struct lpfc_hba *phba, struct lpfc_iocbq *iocbq)
  * @nextiocb: Pointer to driver iocb object which need to be
  *            posted to firmware.
  *
- * This function is called with hbalock held to post a new iocb to
- * the firmware. This function copies the new iocb to ring iocb slot and
- * updates the ring pointers. It adds the new iocb to txcmplq if there is
+ * This function is called to post a new iocb to the firmware. This
+ * function copies the new iocb to ring iocb slot and updates the
+ * ring pointers. It adds the new iocb to txcmplq if there is
  * a completion call back for this iocb else the function will free the
- * iocb object.
+ * iocb object.  The hbalock is asserted held in the code path calling
+ * this routine.
  **/
 static void
 lpfc_sli_submit_iocb(struct lpfc_hba *phba, struct lpfc_sli_ring *pring,
 		IOCB_t *iocb, struct lpfc_iocbq *nextiocb)
 {
-	lockdep_assert_held(&phba->hbalock);
 	/*
 	 * Set up an iotag
 	 */
@@ -11170,6 +11168,7 @@ lpfc_ignore_els_cmpl(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
  * request, this function issues abort out unconditionally. This function is
  * called with hbalock held. The function returns 0 when it fails due to
  * memory allocation failure or when the command iocb is an abort request.
+ * The hbalock is asserted held in the code path calling this routine.
  **/
 static int
 lpfc_sli_abort_iotag_issue(struct lpfc_hba *phba, struct lpfc_sli_ring *pring,
@@ -11183,8 +11182,6 @@ lpfc_sli_abort_iotag_issue(struct lpfc_hba *phba, struct lpfc_sli_ring *pring,
 	unsigned long iflags;
 	struct lpfc_nodelist *ndlp;
 
-	lockdep_assert_held(&phba->hbalock);
-
 	/*
 	 * There are certain command types we don't want to abort.  And we
 	 * don't want to abort commands that are already in the process of
-- 
2.26.1

[PATCH 7/9] lpfc: Fix noderef and address space warnings

[James Smart]

Running make C=1 M=drivers/scsi/lpfc triggers sparse warnings

Correct the code generating the following errors:
- Incompatible address space assignment without proper conversion.
- Deference of usespace and per cpu pointers.

Signed-off-by: Dick Kennedy <dick.kennedy@broadcom.com>
Signed-off-by: James Smart <jsmart2021@gmail.com>
---
 drivers/scsi/lpfc/lpfc_debugfs.c | 3 ++-
 drivers/scsi/lpfc/lpfc_mbox.c    | 3 ++-
 drivers/scsi/lpfc/lpfc_sli.c     | 8 ++++----
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_debugfs.c b/drivers/scsi/lpfc/lpfc_debugfs.c
index 8a6e02aa553f..24d946ef8609 100644
--- a/drivers/scsi/lpfc/lpfc_debugfs.c
+++ b/drivers/scsi/lpfc/lpfc_debugfs.c
@@ -2436,7 +2436,8 @@ lpfc_debugfs_dif_err_write(struct file *file, const char __user *buf,
 		return 0;
 
 	if (dent == phba->debug_InjErrLBA) {
-		if ((buf[0] == 'o') && (buf[1] == 'f') && (buf[2] == 'f'))
+		if ((dstbuf[0] == 'o') && (dstbuf[1] == 'f') &&
+		    (dstbuf[2] == 'f'))
 			tmp = (uint64_t)(-1);
 	}
 
diff --git a/drivers/scsi/lpfc/lpfc_mbox.c b/drivers/scsi/lpfc/lpfc_mbox.c
index e35b52b66d6c..e34e0f11bfdd 100644
--- a/drivers/scsi/lpfc/lpfc_mbox.c
+++ b/drivers/scsi/lpfc/lpfc_mbox.c
@@ -1378,7 +1378,8 @@ lpfc_config_port(struct lpfc_hba *phba, LPFC_MBOXQ_t *pmb)
 	 */
 
 	if (phba->cfg_hostmem_hgp && phba->sli_rev != 3) {
-		phba->host_gp = &phba->mbox->us.s2.host[0];
+		phba->host_gp = (struct lpfc_hgp __iomem *)
+				 &phba->mbox->us.s2.host[0];
 		phba->hbq_put = NULL;
 		offset = (uint8_t *)&phba->mbox->us.s2.host -
 			(uint8_t *)phba->slim2p.virt;
diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
index df77e75b9f53..0c9844cac3aa 100644
--- a/drivers/scsi/lpfc/lpfc_sli.c
+++ b/drivers/scsi/lpfc/lpfc_sli.c
@@ -14272,7 +14272,6 @@ lpfc_sli4_hba_intr_handler(int irq, void *dev_id)
 	int ecount = 0;
 	int hba_eqidx;
 	struct lpfc_eq_intr_info *eqi;
-	uint32_t icnt;
 
 	/* Get the driver's phba structure from the dev_id */
 	hba_eq_hdl = (struct lpfc_hba_eq_hdl *)dev_id;
@@ -14300,11 +14299,12 @@ lpfc_sli4_hba_intr_handler(int irq, void *dev_id)
 		return IRQ_NONE;
 	}
 
-	eqi = phba->sli4_hba.eq_info;
-	icnt = this_cpu_inc_return(eqi->icnt);
+	eqi = this_cpu_ptr(phba->sli4_hba.eq_info);
+	eqi->icnt++;
+
 	fpeq->last_cpu = raw_smp_processor_id();
 
-	if (icnt > LPFC_EQD_ISR_TRIGGER &&
+	if (eqi->icnt > LPFC_EQD_ISR_TRIGGER &&
 	    fpeq->q_flag & HBA_EQ_DELAY_CHK &&
 	    phba->cfg_auto_imax &&
 	    fpeq->q_mode != LPFC_MAX_AUTO_EQ_DELAY &&
-- 
2.26.1

[PATCH 8/9] lpfc: Fix MDS Diagnostic Enablement definition

[James Smart]

The MDS diagnostic enablement bit for the adapter interface is incorrect
in the driver header.

Correct the bit position for the SET_FEATURE MDS bit.

Signed-off-by: Dick Kennedy <dick.kennedy@broadcom.com>
Signed-off-by: James Smart <jsmart2021@gmail.com>
---
 drivers/scsi/lpfc/lpfc_hw4.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/lpfc/lpfc_hw4.h b/drivers/scsi/lpfc/lpfc_hw4.h
index 10c5d1c3122e..6dfff0376547 100644
--- a/drivers/scsi/lpfc/lpfc_hw4.h
+++ b/drivers/scsi/lpfc/lpfc_hw4.h
@@ -3541,7 +3541,7 @@ struct lpfc_mbx_set_feature {
 #define lpfc_mbx_set_feature_UER_SHIFT  0
 #define lpfc_mbx_set_feature_UER_MASK   0x00000001
 #define lpfc_mbx_set_feature_UER_WORD   word6
-#define lpfc_mbx_set_feature_mds_SHIFT  0
+#define lpfc_mbx_set_feature_mds_SHIFT  2
 #define lpfc_mbx_set_feature_mds_MASK   0x00000001
 #define lpfc_mbx_set_feature_mds_WORD   word6
 #define lpfc_mbx_set_feature_mds_deep_loopbk_SHIFT  1
-- 
2.26.1

[PATCH 9/9] lpfc: Update lpfc version to 12.8.0.1

[James Smart]

Update lpfc version to 12.8.0.1

Signed-off-by: Dick Kennedy <dick.kennedy@broadcom.com>
Signed-off-by: James Smart <jsmart2021@gmail.com>
---
 drivers/scsi/lpfc/lpfc_version.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/lpfc/lpfc_version.h b/drivers/scsi/lpfc/lpfc_version.h
index ca40c47cfbe0..ab0bc26c098d 100644
--- a/drivers/scsi/lpfc/lpfc_version.h
+++ b/drivers/scsi/lpfc/lpfc_version.h
@@ -20,7 +20,7 @@
  * included with this package.                                     *
  *******************************************************************/
 
-#define LPFC_DRIVER_VERSION "12.8.0.0"
+#define LPFC_DRIVER_VERSION "12.8.0.1"
 #define LPFC_DRIVER_NAME		"lpfc"
 
 /* Used for SLI 2/3 */
-- 
2.26.1

Re: [PATCH 1/9] lpfc: Synchronize NVME transport and lpfc driver devloss_tmo

[James Smart]

On 5/1/2020 2:43 PM, James Smart wrote:
> The driver is not passing it's devloss tmo to the nvme-fc transport when
> registering the remote port. Thus devloss tmo for the nvme-fc remote port
> will be set to the transport's default. This causes driver actions to be
> out of sync with transport actions and out of sync with scsi actions for
> perhaps the same remote port.
>
> This is especially notable in the following scenario: while remote port
> is attached, devloss is changed globally for lpfc remote ports via lpfc
> sysfs parameter. lpfc ties this change in with nvme-fc transport. If the
> device disconnects long enough for devloss to expire thus the existing
> remote port is deleted, then the remote port is re-discovered, the newly
> created remote port will end up set at the transport default, not lpfc's
> value.
>
> Fix by setting devloss tmo value when registering the remote port.
>
> Signed-off-by: Dick Kennedy <dick.kennedy@broadcom.com>
> Signed-off-by: James Smart <jsmart2021@gmail.com>
> ---
>   drivers/scsi/lpfc/lpfc_nvme.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/drivers/scsi/lpfc/lpfc_nvme.c b/drivers/scsi/lpfc/lpfc_nvme.c
> index 12d2b2775773..43df08aeecf1 100644
> --- a/drivers/scsi/lpfc/lpfc_nvme.c
> +++ b/drivers/scsi/lpfc/lpfc_nvme.c
> @@ -2296,6 +2296,7 @@ lpfc_nvme_register_port(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
>   
>   	rpinfo.port_name = wwn_to_u64(ndlp->nlp_portname.u.wwn);
>   	rpinfo.node_name = wwn_to_u64(ndlp->nlp_nodename.u.wwn);
> +	rpinfo.dev_loss_tmo = vport->cfg_devloss_tmo;
>   
>   	spin_lock_irq(&vport->phba->hbalock);
>   	oldrport = lpfc_ndlp_get_nrport(ndlp);

Please drop this patch from this set.  Christoph is having me change a 
patch submitted to the nvme tree that will require this change and one 
other lpfc modification. Easier to manage if this patch merges via the 
nvme tree rather than the scsi tree.

-- james

Re: [PATCH 1/9] lpfc: Synchronize NVME transport and lpfc driver devloss_tmo

[Hannes Reinecke]

On 5/1/20 11:43 PM, James Smart wrote:
> The driver is not passing it's devloss tmo to the nvme-fc transport when
> registering the remote port. Thus devloss tmo for the nvme-fc remote port
> will be set to the transport's default. This causes driver actions to be
> out of sync with transport actions and out of sync with scsi actions for
> perhaps the same remote port.
> 
> This is especially notable in the following scenario: while remote port
> is attached, devloss is changed globally for lpfc remote ports via lpfc
> sysfs parameter. lpfc ties this change in with nvme-fc transport. If the
> device disconnects long enough for devloss to expire thus the existing
> remote port is deleted, then the remote port is re-discovered, the newly
> created remote port will end up set at the transport default, not lpfc's
> value.
> 
> Fix by setting devloss tmo value when registering the remote port.
> 
> Signed-off-by: Dick Kennedy <dick.kennedy@broadcom.com>
> Signed-off-by: James Smart <jsmart2021@gmail.com>
> ---
>   drivers/scsi/lpfc/lpfc_nvme.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/scsi/lpfc/lpfc_nvme.c b/drivers/scsi/lpfc/lpfc_nvme.c
> index 12d2b2775773..43df08aeecf1 100644
> --- a/drivers/scsi/lpfc/lpfc_nvme.c
> +++ b/drivers/scsi/lpfc/lpfc_nvme.c
> @@ -2296,6 +2296,7 @@ lpfc_nvme_register_port(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
>   
>   	rpinfo.port_name = wwn_to_u64(ndlp->nlp_portname.u.wwn);
>   	rpinfo.node_name = wwn_to_u64(ndlp->nlp_nodename.u.wwn);
> +	rpinfo.dev_loss_tmo = vport->cfg_devloss_tmo;
>   
>   	spin_lock_irq(&vport->phba->hbalock);
>   	oldrport = lpfc_ndlp_get_nrport(ndlp);
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke            Teamlead Storage & Networking
hare@suse.de                               +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

Re: [PATCH 2/9] lpfc: Maintain atomic consistency of queue_claimed flag

[Hannes Reinecke]

On 5/1/20 11:43 PM, James Smart wrote:
> A previous change introduced the atomic use of queue_claimed flag for
> eq's and cq's.  The code works fine, but the clearing of the
> queue_claimed flag is not atomic.
> 
> Change queue_claimed = 0 into xchg(&queue_claimed, 0) to be consistent
> for change under atomicity.
> 
> Signed-off-by: Dick Kennedy <dick.kennedy@broadcom.com>
> Signed-off-by: James Smart <jsmart2021@gmail.com>
> ---
>   drivers/scsi/lpfc/lpfc_sli.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
> index b6fb665e6ec4..9ce37560f4c0 100644
> --- a/drivers/scsi/lpfc/lpfc_sli.c
> +++ b/drivers/scsi/lpfc/lpfc_sli.c
> @@ -538,7 +538,7 @@ lpfc_sli4_process_eq(struct lpfc_hba *phba, struct lpfc_queue *eq,
>   	if (count > eq->EQ_max_eqe)
>   		eq->EQ_max_eqe = count;
>   
> -	eq->queue_claimed = 0;
> +	xchg(&eq->queue_claimed, 0);
>   
>   rearm_and_exit:
>   	/* Always clear the EQ. */
> @@ -13694,7 +13694,7 @@ __lpfc_sli4_process_cq(struct lpfc_hba *phba, struct lpfc_queue *cq,
>   				"0369 No entry from completion queue "
>   				"qid=%d\n", cq->queue_id);
>   
> -	cq->queue_claimed = 0;
> +	xchg(&cq->queue_claimed, 0);
>   
>   rearm_and_exit:
>   	phba->sli4_hba.sli4_write_cq_db(phba, cq, consumed,
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke            Teamlead Storage & Networking
hare@suse.de                               +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

Re: [PATCH 3/9] lpfc: Remove re-binding of nvme rport during registration

[Hannes Reinecke]

On 5/1/20 11:43 PM, James Smart wrote:
> The lldd rebinds the ndlp with rport during a nvme rport registration
> (va nvme_fc_register_remoteport). If rport & ndlp pointers are same as
> the previous one, the lldd will re-use the ndlp and rport association
> without re-iniitalization. This assumption is incorrect. The lldd should
> be ignorant of whether the returned rport pointer is new or not, and
> should always assume it is new.
> 
> Remove the re-binding code, always assumes that rport pointer received
> from transport is a new pointer.
> 
> Signed-off-by: Dick Kennedy <dick.kennedy@broadcom.com>
> Signed-off-by: James Smart <jsmart2021@gmail.com>
> ---
>   drivers/scsi/lpfc/lpfc_nvme.c | 32 --------------------------------
>   1 file changed, 32 deletions(-)
> 
> diff --git a/drivers/scsi/lpfc/lpfc_nvme.c b/drivers/scsi/lpfc/lpfc_nvme.c
> index 43df08aeecf1..3121cf37a572 100644
> --- a/drivers/scsi/lpfc/lpfc_nvme.c
> +++ b/drivers/scsi/lpfc/lpfc_nvme.c
> @@ -2322,38 +2322,6 @@ lpfc_nvme_register_port(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
>   		spin_unlock_irq(&vport->phba->hbalock);
>   		rport = remote_port->private;
>   		if (oldrport) {
> -			/* New remoteport record does not guarantee valid
> -			 * host private memory area.
> -			 */
> -			if (oldrport == remote_port->private) {
> -				/* Same remoteport - ndlp should match.
> -				 * Just reuse.
> -				 */
> -				lpfc_printf_vlog(ndlp->vport, KERN_INFO,
> -						 LOG_NVME_DISC,
> -						 "6014 Rebind lport to current "
> -						 "remoteport x%px wwpn 0x%llx, "
> -						 "Data: x%x x%x x%px x%px x%x "
> -						 " x%06x\n",
> -						 remote_port,
> -						 remote_port->port_name,
> -						 remote_port->port_id,
> -						 remote_port->port_role,
> -						 oldrport->ndlp,
> -						 ndlp,
> -						 ndlp->nlp_type,
> -						 ndlp->nlp_DID);
> -
> -				/* It's a complete rebind only if the driver
> -				 * is registering with the same ndlp. Otherwise
> -				 * the driver likely executed a node swap
> -				 * prior to this registration and the ndlp to
> -				 * remoteport binding needs to be redone.
> -				 */
> -				if (prev_ndlp == ndlp)
> -					return 0;
> -
> -			}
>   
>   			/* Sever the ndlp<->rport association
>   			 * before dropping the ndlp ref from
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke            Teamlead Storage & Networking
hare@suse.de                               +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

Re: [PATCH 4/9] lpfc: Fix negation of else clause in lpfc_prep_node_fc4type

[Hannes Reinecke]

On 5/1/20 11:43 PM, James Smart wrote:
> Implementation of a previous patch added a condition to an if check
> that always end up with the if test being true. Execution of the else
> clause was inadvertantly negated.  The additional condition check was
> incorrect and unnecessary after the other modifications had been done
> in that patch.
> 
> Remove the check from the if series.
> 
> Fixes: b95b21193c85 ("scsi: lpfc: Fix loss of remote port after devloss due to lack of RPIs")
> Cc: <stable@vger.kernel.org> # v5.4+
> Signed-off-by: Dick Kennedy <dick.kennedy@broadcom.com>
> Signed-off-by: James Smart <jsmart2021@gmail.com>
> ---
>   drivers/scsi/lpfc/lpfc_ct.c | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/scsi/lpfc/lpfc_ct.c b/drivers/scsi/lpfc/lpfc_ct.c
> index 2aa578d20f8c..7fce73c39c1c 100644
> --- a/drivers/scsi/lpfc/lpfc_ct.c
> +++ b/drivers/scsi/lpfc/lpfc_ct.c
> @@ -462,7 +462,6 @@ lpfc_prep_node_fc4type(struct lpfc_vport *vport, uint32_t Did, uint8_t fc4_type)
>   	struct lpfc_nodelist *ndlp;
>   
>   	if ((vport->port_type != LPFC_NPIV_PORT) ||
> -	    (fc4_type == FC_TYPE_FCP) ||
>   	    !(vport->ct_flags & FC_CT_RFF_ID) || !vport->cfg_restrict_login) {
>   
>   		ndlp = lpfc_setup_disc_node(vport, Did);
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke            Teamlead Storage & Networking
hare@suse.de                               +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

Re: [PATCH 5/9] lpfc: Change default queue allocation for reduced memory consumption

[Hannes Reinecke]

On 5/1/20 11:43 PM, James Smart wrote:
> By default, the driver attempts to allocate a hdwq per logical cpu in
> order to provide good cpu affinity. Some systems have extremely high cpu
> counts and this can significantly raise memory consumption.
> 
> In testing on x86 platforms (non-AMD) it is found that sharing of a hdwq
> by a physical cpu and it's HT cpu can occur with little performance
> degredation. By sharing the hdwq count can be halved, significantly
> reducing the memory overhead.
> 
> Change the default behavior of the driver on non-AMD x86 platforms to
> share a hdwq by the cpu and its HT cpu.
> 
> Signed-off-by: Dick Kennedy <dick.kennedy@broadcom.com>
> Signed-off-by: James Smart <jsmart2021@gmail.com>
> ---
>   drivers/scsi/lpfc/lpfc.h      |  23 ++++++--
>   drivers/scsi/lpfc/lpfc_attr.c | 106 +++++++++++++++++++++++++++-------
>   drivers/scsi/lpfc/lpfc_init.c |  82 +++++++++++---------------
>   drivers/scsi/lpfc/lpfc_sli4.h |   2 +-
>   4 files changed, 137 insertions(+), 76 deletions(-)
> I do assume that you've checked that this matches with blk-mq queue 
layout, right?

Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke            Teamlead Storage & Networking
hare@suse.de                               +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

Re: [PATCH 6/9] lpfc: Remove unnecessary lockdep_assert_held calls

[Hannes Reinecke]

On 5/1/20 11:43 PM, James Smart wrote:
> In an audit of lockdep calls in the driver, there are multiple lockdep
> checks in successive calling layers. E.g. a routine checks, and then
> calls a lower routine that also checks, and so on. Calling sequences
> result in many redundant checks.
> 
> Refine the code to remove lower-level lockdep checks.
> Update comments on the lock, correcting a few places where lock object
> in comment was incorrect.
> 
> Signed-off-by: Dick Kennedy <dick.kennedy@broadcom.com>
> Signed-off-by: James Smart <jsmart2021@gmail.com>
> ---
>   drivers/scsi/lpfc/lpfc_hbadisc.c |  8 +++-----
>   drivers/scsi/lpfc/lpfc_sli.c     | 33 +++++++++++++++-----------------
>   2 files changed, 18 insertions(+), 23 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke            Teamlead Storage & Networking
hare@suse.de                               +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

Re: [PATCH 7/9] lpfc: Fix noderef and address space warnings

[Hannes Reinecke]

On 5/1/20 11:43 PM, James Smart wrote:
> Running make C=1 M=drivers/scsi/lpfc triggers sparse warnings
> 
> Correct the code generating the following errors:
> - Incompatible address space assignment without proper conversion.
> - Deference of usespace and per cpu pointers.
> 
> Signed-off-by: Dick Kennedy <dick.kennedy@broadcom.com>
> Signed-off-by: James Smart <jsmart2021@gmail.com>
> ---
>   drivers/scsi/lpfc/lpfc_debugfs.c | 3 ++-
>   drivers/scsi/lpfc/lpfc_mbox.c    | 3 ++-
>   drivers/scsi/lpfc/lpfc_sli.c     | 8 ++++----
>   3 files changed, 8 insertions(+), 6 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke            Teamlead Storage & Networking
hare@suse.de                               +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

Re: [PATCH 8/9] lpfc: Fix MDS Diagnostic Enablement definition

[Hannes Reinecke]

On 5/1/20 11:43 PM, James Smart wrote:
> The MDS diagnostic enablement bit for the adapter interface is incorrect
> in the driver header.
> 
> Correct the bit position for the SET_FEATURE MDS bit.
> 
> Signed-off-by: Dick Kennedy <dick.kennedy@broadcom.com>
> Signed-off-by: James Smart <jsmart2021@gmail.com>
> ---
>   drivers/scsi/lpfc/lpfc_hw4.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/lpfc/lpfc_hw4.h b/drivers/scsi/lpfc/lpfc_hw4.h
> index 10c5d1c3122e..6dfff0376547 100644
> --- a/drivers/scsi/lpfc/lpfc_hw4.h
> +++ b/drivers/scsi/lpfc/lpfc_hw4.h
> @@ -3541,7 +3541,7 @@ struct lpfc_mbx_set_feature {
>   #define lpfc_mbx_set_feature_UER_SHIFT  0
>   #define lpfc_mbx_set_feature_UER_MASK   0x00000001
>   #define lpfc_mbx_set_feature_UER_WORD   word6
> -#define lpfc_mbx_set_feature_mds_SHIFT  0
> +#define lpfc_mbx_set_feature_mds_SHIFT  2
>   #define lpfc_mbx_set_feature_mds_MASK   0x00000001
>   #define lpfc_mbx_set_feature_mds_WORD   word6
>   #define lpfc_mbx_set_feature_mds_deep_loopbk_SHIFT  1
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke            Teamlead Storage & Networking
hare@suse.de                               +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

Re: [PATCH 9/9] lpfc: Update lpfc version to 12.8.0.1

[Hannes Reinecke]

On 5/1/20 11:43 PM, James Smart wrote:
> Update lpfc version to 12.8.0.1
> 
> Signed-off-by: Dick Kennedy <dick.kennedy@broadcom.com>
> Signed-off-by: James Smart <jsmart2021@gmail.com>
> ---
>   drivers/scsi/lpfc/lpfc_version.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/lpfc/lpfc_version.h b/drivers/scsi/lpfc/lpfc_version.h
> index ca40c47cfbe0..ab0bc26c098d 100644
> --- a/drivers/scsi/lpfc/lpfc_version.h
> +++ b/drivers/scsi/lpfc/lpfc_version.h
> @@ -20,7 +20,7 @@
>    * included with this package.                                     *
>    *******************************************************************/
>   
> -#define LPFC_DRIVER_VERSION "12.8.0.0"
> +#define LPFC_DRIVER_VERSION "12.8.0.1"
>   #define LPFC_DRIVER_NAME		"lpfc"
>   
>   /* Used for SLI 2/3 */
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes

-- 
Dr. Hannes Reinecke            Teamlead Storage & Networking
hare@suse.de                               +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

Re: [PATCH 0/9] lpfc: Update lpfc to revision 12.8.0.1

[Martin K. Petersen]

On Fri, 1 May 2020 14:43:01 -0700, James Smart wrote:

> Update lpfc to revision 12.8.0.1
> 
> Patch set contains several small fixes, code cleanups, a change to
> default number of hdwqs at init, and a sync of devloss with the
> nvme-fc transport.
> 
> The patches were cut against Martin's 5.8/scsi-queue tree
> 
> [...]

Applied to 5.8/scsi-queue, thanks!

[2/9] scsi: lpfc: Maintain atomic consistency of queue_claimed flag
      https://git.kernel.org/mkp/scsi/c/164ba8d2df66
[3/9] scsi: lpfc: Remove re-binding of nvme rport during registration
      https://git.kernel.org/mkp/scsi/c/b98214f6070e
[4/9] scsi: lpfc: Fix negation of else clause in lpfc_prep_node_fc4type
      https://git.kernel.org/mkp/scsi/c/f809da6db68a
[5/9] scsi: lpfc: Change default queue allocation for reduced memory consumption
      https://git.kernel.org/mkp/scsi/c/3048e3e805e3
[6/9] scsi: lpfc: Remove unnecessary lockdep_assert_held calls
      https://git.kernel.org/mkp/scsi/c/88acb4d9ff98
[7/9] scsi: lpfc: Fix noderef and address space warnings
      https://git.kernel.org/mkp/scsi/c/a7fc071ab56e
[8/9] scsi: lpfc: Fix MDS Diagnostic Enablement definition
      https://git.kernel.org/mkp/scsi/c/8cdc5a223ed0
[9/9] scsi: lpfc: Update lpfc version to 12.8.0.1
      https://git.kernel.org/mkp/scsi/c/29022b61307f

-- 
Martin K. Petersen	Oracle Linux Engineering