diff mbox

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

Message ID 201606012043.u51KflcX004680@mx0a-001b2d01.pphosted.com (mailing list archive)
State Changes Requested, archived
Headers show

Commit Message

Mauricio Faria de Oliveira June 1, 2016, 8:43 p.m. UTC
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(-)

Comments

Christoph Hellwig June 21, 2016, 2:29 p.m. UTC | #1
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
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mauricio Faria de Oliveira June 21, 2016, 5:42 p.m. UTC | #2
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
Martin K. Petersen June 22, 2016, 2:16 a.m. UTC | #3
>>>>> "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.
Mauricio Faria de Oliveira June 22, 2016, 12:51 p.m. UTC | #4
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),
diff mbox

Patch

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;