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@xxxxxxx>
> > Fixes: 64d513ac31b - "scsi: use host wide tags by default"
> > Cc: stable@xxxxxxxxxxxxxxx # v4.4
> > Cc: Laura Abbott <labbott@xxxxxxxxxx>
> > Cc: Michael Reed <mdr@xxxxxxx>
> > Reviewed-by: Laurence Oberman <loberman@xxxxxxxxxx>
> > Reviewed-by: Lee Duncan <lduncan@xxxxxxxx>
> > ---
> > 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
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html