[PATCH v5] qla1280: Don't allocate 512kb of host tags

[PATCH v5] qla1280: Don't allocate 512kb of host tags

[Johannes Thumshirn]

The qla1280 driver sets the scsi_host_template's can_queue field to 0xfffff
which results in an allocation failure when allocating the block layer tags
for the driver's queues. This was introduced with the change for host wide
tags in commit 64d513ac31b - "scsi: use host wide tags by default".

Reduce can_queue to MAX_OUTSTANDING_COMMANDS (512) to solve the allocation
error.

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
Fixes: 64d513ac31b - "scsi: use host wide tags by default"
Cc: stable@vger.kernel.org # v4.4
Cc: Laura Abbott <labbott@redhat.com>
Cc: Michael Reed <mdr@sgi.com>
Reviewed-by: Laurence Oberman <loberman@redhat.com>
Reviewed-by: Lee Duncan <lduncan@suse.com>
---
Changes to v4:
* Make subject more meaningfull
* Remove warning's backtrace
* Re-add reference to commit  64d513ac31b - "scsi: use host wide tags by default"

Changes to v3:
* Use  MAX_OUTSTANDING_COMMANDS insted of hard coded magical number

Changes to v2:
* Change can_queue to 512 upon James' request

 drivers/scsi/qla1280.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/qla1280.c b/drivers/scsi/qla1280.c
index 5d0ec42..634254a 100644
--- a/drivers/scsi/qla1280.c
+++ b/drivers/scsi/qla1280.c
@@ -4214,7 +4214,7 @@ static struct scsi_host_template qla1280_driver_template = {
 	.eh_bus_reset_handler	= qla1280_eh_bus_reset,
 	.eh_host_reset_handler	= qla1280_eh_adapter_reset,
 	.bios_param		= qla1280_biosparam,
-	.can_queue		= 0xfffff,
+	.can_queue		= MAX_OUTSTANDING_COMMANDS,
 	.this_id		= -1,
 	.sg_tablesize		= SG_ALL,
 	.use_clustering		= ENABLE_CLUSTERING,
-- 
2.8.1

Re: [PATCH v5] qla1280: Don't allocate 512kb of host tags

[Hannes Reinecke]

On 04/27/2016 10:48 AM, Johannes Thumshirn wrote:
> The qla1280 driver sets the scsi_host_template's can_queue field to 0xfffff
> which results in an allocation failure when allocating the block layer tags
> for the driver's queues. This was introduced with the change for host wide
> tags in commit 64d513ac31b - "scsi: use host wide tags by default".
> 
> Reduce can_queue to MAX_OUTSTANDING_COMMANDS (512) to solve the allocation
> error.
> 
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
> Fixes: 64d513ac31b - "scsi: use host wide tags by default"
> Cc: stable@vger.kernel.org # v4.4
> Cc: Laura Abbott <labbott@redhat.com>
> Cc: Michael Reed <mdr@sgi.com>
> Reviewed-by: Laurence Oberman <loberman@redhat.com>
> Reviewed-by: Lee Duncan <lduncan@suse.com>
> ---
> Changes to v4:
> * Make subject more meaningfull
> * Remove warning's backtrace
> * Re-add reference to commit  64d513ac31b - "scsi: use host wide tags by default"
> 
> Changes to v3:
> * Use  MAX_OUTSTANDING_COMMANDS insted of hard coded magical number
> 
> Changes to v2:
> * Change can_queue to 512 upon James' request
> 
>  drivers/scsi/qla1280.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/qla1280.c b/drivers/scsi/qla1280.c
> index 5d0ec42..634254a 100644
> --- a/drivers/scsi/qla1280.c
> +++ b/drivers/scsi/qla1280.c
> @@ -4214,7 +4214,7 @@ static struct scsi_host_template qla1280_driver_template = {
>  	.eh_bus_reset_handler	= qla1280_eh_bus_reset,
>  	.eh_host_reset_handler	= qla1280_eh_adapter_reset,
>  	.bios_param		= qla1280_biosparam,
> -	.can_queue		= 0xfffff,
> +	.can_queue		= MAX_OUTSTANDING_COMMANDS,
>  	.this_id		= -1,
>  	.sg_tablesize		= SG_ALL,
>  	.use_clustering		= ENABLE_CLUSTERING,
> 
Hmm.

The driver has this:

	ha->request_ring = pci_alloc_consistent(ha->pdev,
			((REQUEST_ENTRY_CNT + 1) * sizeof(request_t)),
			&ha->request_dma);

and

/* ISP request and response entry counts (37-65535) */
#define REQUEST_ENTRY_CNT	255 /* Number of request entries. */
#define RESPONSE_ENTRY_CNT	63  /* Number of response entries. */

IE the driver (as it stands currently) will never be able to queue
more than 255 requests.
So where's the point in setting can_queue to 512? Shouldn't we
rather use 255 here?

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH v5] qla1280: Don't allocate 512kb of host tags

[James Bottomley]

On Wed, 2016-04-27 at 12:39 +0200, Hannes Reinecke wrote:
> On 04/27/2016 10:48 AM, Johannes Thumshirn wrote:
> > The qla1280 driver sets the scsi_host_template's can_queue field to
> > 0xfffff
> > which results in an allocation failure when allocating the block
> > layer tags
> > for the driver's queues. This was introduced with the change for
> > host wide
> > tags in commit 64d513ac31b - "scsi: use host wide tags by default".
> > 
> > Reduce can_queue to MAX_OUTSTANDING_COMMANDS (512) to solve the
> > allocation
> > error.
> > 
> > Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
> > Fixes: 64d513ac31b - "scsi: use host wide tags by default"
> > Cc: stable@vger.kernel.org # v4.4
> > Cc: Laura Abbott <labbott@redhat.com>
> > Cc: Michael Reed <mdr@sgi.com>
> > Reviewed-by: Laurence Oberman <loberman@redhat.com>
> > Reviewed-by: Lee Duncan <lduncan@suse.com>
> > ---
> > Changes to v4:
> > * Make subject more meaningfull
> > * Remove warning's backtrace
> > * Re-add reference to commit  64d513ac31b - "scsi: use host wide
> > tags by default"
> > 
> > Changes to v3:
> > * Use  MAX_OUTSTANDING_COMMANDS insted of hard coded magical number
> > 
> > Changes to v2:
> > * Change can_queue to 512 upon James' request
> > 
> >  drivers/scsi/qla1280.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/scsi/qla1280.c b/drivers/scsi/qla1280.c
> > index 5d0ec42..634254a 100644
> > --- a/drivers/scsi/qla1280.c
> > +++ b/drivers/scsi/qla1280.c
> > @@ -4214,7 +4214,7 @@ static struct scsi_host_template
> > qla1280_driver_template = {
> >  	.eh_bus_reset_handler	= qla1280_eh_bus_reset,
> >  	.eh_host_reset_handler	= qla1280_eh_adapter_reset,
> >  	.bios_param		= qla1280_biosparam,
> > -	.can_queue		= 0xfffff,
> > +	.can_queue		= MAX_OUTSTANDING_COMMANDS,
> >  	.this_id		= -1,
> >  	.sg_tablesize		= SG_ALL,
> >  	.use_clustering		= ENABLE_CLUSTERING,
> > 
> Hmm.
> 
> The driver has this:
> 
	
> ha->request_ring = pci_alloc_consistent(ha->pdev,
> 			((REQUEST_ENTRY_CNT + 1) * sizeof(request_t)),
> 			&ha->request_dma);
> 
> and
> 
> /* ISP request and response entry counts (37-65535) */
> #define REQUEST_ENTRY_CNT	255 /* Number of request entries. */
> #define RESPONSE_ENTRY_CNT	63  /* Number of response entries.
> */
> 
> IE the driver (as it stands currently) will never be able to queue
> more than 255 requests.
> So where's the point in setting can_queue to 512? Shouldn't we
> rather use 255 here?

Because that's for the request/response message ring.  You don't keep a
request allocated while the command is outstanding, you send a request
message with the command handle (the index into the outstanding_cmds
array) and a pointer to the command and get a response back when the
firmware accepts it (at which time you can re-use the request).

You eventually get a completion interrupt with MBA_SCSI_COMPLETION and
a handle number (index into the outstanding_cmds array) when the host
is done with the command.

James

Re: [PATCH v5] qla1280: Don't allocate 512kb of host tags

[Martin K. Petersen]

>>>>> "Johannes" == Johannes Thumshirn <jthumshirn@suse.de> writes:

Johannes> The qla1280 driver sets the scsi_host_template's can_queue
Johannes> field to 0xfffff which results in an allocation failure when
Johannes> allocating the block layer tags for the driver's queues. This
Johannes> was introduced with the change for host wide tags in commit
Johannes> 64d513ac31b - "scsi: use host wide tags by default".

Johannes> Reduce can_queue to MAX_OUTSTANDING_COMMANDS (512) to solve
Johannes> the allocation error.

Applied to 4.6/scsi-fixes.

-- 
Martin K. Petersen	Oracle Linux Engineering