[PATCH 1/2] lpfc: support for CPU phys_id and core_id on PowerPC64

[PATCH 1/2] lpfc: support for CPU phys_id and core_id on PowerPC64

[Mauricio Faria de Oliveira]

This commit uses the macros 'topology_physical_package_id' and
'topology_core_id' from <linux/topology.h>, which are defined in
arch/powerpc/include/asm/topology.h for CONFIG_PPC64 && CONFIG_SMP.

Also, change the initial value for min_phys_id from 0xff to INT_MAX
(the numbers may increment with large steps on some systems).

While in there, include the CPU number in the debug message, which
helps reading it on systems with many CPUs.

This depends on commit 'powerpc: export cpu_to_core_id()' (submitted
to the linuxppc-dev mailing list). Tested on next-20160601 w/ commit.

Signed-off-by: Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com>
---
 drivers/scsi/lpfc/lpfc_init.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c
index b43f7ac..03d1946 100644
--- a/drivers/scsi/lpfc/lpfc_init.c
+++ b/drivers/scsi/lpfc/lpfc_init.c
@@ -34,6 +34,8 @@
 #include <linux/firmware.h>
 #include <linux/miscdevice.h>
 #include <linux/percpu.h>
+#include <linux/kernel.h>
+#include <linux/topology.h>
 
 #include <scsi/scsi.h>
 #include <scsi/scsi_device.h>
@@ -8718,7 +8720,7 @@ lpfc_sli4_set_affinity(struct lpfc_hba *phba, int vectors)
 		phba->sli4_hba.num_present_cpu));
 
 	max_phys_id = 0;
-	min_phys_id = 0xff;
+	min_phys_id = INT_MAX;
 	phys_id = 0;
 	num_io_channel = 0;
 	first_cpu = LPFC_VECTOR_MAP_EMPTY;
@@ -8730,6 +8732,9 @@ lpfc_sli4_set_affinity(struct lpfc_hba *phba, int vectors)
 		cpuinfo = &cpu_data(cpu);
 		cpup->phys_id = cpuinfo->phys_proc_id;
 		cpup->core_id = cpuinfo->cpu_core_id;
+#elif defined(CONFIG_PPC64) && defined(CONFIG_SMP)
+		cpup->phys_id = topology_physical_package_id(cpu);
+		cpup->core_id = topology_core_id(cpu);
 #else
 		/* No distinction between CPUs for other platforms */
 		cpup->phys_id = 0;
@@ -8737,8 +8742,8 @@ lpfc_sli4_set_affinity(struct lpfc_hba *phba, int vectors)
 #endif
 
 		lpfc_printf_log(phba, KERN_INFO, LOG_INIT,
-				"3328 CPU physid %d coreid %d\n",
-				cpup->phys_id, cpup->core_id);
+				"3328 CPU %d physid %d coreid %d\n",
+				cpu, cpup->phys_id, cpup->core_id);
 
 		if (cpup->phys_id > max_phys_id)
 			max_phys_id = cpup->phys_id;
-- 
1.8.3.1

[PATCH 2/2] lpfc: add option for lpfc_fcp_io_sched (LPFC_FCP_SCHED_BY_CPU_CORE)

[Mauricio Faria de Oliveira]

This commit introduces the new value 2 for parameter 'lpfc_fcp_io_sched',
which associates work queues (or IO channels) by core, that is, all CPUs
(or threads) of a particular core are assigned the same IO channel.

The IO channels are assigned to each core in a round-robin fashion.

Tested on next-20160601.

Signed-off-by: Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com>
---
 drivers/scsi/lpfc/lpfc_attr.c |  8 +++++--
 drivers/scsi/lpfc/lpfc_hw4.h  |  1 +
 drivers/scsi/lpfc/lpfc_init.c | 54 ++++++++++++++++++++++++++++++++++++++++++-
 drivers/scsi/lpfc/lpfc_scsi.c |  3 ++-
 4 files changed, 62 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_attr.c b/drivers/scsi/lpfc/lpfc_attr.c
index cfec2ec..f8db2ad 100644
--- a/drivers/scsi/lpfc/lpfc_attr.c
+++ b/drivers/scsi/lpfc/lpfc_attr.c
@@ -4517,12 +4517,16 @@ LPFC_ATTR_R(ack0, 0, 0, 1, "Enable ACK0 support");
 # For [0], FCP commands are issued to Work Queues ina round robin fashion.
 # For [1], FCP commands are issued to a Work Queue associated with the
 #          current CPU.
+# For [2], FCP commands are issued to a Work Queue associated with the
+#          current CPU core (same Work Queue for all CPUs of a core).
 # It would be set to 1 by the driver if it's able to set up cpu affinity
 # for FCP I/Os through Work Queue associated with the current CPU. Otherwise,
 # roundrobin scheduling of FCP I/Os through WQs will be used.
+# It would remain set to 2 by the driver, likewise, if 2 was requested.
 */
-LPFC_ATTR_RW(fcp_io_sched, 0, 0, 1, "Determine scheduling algorithm for "
-		"issuing commands [0] - Round Robin, [1] - Current CPU");
+LPFC_ATTR_RW(fcp_io_sched, 0, 0, 2, "Determine scheduling algorithm for "
+		"issuing commands [0] - Round Robin, [1] - Current CPU, "
+		"[2] - Current CPU core");
 
 /*
 # lpfc_fcp2_no_tgt_reset: Determine bus reset behavior
diff --git a/drivers/scsi/lpfc/lpfc_hw4.h b/drivers/scsi/lpfc/lpfc_hw4.h
index 0c7070b..3c63063 100644
--- a/drivers/scsi/lpfc/lpfc_hw4.h
+++ b/drivers/scsi/lpfc/lpfc_hw4.h
@@ -191,6 +191,7 @@ struct lpfc_sli_intf {
 /* Algrithmns for scheduling FCP commands to WQs */
 #define	LPFC_FCP_SCHED_ROUND_ROBIN	0
 #define	LPFC_FCP_SCHED_BY_CPU		1
+#define	LPFC_FCP_SCHED_BY_CPU_CORE	2
 
 /* Delay Multiplier constant */
 #define LPFC_DMULT_CONST       651042
diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c
index 03d1946..917351c 100644
--- a/drivers/scsi/lpfc/lpfc_init.c
+++ b/drivers/scsi/lpfc/lpfc_init.c
@@ -8703,6 +8703,7 @@ lpfc_sli4_set_affinity(struct lpfc_hba *phba, int vectors)
 {
 	int i, idx, saved_chann, used_chann, cpu, phys_id;
 	int max_phys_id, min_phys_id;
+	int max_core_id, min_core_id, core_id, prev_core_id;
 	int num_io_channel, first_cpu, chan;
 	struct lpfc_vector_map_info *cpup;
 #ifdef CONFIG_X86
@@ -8722,6 +8723,9 @@ lpfc_sli4_set_affinity(struct lpfc_hba *phba, int vectors)
 	max_phys_id = 0;
 	min_phys_id = INT_MAX;
 	phys_id = 0;
+	max_core_id = 0;
+	min_core_id = INT_MAX;
+	core_id = 0;
 	num_io_channel = 0;
 	first_cpu = LPFC_VECTOR_MAP_EMPTY;
 
@@ -8749,6 +8753,12 @@ lpfc_sli4_set_affinity(struct lpfc_hba *phba, int vectors)
 			max_phys_id = cpup->phys_id;
 		if (cpup->phys_id < min_phys_id)
 			min_phys_id = cpup->phys_id;
+
+		if (cpup->core_id > max_core_id)
+			max_core_id = cpup->core_id;
+		if (cpup->core_id < min_core_id)
+			min_core_id = cpup->core_id;
+
 		cpup++;
 	}
 
@@ -8820,6 +8830,46 @@ found:
 	}
 
 	/*
+	 * With lpfc_fcp_io_sched per core, associate IO channels with CPUs
+	 * based only on the core numbers instead of individual CPU numbers,
+	 * and ignore/overwrite already assigned values (from MSI-x vectors).
+	 */
+	if (phba->cfg_fcp_io_sched == LPFC_FCP_SCHED_BY_CPU_CORE) {
+
+		/* The IO channel used by a core */
+		chan = -1;
+
+		/* For each core, assign its (sequential) CPUs the same IO channel */
+		prev_core_id = -1;
+		for (core_id = min_core_id; core_id <= max_core_id; core_id++) {
+
+			cpup = phba->sli4_hba.cpu_map;
+			for (cpu = 0; cpu < phba->sli4_hba.num_present_cpu; cpu++, cpup++) {
+
+				if (cpup->core_id != core_id)
+					continue;
+
+				/* Round-robin on different cores */
+				if (core_id != prev_core_id) {
+					prev_core_id = core_id;
+					chan = (chan + 1) % phba->cfg_fcp_io_channel;
+				}
+
+				cpup->channel_id = chan;
+
+				/* Don't count CPUs w/ IRQ affinity hint (already counted) */
+				if (cpup->irq == LPFC_VECTOR_MAP_EMPTY)
+					num_io_channel++;
+
+				lpfc_printf_log(phba, KERN_INFO, LOG_INIT,
+						"3336 Set IO_CHANN CPU %d channel %d coreid %d\n",
+						cpu, cpup->channel_id, cpup->core_id);
+			}
+		}
+		goto out_fcp_io_sched_per_core;
+	}
+
+	/*
 	 * Finally fill in the IO channel for any remaining CPUs.
 	 * At this point, all IO channels have been assigned to a specific
 	 * MSIx vector, mapped to a specific CPU.
@@ -8891,6 +8941,7 @@ out:
 		}
 	}
 
+out_fcp_io_sched_per_core:
 	if (phba->sli4_hba.num_online_cpu != phba->sli4_hba.num_present_cpu) {
 		cpup = phba->sli4_hba.cpu_map;
 		for (idx = 0; idx < phba->sli4_hba.num_present_cpu; idx++) {
@@ -8916,7 +8967,8 @@ out:
 				vectors);
 
 	/* Enable using cpu affinity for scheduling */
-	phba->cfg_fcp_io_sched = LPFC_FCP_SCHED_BY_CPU;
+	if (phba->cfg_fcp_io_sched != LPFC_FCP_SCHED_BY_CPU_CORE)
+		phba->cfg_fcp_io_sched = LPFC_FCP_SCHED_BY_CPU;
 	return 1;
 }
 
diff --git a/drivers/scsi/lpfc/lpfc_scsi.c b/drivers/scsi/lpfc/lpfc_scsi.c
index 3bd0be6..b34276e 100644
--- a/drivers/scsi/lpfc/lpfc_scsi.c
+++ b/drivers/scsi/lpfc/lpfc_scsi.c
@@ -3881,7 +3881,8 @@ int lpfc_sli4_scmd_to_wqidx_distr(struct lpfc_hba *phba,
 		return hwq;
 	}
 
-	if (phba->cfg_fcp_io_sched == LPFC_FCP_SCHED_BY_CPU
+	if ((phba->cfg_fcp_io_sched == LPFC_FCP_SCHED_BY_CPU ||
+	     phba->cfg_fcp_io_sched == LPFC_FCP_SCHED_BY_CPU_CORE)
 	    && phba->cfg_fcp_io_channel > 1) {
 		cpu = smp_processor_id();
 		if (cpu < phba->sli4_hba.num_present_cpu) {
-- 
1.8.3.1

Re: [PATCH 1/2] lpfc: support for CPU phys_id and core_id on PowerPC64

[Christoph Hellwig]

I really don't think this sort of low level information has business
in a low level driver.  I've been trying to put some infrastructure
together to move this to the core kernel [1], and it would be good to
help use this in more drivers.  Especially given that lpfc may use
blk-mq which will not respect this current assignment for the queue
mapping.

[1] http://lists.infradead.org/pipermail/linux-nvme/2016-June/005012.html

Re: [PATCH 1/2] lpfc: support for CPU phys_id and core_id on PowerPC64

[Mauricio Faria de Oliveira]

On 06/21/2016 11:29 AM, Christoph Hellwig wrote:
> I really don't think this sort of low level information has business
> in a low level driver.  I've been trying to put some infrastructure
> together to move this to the core kernel [1], and it would be good to
> help use this in more drivers.  Especially given that lpfc may use
> blk-mq which will not respect this current assignment for the queue
> mapping.

Agree.

However, this patchset targets the non blk-mq/scsi-mq usage of the
lpfc driver, which falls back to CPU-number/queue-number mapping.

This turns out to still be the case w/ some distros where ppc64/le
usually runs, on which it would be easier to adapt this relatively
small change than moving forward w/ blk-mq/scsi-mq, for example --
even if the latter is clearly a superior approach.

> [1] http://lists.infradead.org/pipermail/linux-nvme/2016-June/005012.html


-- 
Mauricio Faria de Oliveira
IBM Linux Technology Center

Re: [PATCH 1/2] lpfc: support for CPU phys_id and core_id on PowerPC64

[Martin K. Petersen]

>>>>> "Mauricio" == Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com> writes:

Mauricio,

Mauricio> This turns out to still be the case w/ some distros where
Mauricio> ppc64/le usually runs,

Distros are free to carry a patch such as yours. That puts the burden on
them and not on upstream which is going in a different direction as
outlined by Christoph.

Mauricio> on which it would be easier to adapt this relatively small
Mauricio> change than moving forward w/ blk-mq/scsi-mq, for example --
Mauricio> even if the latter is clearly a superior approach.

This is ultimately Broadcom's decision. It is their driver.

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH 1/2] lpfc: support for CPU phys_id and core_id on PowerPC64

[Mauricio Faria de Oliveira]

Hi Martin,

On 06/21/2016 11:16 PM, Martin K. Petersen wrote:
> Distros are free to carry a patch such as yours. That puts the burden on
> them and not on upstream which is going in a different direction as
> outlined by Christoph.

> This is ultimately Broadcom's decision. It is their driver.

Right, I understand.  I submitted the patch in case they see value in
having it upstream, as some distros discuss/ask about what's the status
(or differences to) upstream.

In some cases, a rationale like this one being documented on a mailing
list is sufficient, provided the patch hasn't received other technical
problems, for example.

Thanks for the review/comments (Christoph too),

-- 
Mauricio Faria de Oliveira
IBM Linux Technology Center