PATCH [0/8] qla2xxx: Summary of changes...

PATCH [0/8] qla2xxx: Summary of changes...

[Andrew Vasquez]

James,

Please apply the following patches I'm about to send to update the
qla2xxx driver.  A summary of the patches can be found towards the end
of this message.

With the exception of the rather large firmware image update (direct
pointer to bzip'd patch listed), subsequent messages will contain
inlined patches.

All patches can be found at the following URL:

        ftp://ftp.qlogic.com/outgoing/linux/patches/8.x/8.00.00b21k/

Regards,
Andrew Vasquez
QLogic Corporation


ChangeSet@1.1871, 2004-09-04 23:19:51-07:00, andrew.vasquez@qlogic.com
  PCI posting fixes.

ChangeSet@1.1872, 2004-09-06 07:34:57-07:00, andrew.vasquez@qlogic.com
  Dynamically resize request-queue during initialization based
  on the amount of memory available to the ISP.

ChangeSet@1.1873, 2004-09-06 11:34:15-07:00, andrew.vasquez@qlogic.com
  Rework driver DMA allocations to use the DMA pool APIs
  to minimize potential run-time allocation failures.

ChangeSet@1.1874, 2004-09-06 11:47:07-07:00, andrew.vasquez@qlogic.com
  Misc. small fixes:
  
          o Always set an TCQ modifier for each command submitted.
          o Always (re)-initialize NVRAM during execution.
          o Add pci_disable_device() calls to proper locations during
            pci-probe failure and pci-remove.
          o Do not begin processing until the DPC has been marked
            'active'.
          o Fix off-by-one error while copying sense-data.
          o Fix bus-reset issue during back-door execution.

ChangeSet@1.1875, 2004-09-06 11:51:36-07:00, andrew.vasquez@qlogic.com
  Rework interrupt-handler registration in anticipation of future
  chip types.

ChangeSet@1.1876, 2004-09-06 11:57:40-07:00, andrew.vasquez@qlogic.com
  Misc. small enhancements:
  
          o Add 're-scan' and 'initiate lip' support via /proc
            similar to the 7.x series drivers.
          o Add module parameter ql2xprocessnotready, to selectively
            enable/disable the driver's internal NOT_READY check-
            condition handling.

ChangeSet@1.1877, 2004-09-06 12:02:32-07:00, andrew.vasquez@qlogic.com
  Resync with latest released firmware -- 3.03.02

ChangeSet@1.1878, 2004-09-06 12:04:22-07:00, andrew.vasquez@qlogic.com
  Update version number to 8.00.00b21-k.

Re: PATCH [0/8] qla2xxx: Summary of changes...

[Arjan van de Ven]

[-- Attachment #1: Type: text/plain, Size: 806 bytes --]

On Tue, 2004-09-07 at 06:26, Andrew Vasquez wrote:
> James,
> 
> Please apply the following patches I'm about to send to update the
> qla2xxx driver.  A summary of the patches can be found towards the end
> of this message.


Hi,

I found a "funky" bug the other day with this driver; I was testing on a
RAID device of which I won't mention the vendor, but it had the habit of
returning QUEUE FULL when it's internal queue was full, with multiple
luns active that could even be on a lun which had exactly zero
outstanding IO already. The qlogic driver happily takes the number of
outstanding commands (0) and subtracts 1 from it to guestimate the
max... needless to say that didn't go down to well :-) Sounds like a
bound check would make sense here...

Greetings,
   Arjan van de Ven

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: PATCH [0/8] qla2xxx: Summary of changes...

[James Bottomley]

On Tue, 2004-09-07 at 02:00, Arjan van de Ven wrote:
> I found a "funky" bug the other day with this driver; I was testing on a
> RAID device of which I won't mention the vendor, but it had the habit of
> returning QUEUE FULL when it's internal queue was full, with multiple
> luns active that could even be on a lun which had exactly zero
> outstanding IO already. The qlogic driver happily takes the number of
> outstanding commands (0) and subtracts 1 from it to guestimate the
> max... needless to say that didn't go down to well :-) Sounds like a
> bound check would make sense here...

Actually, this isn't at all unusual.  RAID devices often have a single
memory pool for all the LUN queues.  Heavy activity on a set of LUNs can
lead to no resources for a command on others.

Although we have queue full tracking, predicting the behaviour on
devices with coupled queues like this is impossible.

However, I thought the qlogic driver used the mid-layer queue full
tracking, which takes all of this into account and won't adjust the
depth below a certain number (8 I think).

James

Re: PATCH [0/8] qla2xxx: Summary of changes...

[Andrew Vasquez]

On Tue, 07 Sep 2004, James Bottomley wrote:

> On Tue, 2004-09-07 at 02:00, Arjan van de Ven wrote:
> > I found a "funky" bug the other day with this driver; I was testing on a
> > RAID device of which I won't mention the vendor, but it had the habit of
> > returning QUEUE FULL when it's internal queue was full, with multiple
> > luns active that could even be on a lun which had exactly zero
> > outstanding IO already. The qlogic driver happily takes the number of
> > outstanding commands (0) and subtracts 1 from it to guestimate the
> > max... needless to say that didn't go down to well :-) Sounds like a
> > bound check would make sense here...
> 
> Actually, this isn't at all unusual.  RAID devices often have a single
> memory pool for all the LUN queues.  Heavy activity on a set of LUNs can
> lead to no resources for a command on others.
> 
> Although we have queue full tracking, predicting the behaviour on
> devices with coupled queues like this is impossible.
> 


> However, I thought the qlogic driver used the mid-layer queue full
> tracking, which takes all of this into account and won't adjust the
> depth below a certain number (8 I think).
> 

It does.  Relevant code from qla_isr.c:

	case CS_QUEUE_FULL:

		...

                /* Adjust queue depth */
                ret = scsi_track_queue_full(cp->device,
                    sp->lun_queue->out_cnt - 1);
                if (ret) {
			...

Now, sp->lun_queue->out_cnt at this point should at least be >= 1, so
conceivably we could be passing 0 for the second argument.  But,
scsi_track_queue_full() as James points out should handle this and
drop the depth to cmd_per_lun.

		...
		if (sdev->last_queue_full_depth < 8) {
			/* Drop back to untagged */
			scsi_adjust_queue_depth(sdev, 0, sdev->host->cmd_per_lun);
			return -1;
		}

Do we want checks for this case in all LLDDs who use the mid-layer
API?

Regards,
Andrew Vasquez