[PATCH 1/1 linux-next] hpsa: remove set but unused variable rc

[PATCH 1/1 linux-next] hpsa: remove set but unused variable rc

[Fabian Frederick]

Fix -Wunused-but-set-variable warning

Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
 drivers/scsi/hpsa.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index cef5d49..34330e1 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -6619,11 +6619,11 @@ static void hpsa_free_cmd_pool(struct ctlr_info *h)
 
 static void hpsa_irq_affinity_hints(struct ctlr_info *h)
 {
-	int i, cpu, rc;
+	int i, cpu;
 
 	cpu = cpumask_first(cpu_online_mask);
 	for (i = 0; i < h->msix_vector; i++) {
-		rc = irq_set_affinity_hint(h->intr[i], get_cpu_mask(cpu));
+		irq_set_affinity_hint(h->intr[i], get_cpu_mask(cpu));
 		cpu = cpumask_next(cpu, cpu_online_mask);
 	}
 }
-- 
1.9.3

Re: [PATCH 1/1 linux-next] hpsa: remove set but unused variable rc

[Sudip Mukherjee]

On Wed, Oct 29, 2014 at 04:15:04PM +0100, Fabian Frederick wrote:
> Fix -Wunused-but-set-variable warning

you should also mention why you have left the call to irq_set_affinity_hint().
i am not sure , but it looks like irq_set_affinity_hint() is only checking if
the lock is available or not. It is just locking ,then if lock is successfull then
returning 0 or if lock fails then return -EINVAL, and unlocks before returnig.
not doing anything else.

thanks
sudip

> 
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
> ---
>  drivers/scsi/hpsa.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
> index cef5d49..34330e1 100644
> --- a/drivers/scsi/hpsa.c
> +++ b/drivers/scsi/hpsa.c
> @@ -6619,11 +6619,11 @@ static void hpsa_free_cmd_pool(struct ctlr_info *h)
>  
>  static void hpsa_irq_affinity_hints(struct ctlr_info *h)
>  {
> -	int i, cpu, rc;
> +	int i, cpu;
>  
>  	cpu = cpumask_first(cpu_online_mask);
>  	for (i = 0; i < h->msix_vector; i++) {
> -		rc = irq_set_affinity_hint(h->intr[i], get_cpu_mask(cpu));
> +		irq_set_affinity_hint(h->intr[i], get_cpu_mask(cpu));
>  		cpu = cpumask_next(cpu, cpu_online_mask);
>  	}
>  }
> -- 
> 1.9.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 1/1 linux-next] hpsa: remove set but unused variable rc

[Elliott, Robert (Server Storage)]

> -----Original Message-----
> From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-
> owner@vger.kernel.org] On Behalf Of Sudip Mukherjee
> Sent: Thursday, October 30, 2014 12:55 AM
> To: Fabian Frederick
> Cc: linux-kernel@vger.kernel.org; Stephen M. Cameron; James E.J.
> Bottomley; iss_storagedev@hp.com; linux-scsi@vger.kernel.org
> Subject: Re: [PATCH 1/1 linux-next] hpsa: remove set but unused variable
> rc
> 
> On Wed, Oct 29, 2014 at 04:15:04PM +0100, Fabian Frederick wrote:
> > Fix -Wunused-but-set-variable warning
> 
> you should also mention why you have left the call to
> irq_set_affinity_hint().
> i am not sure , but it looks like irq_set_affinity_hint() is only
> checking if
> the lock is available or not. It is just locking ,then if lock is
> successfull then
> returning 0 or if lock fails then return -EINVAL, and unlocks before
> returnig.
> not doing anything else.
> 
> thanks
> sudip

No, that function sets a mask value that shows up in 
	/proc/irq/nnn/affinity_hint
that a program like irqbalance may use to set the CPU affinity mask
for each irq via 
	/proc/irq/nnn/smp_affinity   (bitmap format)
	/proc/irq/nnn/smp_affinity_list   (range format)

The reason is that in many cases, it is best when all these occur 
on the same CPU that submitted the IO:
* LLD submission queues (if multiple are supported)
* LDD completion queues
* MSI-X interrupt indicating completion
* LLD completion interrupt handler
* block layer completion handler

Benefits include:
* cache efficiency - the data structures for the IO aren't
pulled from CPU to CPU.

* avoid IPI overhead in the block layer to get the completion 
processed on the submitting CPU (which is done if using 
rq_affinity=2 and the interrupt is routed to on another CPU).

* self-throttle the CPUs - avoid swamping one CPU with 
completion processing for IOs submitted by many other CPUs
(which leads to stalls on the victim and timeouts on the
aggressors).

You must run irqbalance with an option to honor the hints;
some versions default to that, others don't.  Or, disable
the irqbalance service and set them up the affinities
manually.

Re: [PATCH 1/1 linux-next] hpsa: remove set but unused variable rc

[Sudip Mukherjee]

On Thu, Oct 30, 2014 at 06:28:13AM +0000, Elliott, Robert (Server Storage) wrote:
> 
> 
> > -----Original Message-----
> > From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-
> > owner@vger.kernel.org] On Behalf Of Sudip Mukherjee
> > Sent: Thursday, October 30, 2014 12:55 AM
> > To: Fabian Frederick
> > Cc: linux-kernel@vger.kernel.org; Stephen M. Cameron; James E.J.
> > Bottomley; iss_storagedev@hp.com; linux-scsi@vger.kernel.org
> > Subject: Re: [PATCH 1/1 linux-next] hpsa: remove set but unused variable
> > rc
> > 
> > On Wed, Oct 29, 2014 at 04:15:04PM +0100, Fabian Frederick wrote:
> > > Fix -Wunused-but-set-variable warning
> > 
> > you should also mention why you have left the call to
> > irq_set_affinity_hint().
> > i am not sure , but it looks like irq_set_affinity_hint() is only
> > checking if
> > the lock is available or not. It is just locking ,then if lock is
> > successfull then
> > returning 0 or if lock fails then return -EINVAL, and unlocks before
> > returnig.
> > not doing anything else.
> > 
> > thanks
> > sudip
> 
> No, that function sets a mask value that shows up in 
> 	/proc/irq/nnn/affinity_hint
> that a program like irqbalance may use to set the CPU affinity mask
> for each irq via 
> 	/proc/irq/nnn/smp_affinity   (bitmap format)
> 	/proc/irq/nnn/smp_affinity_list   (range format)
> 
> The reason is that in many cases, it is best when all these occur 
> on the same CPU that submitted the IO:
> * LLD submission queues (if multiple are supported)
> * LDD completion queues
> * MSI-X interrupt indicating completion
> * LLD completion interrupt handler
> * block layer completion handler
> 
> Benefits include:
> * cache efficiency - the data structures for the IO aren't
> pulled from CPU to CPU.
> 
> * avoid IPI overhead in the block layer to get the completion 
> processed on the submitting CPU (which is done if using 
> rq_affinity=2 and the interrupt is routed to on another CPU).
> 
> * self-throttle the CPUs - avoid swamping one CPU with 
> completion processing for IOs submitted by many other CPUs
> (which leads to stalls on the victim and timeouts on the
> aggressors).
> 
> You must run irqbalance with an option to honor the hints;
> some versions default to that, others don't.  Or, disable
> the irqbalance service and set them up the affinities
> manually.
> 
thanks , i think this is happening at desc->affinity_hint = m;

thanks
sudip
> 

Re: [PATCH 1/1 linux-next] hpsa: remove set but unused variable rc

[Don Brace]

On 10/29/2014 10:15 AM, Fabian Frederick wrote:
> Fix -Wunused-but-set-variable warning
>
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
> ---
>   drivers/scsi/hpsa.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
> index cef5d49..34330e1 100644
> --- a/drivers/scsi/hpsa.c
> +++ b/drivers/scsi/hpsa.c
> @@ -6619,11 +6619,11 @@ static void hpsa_free_cmd_pool(struct ctlr_info *h)
>
>   static void hpsa_irq_affinity_hints(struct ctlr_info *h)
>   {
> -	int i, cpu, rc;
> +	int i, cpu;
>
>   	cpu = cpumask_first(cpu_online_mask);
>   	for (i = 0; i < h->msix_vector; i++) {
> -		rc = irq_set_affinity_hint(h->intr[i], get_cpu_mask(cpu));
> +		irq_set_affinity_hint(h->intr[i], get_cpu_mask(cpu));
>   		cpu = cpumask_next(cpu, cpu_online_mask);
>   	}
>   }
>

I can add this patch to my current set of patches for 3.19 or ack this 
now, What would you prefer?

---
Don Brace
don.brace@pmcs.com

Re: [PATCH 1/1 linux-next] hpsa: remove set but unused variable rc

[Christoph Hellwig]

On Thu, Oct 30, 2014 at 10:21:05AM -0500, Don Brace wrote:
> I can add this patch to my current set of patches for 3.19 or ack this now,
> What would you prefer?

Please queue it up, I'm much more interested in reducing the massive
backlog of real hpsa changes than in litte cleanups at the moment.