All of lore.kernel.org
 help / color / mirror / Atom feed
* RE: [PATCH 09/22] [SCSI] mpt2sas, mpt3sas: Added a support to set cpu affinity for each MSIX vector enabled by the HBA
@ 2015-01-07 12:55 Sreekanth Reddy
  2015-01-09 20:58 ` Martin K. Petersen
  0 siblings, 1 reply; 12+ messages in thread
From: Sreekanth Reddy @ 2015-01-07 12:55 UTC (permalink / raw)
  To: martin.petersen, jejb, hch
  Cc: linux-scsi, JBottomley, Sathya.Prakash, Nagalakshmi.Nandigama,
	linux-kernel, Elliott, Sreekanth Reddy

Martin,
I have kept yours CPUs affinity with reply queues logic as it is. I have just replaced
do while loop  with list_for_each_entry loop over reply queues and it doesn't yours 
CPUs affinity with reply queues logic, I am confident on it since I have tested this code
in veriour senarioes i.e when CPUs are non power of two, when CPUs are greater than
reply_ques etc and the things are working properly.

Robert:
In this patch I have replaced zalloc_cpumask_var() API with alloc_cpumask_var() API.

Added a support to set cpu affinity mask for each MSIX vector enabled by the HBA.
So that, running the irqbalancer will balance interrupts among the cpus.

Change_set:
1. Added affinity_hint varable of type cpumask_var_t in adapter_reply_queue
   structure. And allocated a memory for this varable by calling alloc_cpumask_var.
2. Call the API irq_set_affinity_hint for each MSIx vector to affiniate it
   with calculated cpus at driver inilization time.
3. While freeing the MSIX vector, call this same API to release the cpu affinity mask
   for each MSIx vector by providing the NULL value in cpumask argument.
4. then call the free_cpumask_var API to free the memory allocated in step 2.

Signed-off-by: Sreekanth Reddy <Sreekanth.Reddy@avagotech.com>
---
 drivers/scsi/mpt2sas/mpt2sas_base.c | 24 +++++++++++++++++++++---
 drivers/scsi/mpt2sas/mpt2sas_base.h |  1 +
 drivers/scsi/mpt3sas/mpt3sas_base.c | 24 +++++++++++++++++++++---
 drivers/scsi/mpt3sas/mpt3sas_base.h |  1 +
 4 files changed, 44 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c b/drivers/scsi/mpt2sas/mpt2sas_base.c
index 137862c..599606b 100644
--- a/drivers/scsi/mpt2sas/mpt2sas_base.c
+++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
@@ -1300,6 +1300,8 @@ _base_free_irq(struct MPT2SAS_ADAPTER *ioc)
 
 	list_for_each_entry_safe(reply_q, next, &ioc->reply_queue_list, list) {
 		list_del(&reply_q->list);
+		irq_set_affinity_hint(reply_q->vector, NULL);
+		free_cpumask_var(reply_q->affinity_hint);
 		synchronize_irq(reply_q->vector);
 		free_irq(reply_q->vector, reply_q);
 		kfree(reply_q);
@@ -1329,6 +1331,11 @@ _base_request_irq(struct MPT2SAS_ADAPTER *ioc, u8 index, u32 vector)
 	reply_q->ioc = ioc;
 	reply_q->msix_index = index;
 	reply_q->vector = vector;
+
+	if (!alloc_cpumask_var(&reply_q->affinity_hint, GFP_KERNEL))
+		return -ENOMEM;
+	cpumask_clear(reply_q->affinity_hint);
+
 	atomic_set(&reply_q->busy, 0);
 	if (ioc->msix_enable)
 		snprintf(reply_q->name, MPT_NAME_LENGTH, "%s%d-msix%d",
@@ -1363,6 +1370,7 @@ static void
 _base_assign_reply_queues(struct MPT2SAS_ADAPTER *ioc)
 {
 	unsigned int cpu, nr_cpus, nr_msix, index = 0;
+	struct adapter_reply_queue *reply_q;
 
 	if (!_base_is_controller_msix_enabled(ioc))
 		return;
@@ -1377,20 +1385,30 @@ _base_assign_reply_queues(struct MPT2SAS_ADAPTER *ioc)
 
 	cpu = cpumask_first(cpu_online_mask);
 
-	do {
+	list_for_each_entry(reply_q, &ioc->reply_queue_list, list) {
+
 		unsigned int i, group = nr_cpus / nr_msix;
 
+		if (cpu >= nr_cpus)
+			break;
+
 		if (index < nr_cpus % nr_msix)
 			group++;
 
 		for (i = 0 ; i < group ; i++) {
 			ioc->cpu_msix_table[cpu] = index;
+			cpumask_or(reply_q->affinity_hint,
+				   reply_q->affinity_hint, get_cpu_mask(cpu));
 			cpu = cpumask_next(cpu, cpu_online_mask);
 		}
 
+		if (irq_set_affinity_hint(reply_q->vector,
+					   reply_q->affinity_hint))
+			dinitprintk(ioc, pr_info(MPT2SAS_FMT
+			    "error setting affinity hint for irq vector %d\n",
+			    ioc->name, reply_q->vector));
 		index++;
-
-	} while (cpu < nr_cpus);
+	}
 }
 
 /**
diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.h b/drivers/scsi/mpt2sas/mpt2sas_base.h
index 72bffec..46e8d51 100644
--- a/drivers/scsi/mpt2sas/mpt2sas_base.h
+++ b/drivers/scsi/mpt2sas/mpt2sas_base.h
@@ -587,6 +587,7 @@ struct adapter_reply_queue {
 	Mpi2ReplyDescriptorsUnion_t *reply_post_free;
 	char			name[MPT_NAME_LENGTH];
 	atomic_t		busy;
+	cpumask_var_t		affinity_hint;
 	struct list_head	list;
 };
 
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 961bd9d..4f6fb88 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -1584,6 +1584,8 @@ _base_free_irq(struct MPT3SAS_ADAPTER *ioc)
 
 	list_for_each_entry_safe(reply_q, next, &ioc->reply_queue_list, list) {
 		list_del(&reply_q->list);
+		irq_set_affinity_hint(reply_q->vector, NULL);
+		free_cpumask_var(reply_q->affinity_hint);
 		synchronize_irq(reply_q->vector);
 		free_irq(reply_q->vector, reply_q);
 		kfree(reply_q);
@@ -1613,6 +1615,11 @@ _base_request_irq(struct MPT3SAS_ADAPTER *ioc, u8 index, u32 vector)
 	reply_q->ioc = ioc;
 	reply_q->msix_index = index;
 	reply_q->vector = vector;
+
+	if (!alloc_cpumask_var(&reply_q->affinity_hint, GFP_KERNEL))
+		return -ENOMEM;
+	cpumask_clear(reply_q->affinity_hint);
+
 	atomic_set(&reply_q->busy, 0);
 	if (ioc->msix_enable)
 		snprintf(reply_q->name, MPT_NAME_LENGTH, "%s%d-msix%d",
@@ -1647,6 +1654,7 @@ static void
 _base_assign_reply_queues(struct MPT3SAS_ADAPTER *ioc)
 {
 	unsigned int cpu, nr_cpus, nr_msix, index = 0;
+	struct adapter_reply_queue *reply_q;
 
 	if (!_base_is_controller_msix_enabled(ioc))
 		return;
@@ -1661,20 +1669,30 @@ _base_assign_reply_queues(struct MPT3SAS_ADAPTER *ioc)
 
 	cpu = cpumask_first(cpu_online_mask);
 
-	do {
+	list_for_each_entry(reply_q, &ioc->reply_queue_list, list) {
+
 		unsigned int i, group = nr_cpus / nr_msix;
 
+		if (cpu >= nr_cpus)
+			break;
+
 		if (index < nr_cpus % nr_msix)
 			group++;
 
 		for (i = 0 ; i < group ; i++) {
 			ioc->cpu_msix_table[cpu] = index;
+			cpumask_or(reply_q->affinity_hint,
+				   reply_q->affinity_hint, get_cpu_mask(cpu));
 			cpu = cpumask_next(cpu, cpu_online_mask);
 		}
 
+		if (irq_set_affinity_hint(reply_q->vector,
+					   reply_q->affinity_hint))
+			dinitprintk(ioc, pr_info(MPT3SAS_FMT
+			    "error setting affinity hint for irq vector %d\n",
+			    ioc->name, reply_q->vector));
 		index++;
-
-	} while (cpu < nr_cpus);
+	}
 }
 
 /**
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h b/drivers/scsi/mpt3sas/mpt3sas_base.h
index 7e9f55b..afa8816 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.h
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.h
@@ -507,6 +507,7 @@ struct adapter_reply_queue {
 	Mpi2ReplyDescriptorsUnion_t *reply_post_free;
 	char			name[MPT_NAME_LENGTH];
 	atomic_t		busy;
+	cpumask_var_t		affinity_hint;
 	struct list_head	list;
 };
 
-- 
2.0.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 09/22] [SCSI] mpt2sas, mpt3sas: Added a support to set cpu affinity for each MSIX vector enabled by the HBA
  2015-01-07 12:55 [PATCH 09/22] [SCSI] mpt2sas, mpt3sas: Added a support to set cpu affinity for each MSIX vector enabled by the HBA Sreekanth Reddy
@ 2015-01-09 20:58 ` Martin K. Petersen
  2015-01-10 10:14   ` Christoph Hellwig
  0 siblings, 1 reply; 12+ messages in thread
From: Martin K. Petersen @ 2015-01-09 20:58 UTC (permalink / raw)
  To: Sreekanth Reddy
  Cc: martin.petersen, jejb, hch, linux-scsi, JBottomley,
	Sathya.Prakash, Nagalakshmi.Nandigama, linux-kernel, Elliott

>>>>> "Sreekanth" == Sreekanth Reddy <sreekanth.reddy@avagotech.com> writes:

Sreekanth> Change_set: 1. Added affinity_hint varable of type
Sreekanth> cpumask_var_t in adapter_reply_queue structure. And allocated
Sreekanth> a memory for this varable by calling alloc_cpumask_var.
Sreekanth> 2. Call the API irq_set_affinity_hint for each MSIx vector to
Sreekanth> affiniate it with calculated cpus at driver inilization time.
Sreekanth> 3. While freeing the MSIX vector, call this same API to
Sreekanth> release the cpu affinity mask for each MSIx vector by
Sreekanth> providing the NULL value in cpumask argument.  4. then call
Sreekanth> the free_cpumask_var API to free the memory allocated in step
Sreekanth> 2.

(Still dreaming of a combined mpt2sas and mpt3sas so I wouldn't have to
review everything twice).

Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 09/22] [SCSI] mpt2sas, mpt3sas: Added a support to set cpu affinity for each MSIX vector enabled by the HBA
  2015-01-09 20:58 ` Martin K. Petersen
@ 2015-01-10 10:14   ` Christoph Hellwig
  0 siblings, 0 replies; 12+ messages in thread
From: Christoph Hellwig @ 2015-01-10 10:14 UTC (permalink / raw)
  To: Martin K. Petersen
  Cc: Sreekanth Reddy, jejb, hch, linux-scsi, JBottomley,
	Sathya.Prakash, Nagalakshmi.Nandigama, linux-kernel, Elliott

On Fri, Jan 09, 2015 at 03:58:16PM -0500, Martin K. Petersen wrote:
> (Still dreaming of a combined mpt2sas and mpt3sas so I wouldn't have to
> review everything twice).

We really need to start that action instead of dreaming.  Once this
series is in I'll move the two drivers to at least a shared set of
mpt firmware headers as a start, and then we'll need to find ways to
work from that piecemail.

Note that the mpt headers also would be useful for the megaraid_sas
driver which has it's own hand crafted version, but that's another
story..

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 09/22] [SCSI] mpt2sas, mpt3sas: Added a support to set cpu affinity for each MSIX vector enabled by the HBA
  2014-12-11  1:53   ` Elliott, Robert (Server Storage)
@ 2014-12-11 11:58     ` Sreekanth Reddy
  -1 siblings, 0 replies; 12+ messages in thread
From: Sreekanth Reddy @ 2014-12-11 11:58 UTC (permalink / raw)
  To: Elliott, Robert (Server Storage)
  Cc: martin.petersen, jejb, hch, linux-scsi, JBottomley@Parallels.com,
	Sathya.Prakash, Nagalakshmi.Nandigama, linux-kernel

>> @@ -1609,6 +1611,10 @@ _base_request_irq(struct MPT3SAS_ADAPTER *ioc, u8
>> index, u32 vector)
>>       reply_q->ioc = ioc;
>>       reply_q->msix_index = index;
>>       reply_q->vector = vector;
>> +
>> +     if (!zalloc_cpumask_var(&reply_q->affinity_hint, GFP_KERNEL))
>> +             return -ENOMEM;
>
> I think this will create the problem Alex Thorlton just reported
> with lpfc on a system with a huge number (6144) of CPUs.
>
> See this thread:
>         [BUG] kzalloc overflow in lpfc driver on 6k core system

Oh ok, Then I will use alloc_cpumask_var() API and cpumask_clear() to
initialize all the CPU bits in the mask to zero. Is it fine?

Regards,
Sreekanth

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 09/22] [SCSI] mpt2sas, mpt3sas: Added a support to set cpu affinity for each MSIX vector enabled by the HBA
@ 2014-12-11 11:58     ` Sreekanth Reddy
  0 siblings, 0 replies; 12+ messages in thread
From: Sreekanth Reddy @ 2014-12-11 11:58 UTC (permalink / raw)
  To: Elliott, Robert (Server Storage)
  Cc: martin.petersen, jejb, hch, linux-scsi, JBottomley@Parallels.com,
	Sathya.Prakash, Nagalakshmi.Nandigama, linux-kernel

>> @@ -1609,6 +1611,10 @@ _base_request_irq(struct MPT3SAS_ADAPTER *ioc, u8
>> index, u32 vector)
>>       reply_q->ioc = ioc;
>>       reply_q->msix_index = index;
>>       reply_q->vector = vector;
>> +
>> +     if (!zalloc_cpumask_var(&reply_q->affinity_hint, GFP_KERNEL))
>> +             return -ENOMEM;
>
> I think this will create the problem Alex Thorlton just reported
> with lpfc on a system with a huge number (6144) of CPUs.
>
> See this thread:
>         [BUG] kzalloc overflow in lpfc driver on 6k core system

Oh ok, Then I will use alloc_cpumask_var() API and cpumask_clear() to
initialize all the CPU bits in the mask to zero. Is it fine?

Regards,
Sreekanth

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 09/22] [SCSI] mpt2sas, mpt3sas: Added a support to set cpu affinity for each MSIX vector enabled by the HBA
  2014-12-10 20:48 ` Martin K. Petersen
@ 2014-12-11 11:51   ` Sreekanth Reddy
  0 siblings, 0 replies; 12+ messages in thread
From: Sreekanth Reddy @ 2014-12-11 11:51 UTC (permalink / raw)
  To: Martin K. Petersen
  Cc: jejb, Christoph Hellwig, linux-scsi, James E.J. Bottomley,
	Sathya Prakash, Nagalakshmi Nandigama, linux-kernel

> @@ -1373,20 +1380,30 @@ _base_assign_reply_queues(struct MPT2SAS_ADAPTER *ioc)
>
>         cpu = cpumask_first(cpu_online_mask);
>
> -       do {
> +       list_for_each_entry(reply_q, &ioc->reply_queue_list, list) {
> +
>
> Why are you reverting to iterating over the queues? A while back I fixed
> this up so it wouldn't fail when nr_cpus > the number of reply queues.
>

I have kept yours CPUs affinity with reply queues logic as it is. This
patch doesn't break any of it's original CPUs affinity with reply
queues calculation  even when nr_cpus > the number of reply queues.
since below loop will help us to calculate CPU groups which needs to
affinity to particular reply queue when nr_cpus > the number of reply
queues

               for (i = 0 ; i < group ; i++) {
                        ioc->cpu_msix_table[cpu] = index;
+                       cpumask_or(reply_q->affinity_hint,
+                                  reply_q->affinity_hint, get_cpu_mask(cpu));
                        cpu = cpumask_next(cpu, cpu_online_mask);
                }

I am iterating over the queues to get the MSIX vector number easily,
while calling irq_set_affinity_hint() API for setting CPUs affinity
hint for that MSIX vector.

Regards,
Sreekanth

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCH 09/22] [SCSI] mpt2sas, mpt3sas: Added a support to set cpu affinity for each MSIX vector enabled by the HBA
  2014-12-09 12:16 Sreekanth Reddy
@ 2014-12-11  1:53   ` Elliott, Robert (Server Storage)
  2014-12-11  1:53   ` Elliott, Robert (Server Storage)
  1 sibling, 0 replies; 12+ messages in thread
From: Elliott, Robert (Server Storage) @ 2014-12-11  1:53 UTC (permalink / raw)
  To: Sreekanth Reddy, martin.petersen, jejb, hch
  Cc: linux-scsi, JBottomley, Sathya.Prakash, Nagalakshmi.Nandigama,
	linux-kernel

> -----Original Message-----
> From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-
> owner@vger.kernel.org] On Behalf Of Sreekanth Reddy
> Sent: Tuesday, 09 December, 2014 6:17 AM
> To: martin.petersen@oracle.com; jejb@kernel.org; hch@infradead.org
...
> Change_set:
> 1. Added affinity_hint varable of type cpumask_var_t in adapter_reply_queue
>    structure. And allocated a memory for this varable by calling
> zalloc_cpumask_var.
> 2. Call the API irq_set_affinity_hint for each MSIx vector to affiniate it
>    with calculated cpus at driver inilization time.
> 3. While freeing the MSIX vector, call this same API to release the cpu
> affinity mask
>    for each MSIx vector by providing the NULL value in cpumask argument.
> 4. then call the free_cpumask_var API to free the memory allocated in step 2.
> 
...

> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c
> b/drivers/scsi/mpt3sas/mpt3sas_base.c
> index 1560115..f0f8ba0 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_base.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
...
> @@ -1609,6 +1611,10 @@ _base_request_irq(struct MPT3SAS_ADAPTER *ioc, u8
> index, u32 vector)
>  	reply_q->ioc = ioc;
>  	reply_q->msix_index = index;
>  	reply_q->vector = vector;
> +
> +	if (!zalloc_cpumask_var(&reply_q->affinity_hint, GFP_KERNEL))
> +		return -ENOMEM;

I think this will create the problem Alex Thorlton just reported
with lpfc on a system with a huge number (6144) of CPUs.

See this thread:
	[BUG] kzalloc overflow in lpfc driver on 6k core system

---
Rob Elliott    HP Server Storage




^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCH 09/22] [SCSI] mpt2sas, mpt3sas: Added a support to set cpu affinity for each MSIX vector enabled by the HBA
@ 2014-12-11  1:53   ` Elliott, Robert (Server Storage)
  0 siblings, 0 replies; 12+ messages in thread
From: Elliott, Robert (Server Storage) @ 2014-12-11  1:53 UTC (permalink / raw)
  To: Sreekanth Reddy, martin.petersen, jejb, hch
  Cc: linux-scsi, JBottomley, Sathya.Prakash, Nagalakshmi.Nandigama,
	linux-kernel

> -----Original Message-----
> From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-
> owner@vger.kernel.org] On Behalf Of Sreekanth Reddy
> Sent: Tuesday, 09 December, 2014 6:17 AM
> To: martin.petersen@oracle.com; jejb@kernel.org; hch@infradead.org
...
> Change_set:
> 1. Added affinity_hint varable of type cpumask_var_t in adapter_reply_queue
>    structure. And allocated a memory for this varable by calling
> zalloc_cpumask_var.
> 2. Call the API irq_set_affinity_hint for each MSIx vector to affiniate it
>    with calculated cpus at driver inilization time.
> 3. While freeing the MSIX vector, call this same API to release the cpu
> affinity mask
>    for each MSIx vector by providing the NULL value in cpumask argument.
> 4. then call the free_cpumask_var API to free the memory allocated in step 2.
> 
...

> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c
> b/drivers/scsi/mpt3sas/mpt3sas_base.c
> index 1560115..f0f8ba0 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_base.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
...
> @@ -1609,6 +1611,10 @@ _base_request_irq(struct MPT3SAS_ADAPTER *ioc, u8
> index, u32 vector)
>  	reply_q->ioc = ioc;
>  	reply_q->msix_index = index;
>  	reply_q->vector = vector;
> +
> +	if (!zalloc_cpumask_var(&reply_q->affinity_hint, GFP_KERNEL))
> +		return -ENOMEM;

I think this will create the problem Alex Thorlton just reported
with lpfc on a system with a huge number (6144) of CPUs.

See this thread:
	[BUG] kzalloc overflow in lpfc driver on 6k core system

---
Rob Elliott    HP Server Storage

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 09/22] [SCSI] mpt2sas, mpt3sas: Added a support to set cpu affinity for each MSIX vector enabled by the HBA
  2014-12-09 12:16 Sreekanth Reddy
@ 2014-12-10 20:48 ` Martin K. Petersen
  2014-12-11 11:51   ` Sreekanth Reddy
  2014-12-11  1:53   ` Elliott, Robert (Server Storage)
  1 sibling, 1 reply; 12+ messages in thread
From: Martin K. Petersen @ 2014-12-10 20:48 UTC (permalink / raw)
  To: Sreekanth Reddy
  Cc: martin.petersen, jejb, hch, linux-scsi, JBottomley,
	Sathya.Prakash, Nagalakshmi.Nandigama, linux-kernel

>>>>> "Sreekanth" == Sreekanth Reddy <sreekanth.reddy@avagotech.com> writes:

Sreekanth,

@@ -1373,20 +1380,30 @@ _base_assign_reply_queues(struct MPT2SAS_ADAPTER *ioc)
 
 	cpu = cpumask_first(cpu_online_mask);
 
-	do {
+	list_for_each_entry(reply_q, &ioc->reply_queue_list, list) {
+

Why are you reverting to iterating over the queues? A while back I fixed
this up so it wouldn't fail when nr_cpus > the number of reply queues.

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCH 09/22] [SCSI] mpt2sas, mpt3sas: Added a support to set cpu affinity for each MSIX vector enabled by the HBA
@ 2014-12-09 12:16 Sreekanth Reddy
  2014-12-10 20:48 ` Martin K. Petersen
  2014-12-11  1:53   ` Elliott, Robert (Server Storage)
  0 siblings, 2 replies; 12+ messages in thread
From: Sreekanth Reddy @ 2014-12-09 12:16 UTC (permalink / raw)
  To: martin.petersen, jejb, hch
  Cc: linux-scsi, JBottomley, Sathya.Prakash, Nagalakshmi.Nandigama,
	linux-kernel, Sreekanth Reddy

> Wouldn't it be better to do this in _base_assign_reply_queues since
> we're already iterating there?

Hi Martin,

As per your suggestion, I modified this feature with below changes.

Added a support to set cpu affinity mask for each MSIX vector enabled by the HBA.
So that, runnig the irqbalancer will balance interrupts among the cpus.

Change_set:
1. Added affinity_hint varable of type cpumask_var_t in adapter_reply_queue
   structure. And allocated a memory for this varable by calling zalloc_cpumask_var.
2. Call the API irq_set_affinity_hint for each MSIx vector to affiniate it
   with calculated cpus at driver inilization time.
3. While freeing the MSIX vector, call this same API to release the cpu affinity mask
   for each MSIx vector by providing the NULL value in cpumask argument.
4. then call the free_cpumask_var API to free the memory allocated in step 2.

Signed-off-by: Sreekanth Reddy <Sreekanth.Reddy@avagotech.com>
---
 drivers/scsi/mpt2sas/mpt2sas_base.c |   23 ++++++++++++++++++++---
 drivers/scsi/mpt2sas/mpt2sas_base.h |    1 +
 drivers/scsi/mpt3sas/mpt3sas_base.c |   23 ++++++++++++++++++++---
 drivers/scsi/mpt3sas/mpt3sas_base.h |    1 +
 4 files changed, 42 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c b/drivers/scsi/mpt2sas/mpt2sas_base.c
index 58e4521..c58df3d 100644
--- a/drivers/scsi/mpt2sas/mpt2sas_base.c
+++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
@@ -1296,6 +1296,8 @@ _base_free_irq(struct MPT2SAS_ADAPTER *ioc)
 
 	list_for_each_entry_safe(reply_q, next, &ioc->reply_queue_list, list) {
 		list_del(&reply_q->list);
+		irq_set_affinity_hint(reply_q->vector, NULL);
+		free_cpumask_var(reply_q->affinity_hint);
 		synchronize_irq(reply_q->vector);
 		free_irq(reply_q->vector, reply_q);
 		kfree(reply_q);
@@ -1325,6 +1327,10 @@ _base_request_irq(struct MPT2SAS_ADAPTER *ioc, u8 index, u32 vector)
 	reply_q->ioc = ioc;
 	reply_q->msix_index = index;
 	reply_q->vector = vector;
+
+	if (!zalloc_cpumask_var(&reply_q->affinity_hint, GFP_KERNEL))
+		return -ENOMEM;
+
 	atomic_set(&reply_q->busy, 0);
 	if (ioc->msix_enable)
 		snprintf(reply_q->name, MPT_NAME_LENGTH, "%s%d-msix%d",
@@ -1359,6 +1365,7 @@ static void
 _base_assign_reply_queues(struct MPT2SAS_ADAPTER *ioc)
 {
 	unsigned int cpu, nr_cpus, nr_msix, index = 0;
+	struct adapter_reply_queue *reply_q;
 
 	if (!_base_is_controller_msix_enabled(ioc))
 		return;
@@ -1373,20 +1380,30 @@ _base_assign_reply_queues(struct MPT2SAS_ADAPTER *ioc)
 
 	cpu = cpumask_first(cpu_online_mask);
 
-	do {
+	list_for_each_entry(reply_q, &ioc->reply_queue_list, list) {
+
 		unsigned int i, group = nr_cpus / nr_msix;
+
+		if (cpu >= nr_cpus)
+			break;
 
 		if (index < nr_cpus % nr_msix)
 			group++;
 
 		for (i = 0 ; i < group ; i++) {
 			ioc->cpu_msix_table[cpu] = index;
+			cpumask_or(reply_q->affinity_hint,
+				   reply_q->affinity_hint, get_cpu_mask(cpu));
 			cpu = cpumask_next(cpu, cpu_online_mask);
 		}
 
+		if (irq_set_affinity_hint(reply_q->vector,
+					   reply_q->affinity_hint))
+			dinitprintk(ioc, pr_info(MPT2SAS_FMT
+			    "error setting affinity hint for irq vector %d\n",
+			    ioc->name, reply_q->vector));
 		index++;
-
-	} while (cpu < nr_cpus);
+	}
 }
 
 /**
diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.h b/drivers/scsi/mpt2sas/mpt2sas_base.h
index 239f169..295f98c 100644
--- a/drivers/scsi/mpt2sas/mpt2sas_base.h
+++ b/drivers/scsi/mpt2sas/mpt2sas_base.h
@@ -586,6 +586,7 @@ struct adapter_reply_queue {
 	Mpi2ReplyDescriptorsUnion_t *reply_post_free;
 	char			name[MPT_NAME_LENGTH];
 	atomic_t		busy;
+	cpumask_var_t		affinity_hint;
 	struct list_head	list;
 };
 
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 1560115..f0f8ba0 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -1580,6 +1580,8 @@ _base_free_irq(struct MPT3SAS_ADAPTER *ioc)
 
 	list_for_each_entry_safe(reply_q, next, &ioc->reply_queue_list, list) {
 		list_del(&reply_q->list);
+		irq_set_affinity_hint(reply_q->vector, NULL);
+		free_cpumask_var(reply_q->affinity_hint);
 		synchronize_irq(reply_q->vector);
 		free_irq(reply_q->vector, reply_q);
 		kfree(reply_q);
@@ -1609,6 +1611,10 @@ _base_request_irq(struct MPT3SAS_ADAPTER *ioc, u8 index, u32 vector)
 	reply_q->ioc = ioc;
 	reply_q->msix_index = index;
 	reply_q->vector = vector;
+
+	if (!zalloc_cpumask_var(&reply_q->affinity_hint, GFP_KERNEL))
+		return -ENOMEM;
+
 	atomic_set(&reply_q->busy, 0);
 	if (ioc->msix_enable)
 		snprintf(reply_q->name, MPT_NAME_LENGTH, "%s%d-msix%d",
@@ -1643,6 +1649,7 @@ static void
 _base_assign_reply_queues(struct MPT3SAS_ADAPTER *ioc)
 {
 	unsigned int cpu, nr_cpus, nr_msix, index = 0;
+	struct adapter_reply_queue *reply_q;
 
 	if (!_base_is_controller_msix_enabled(ioc))
 		return;
@@ -1657,20 +1664,30 @@ _base_assign_reply_queues(struct MPT3SAS_ADAPTER *ioc)
 
 	cpu = cpumask_first(cpu_online_mask);
 
-	do {
+	list_for_each_entry(reply_q, &ioc->reply_queue_list, list) {
+
 		unsigned int i, group = nr_cpus / nr_msix;
+
+		if (cpu >= nr_cpus)
+			break;
 
 		if (index < nr_cpus % nr_msix)
 			group++;
 
 		for (i = 0 ; i < group ; i++) {
 			ioc->cpu_msix_table[cpu] = index;
+			cpumask_or(reply_q->affinity_hint,
+				   reply_q->affinity_hint, get_cpu_mask(cpu));
 			cpu = cpumask_next(cpu, cpu_online_mask);
 		}
 
+		if (irq_set_affinity_hint(reply_q->vector,
+					   reply_q->affinity_hint))
+			dinitprintk(ioc, pr_info(MPT3SAS_FMT
+			    "error setting affinity hint for irq vector %d\n",
+			    ioc->name, reply_q->vector));
 		index++;
-
-	} while (cpu < nr_cpus);
+	}
 }
 
 /**
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h b/drivers/scsi/mpt3sas/mpt3sas_base.h
index 40926aa..fee37b7 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.h
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.h
@@ -506,6 +506,7 @@ struct adapter_reply_queue {
 	Mpi2ReplyDescriptorsUnion_t *reply_post_free;
 	char			name[MPT_NAME_LENGTH];
 	atomic_t		busy;
+	cpumask_var_t		affinity_hint;
 	struct list_head	list;
 };
 
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 09/22] [SCSI] mpt2sas, mpt3sas: Added a support to set cpu affinity for each MSIX vector enabled by the HBA
  2014-11-20  7:05 ` [PATCH 09/22] [SCSI] mpt2sas, mpt3sas: Added a support to set cpu affinity for each MSIX vector enabled by the HBA Sreekanth Reddy
@ 2014-12-04  3:16   ` Martin K. Petersen
  0 siblings, 0 replies; 12+ messages in thread
From: Martin K. Petersen @ 2014-12-04  3:16 UTC (permalink / raw)
  To: Sreekanth Reddy
  Cc: jejb, hch, martin.petersen, linux-scsi, JBottomley,
	Sathya.Prakash, Nagalakshmi.Nandigama, linux-kernel

>>>>> "Sreekanth" == Sreekanth Reddy <sreekanth.reddy@avagotech.com> writes:

Sreekanth> Added a support to set cpu affinity mask for each MSIX vector
Sreekanth> enabled by the HBA, So that by runnig the irqbalancer,
Sreekanth> interrupts can be balanced among the cpus.

Wouldn't it be better to do this in _base_assign_reply_queues since
we're already iterating there?

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 09/22] [SCSI] mpt2sas, mpt3sas: Added a support to set cpu affinity for each MSIX vector enabled by the HBA
  2014-11-20  7:05 [PATCH 00/22] mpt2sas, mpt3sas: SAS2 Phase 19,20 and SAS3 Phase 4,5 patches Sreekanth Reddy
@ 2014-11-20  7:05 ` Sreekanth Reddy
  2014-12-04  3:16   ` Martin K. Petersen
  0 siblings, 1 reply; 12+ messages in thread
From: Sreekanth Reddy @ 2014-11-20  7:05 UTC (permalink / raw)
  To: jejb, hch
  Cc: martin.petersen, linux-scsi, JBottomley, Sathya.Prakash,
	Nagalakshmi.Nandigama, linux-kernel, Sreekanth Reddy

Added a support to set cpu affinity mask for each MSIX vector enabled by the HBA,
So that by runnig the irqbalancer, interrupts can be balanced among the cpus.

Change_set:
1. Call the API irq_set_affinity_hint for each MSIX vector to affiniate it with avalibale online cpus at driver inilization time.
2. At the driver unload time, call this same API to release the cpu affinity mask for each MSIx vector by providing the NULL value in cpumask argument.

Signed-off-by: Sreekanth Reddy <Sreekanth.Reddy@avagotech.com>
---
 drivers/scsi/mpt2sas/mpt2sas_base.c | 11 +++++++++++
 drivers/scsi/mpt3sas/mpt3sas_base.c | 11 +++++++++++
 2 files changed, 22 insertions(+)

diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c b/drivers/scsi/mpt2sas/mpt2sas_base.c
index f10ee41..7286cd2 100644
--- a/drivers/scsi/mpt2sas/mpt2sas_base.c
+++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
@@ -1301,6 +1301,7 @@ _base_free_irq(struct MPT2SAS_ADAPTER *ioc)
 	list_for_each_entry_safe(reply_q, next, &ioc->reply_queue_list, list) {
 		list_del(&reply_q->list);
 		synchronize_irq(reply_q->vector);
+		irq_set_affinity_hint(reply_q->vector, NULL);
 		free_irq(reply_q->vector, reply_q);
 		kfree(reply_q);
 	}
@@ -1419,6 +1420,7 @@ _base_enable_msix(struct MPT2SAS_ADAPTER *ioc)
 	int r;
 	int i;
 	u8 try_msix = 0;
+	int cpu;
 
 	if (msix_disable == -1 || msix_disable == 0)
 		try_msix = 1;
@@ -1467,6 +1469,7 @@ _base_enable_msix(struct MPT2SAS_ADAPTER *ioc)
 	}
 
 	ioc->msix_enable = 1;
+	cpu = cpumask_first(cpu_online_mask);
 	for (i = 0, a = entries; i < ioc->reply_queue_count; i++, a++) {
 		r = _base_request_irq(ioc, i, a->vector);
 		if (r) {
@@ -1475,6 +1478,14 @@ _base_enable_msix(struct MPT2SAS_ADAPTER *ioc)
 			kfree(entries);
 			goto try_ioapic;
 		}
+		dinitprintk(ioc, printk(MPT2SAS_INFO_FMT
+			"cpu %d affinity hint for vector %d\n",
+			 ioc->name,  cpu, a->vector));
+		if (irq_set_affinity_hint(a->vector, get_cpu_mask(cpu)))
+			dinitprintk(ioc, printk(MPT2SAS_INFO_FMT
+				"error setting affinity hint for cpu %d\n",
+				ioc->name,  cpu));
+		cpu = cpumask_next(cpu, cpu_online_mask);
 	}
 
 	kfree(entries);
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index ce18ad2..d9f1943 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -1585,6 +1585,7 @@ _base_free_irq(struct MPT3SAS_ADAPTER *ioc)
 	list_for_each_entry_safe(reply_q, next, &ioc->reply_queue_list, list) {
 		list_del(&reply_q->list);
 		synchronize_irq(reply_q->vector);
+		irq_set_affinity_hint(reply_q->vector, NULL);
 		free_irq(reply_q->vector, reply_q);
 		kfree(reply_q);
 	}
@@ -1703,6 +1704,7 @@ _base_enable_msix(struct MPT3SAS_ADAPTER *ioc)
 	int r;
 	int i;
 	u8 try_msix = 0;
+	int cpu;
 
 	if (msix_disable == -1 || msix_disable == 0)
 		try_msix = 1;
@@ -1752,6 +1754,7 @@ _base_enable_msix(struct MPT3SAS_ADAPTER *ioc)
 	}
 
 	ioc->msix_enable = 1;
+	cpu = cpumask_first(cpu_online_mask);
 	for (i = 0, a = entries; i < ioc->reply_queue_count; i++, a++) {
 		r = _base_request_irq(ioc, i, a->vector);
 		if (r) {
@@ -1760,6 +1763,14 @@ _base_enable_msix(struct MPT3SAS_ADAPTER *ioc)
 			kfree(entries);
 			goto try_ioapic;
 		}
+		dinitprintk(ioc, pr_info(MPT3SAS_FMT
+		    "cpu %d affinity hint for vector %d\n",
+		     ioc->name,  cpu, a->vector));
+		if (irq_set_affinity_hint(a->vector, get_cpu_mask(cpu)))
+			dinitprintk(ioc, pr_info(MPT3SAS_FMT
+			     "error setting affinity hint for cpu %d\n",
+			     ioc->name,  cpu));
+		cpu = cpumask_next(cpu, cpu_online_mask);
 	}
 
 	kfree(entries);
-- 
2.0.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2015-01-10 10:15 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-07 12:55 [PATCH 09/22] [SCSI] mpt2sas, mpt3sas: Added a support to set cpu affinity for each MSIX vector enabled by the HBA Sreekanth Reddy
2015-01-09 20:58 ` Martin K. Petersen
2015-01-10 10:14   ` Christoph Hellwig
  -- strict thread matches above, loose matches on Subject: below --
2014-12-09 12:16 Sreekanth Reddy
2014-12-10 20:48 ` Martin K. Petersen
2014-12-11 11:51   ` Sreekanth Reddy
2014-12-11  1:53 ` Elliott, Robert (Server Storage)
2014-12-11  1:53   ` Elliott, Robert (Server Storage)
2014-12-11 11:58   ` Sreekanth Reddy
2014-12-11 11:58     ` Sreekanth Reddy
2014-11-20  7:05 [PATCH 00/22] mpt2sas, mpt3sas: SAS2 Phase 19,20 and SAS3 Phase 4,5 patches Sreekanth Reddy
2014-11-20  7:05 ` [PATCH 09/22] [SCSI] mpt2sas, mpt3sas: Added a support to set cpu affinity for each MSIX vector enabled by the HBA Sreekanth Reddy
2014-12-04  3:16   ` Martin K. Petersen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.