PATCH [5/15] qla2xxx: SG tablesize update

ChangeSet
1.1661 04/02/28 22:35:59 [email protected] +3 -0 Limit SG entry size to make more efficient use of the size contraints of the request-queue.

This SG entry size is inline with the size specified in the 6.x and 7.x series drivers.

drivers/scsi/qla2xxx/qla_os.c
1.9 04/02/28 22:35:52 [email protected] +1 -1 Limit SG entry size to make more efficient use of the size contraints of the request-queue.

drivers/scsi/qla2xxx/qla_init.c
1.8 04/02/28 22:35:52 [email protected] +4 -1 Limit the size of the buffer used to load firmware.

drivers/scsi/qla2xxx/qla_def.h
1.8 04/02/28 22:35:52 [email protected] +1 -1 Increase an ISP's request queue size.

 drivers/scsi/qla2xxx/qla_def.h  |    2 +-
 drivers/scsi/qla2xxx/qla_init.c |    5 ++++-
 drivers/scsi/qla2xxx/qla_os.c   |    2 +-
 3 files changed, 6 insertions(+), 3 deletions(-)

ftp://ftp.qlogic.com/outgoing/linux/patches/8.x/8.00.00b11k/14_sg_resize.patch

diff -Nru a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
@@ -209,7 +209,7 @@
 #define MAX_OUTSTANDING_COMMANDS	1024
 
 /* ISP request and response entry counts (37-65535) */
-#define REQUEST_ENTRY_CNT		1024	/* Number of request entries. */
+#define REQUEST_ENTRY_CNT		2048	/* Number of request entries. */
 #define RESPONSE_ENTRY_CNT_2100		64	/* Number of response entries.*/
 #define RESPONSE_ENTRY_CNT_2300		512	/* Number of response entries.*/
 

diff -Nru a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
@@ -603,7 +603,10 @@
 	ha->product_id[3] = mb[4];
 
 	/* Adjust fw RISC transfer size */
-	ha->fw_transfer_size = REQUEST_ENTRY_SIZE * REQUEST_ENTRY_CNT;
+	if (REQUEST_ENTRY_CNT > 1024)
+		ha->fw_transfer_size = REQUEST_ENTRY_SIZE * 1024;
+	else
+		ha->fw_transfer_size = REQUEST_ENTRY_SIZE * REQUEST_ENTRY_CNT;
 
 	if (IS_QLA2200(ha) &&
 	    RD_MAILBOX_REG(ha, reg, 7) == QLA2200A_RISC_ROM_VER) {

diff -Nru a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
@@ -166,7 +166,7 @@
 	.can_queue		= REQUEST_ENTRY_CNT+128,
 	.cmd_per_lun		= 3,
 	.use_clustering		= ENABLE_CLUSTERING,
-	.sg_tablesize		= SG_ALL,
+	.sg_tablesize		= 32,
 
 	/*
 	 * The RISC allows for each command to transfer (2^32-1) bytes of data,

The explanation of why this was done (request queue limitations, huh?) sounded pretty bogus as well.

Jens Axboe

Actually, to be fair, I think he means the qlogic request queue, not the block one.

The Qla chips are rather weird in that they have a single issue queue (whose size you can vary) but whose entry formats are fixed. If I remember correctly, an initial command can have 4 SG elements, but a follow on entry can have 7 (not sure of the figures). But anyway, large SG commands end up having to find multiple entries in this queue (and being a single issue queue for the entire card, it has to be mutexed while you search). The more resources you need, the more difficult the search and the more contention you generate on the resource mutex.

I talked to Arjan a bit about this type of card and how best to handle it in our current infrastructure, but I haven't got any further than thinking about it.

James

Ah, makes a lot more sense now, thanks :-)

Jens Axboe

With 2.6.3 on a ppc64 box connected to a shark with 8 LUNs I could easily consume all the request slots on the card. At this point we spent

realising it wont fit and doing a pci_unmap_sg.

As Andrew points out, the driver shouldnt be doing this constant map/unmap stuff, but is there some way for a scsi device driver to tell the layers above it to temporarily leave it alone?

Anton

Well, since the pool is fixed size, can't the driver do some rule of thumb check before it actually gets to the map_sg (perhaps even pre-reserve the slots before mapping)?

Mappings can be very limited in certain machines. I really don't think we want precious mapping resources sitting in an issue queue in the mid-layer just because the qla driver couldn't manage them efficiently.

The layering rules say that the driver is responsible for getting the IOMMU resources with dma_map_sg before it places the I/O in-flight and should free them ASAP after the I/O returns.

All drivers have to manage their issue slots efficiently. It really sounds like the root cause of this problem is mapping too early.

If you think you don't know how many resource slots you'll need, that's in scsi_cmnd->request->nr_hw_segments (give or take, this is always guaranteed to be at or over the actual number dma_map_sg eventually returns). So: fix the qlogic mapping routines.

James

Yes I agree the driver should be fixed. My question was more a response to the Jens' query about why the SG limit was introduced.

Lets ignore the pci_map_sg problem and assume the qlogic driver did the right thing. Whats to stop the upper layers from continually trying to queue things when we run out of request slots? If its just sg_tablesize then we are stuck with the current compromise (ie set it to something reasonably low).

Anton

Because on this type of failure, the driver will return SCSI_MLQUEUE_HOST_BUSY from its queuecommand routine. This will tell the mid layer not to try any more commands for the entire host until an outstanding one returns (thus hopefully freeing up enough resources).

James

That makes sense, it sounds like the qlogic could do that and remove the SG limit.

Anton

The block layer is quite simple, really.

You have an asynchronous event, "start queue drain."

And the block layer won't touch your queue again until you have removed all requests from the queue. Once you have removed all requests from the queue, the cycle repeats.

It is up to the block driver (and/or SCSI layer in this case) to stop taking requests off the queue, when it can take no more. And it is up to that same driver to start taking requests again, when hardware resources are once again available.

So, the block layer supports this hardware situation just fine... it's a matter of getting SCSI to understand that, I suppose :)

The issue of hardware-global resources constraining multiple device queues is one that requires a bit of thought. Not impossible... I did it in my Carmel driver. But so far, I have not seen any _real_ solutions excepts for mine. All the others have been hacks -- such as "hardware queue size 1024 for all devices, so limit each device to '1024 / n_devices' requests at a time."

Jeff

We already do all of that, if you look ...

The place where all of this becomes rather unoptimal for SCSI is in an SMP where we get multiple queues racing on multiple CPUs for different devices then having to be coalesced into a single queue for the host adapter. It causes cache line bouncing like you wouldn't believe...

We do this too.

If you look int the scsi_host structure you see can_queue, which is the maximum number of total requests we allow to the entire host. Then there's queue_depth in scsi_device, which is the maximum number of requests to the individual devices. We even have a very simplistic fairness scheme to prevent one device hogging all of the host queue elements.

For dynamic resource situations we have the queuecommand return codes

SCSI_MLQUEUE_HOST_BUSY which means the entire host is temporarily out of resources and causes the mid layer to hold off all commands for that host until we get one back from any device on the host and

SCSI_MLQUEUE_DEVICE_BUSY which means that the device queue is temporarily out of resources, hold off all commands to that device until one returns from that device.

James

[...]

[...]

Oh, no argument. I use some of that stuff in libata, too.

These are features, however are not specific to SCSI. They are just thin wrappers on top of the block layer, and would be more generally useful one layer up... :)

Jeff

The block layer can handle this type of hardware just fine ;-)

In my SATA hacking I am finding several devices that one must constrain based on hardware-global resources, rather than just TCQ (or lack thereof).

Jeff

I'm sorry, I must have missed the multiple queues down to a single HBA queue API in the block layer, which one is it again?

James

Not saying the block layer provides this capability, simply that it is possible in any block driver. It required < 20 LOC in carmel.c to do this in a hardware-specific fashion, so it seemed unnecessary to invent a new API... I would like to see something similar in cciss.c, in fact...

Jeff