Beginning of some updates to scsi mid layer

Linus,

I've got some things queued up to start sending through to you as they pass my testing here. However, I wanted to get some input from people to make sure that these are heading down paths people can agree with...

So, this first patch extends the slave_attach() function that Eric put as a stub in scsi.c long ago and then never used. The idea is actually sound, it was just never implemented. Specifically, the scsi upper layer knows when a device is ready to be used, but some of the limitations on that device are low level driver imposed (and sometimes the low level driver needs to know certain things about a device in order to act appropriately). This patch makes it so that any time we attach a new device into the scsi chain, we call the scsi low level driver's slave_attach() routine. This is where the low level driver should do things like look at the INQUIRY data to determine U160+ support, set queue depths, things like that. When we are removing the device, we call the (new) slave_detach() routine in the driver. The slave_detach will replace the revoke() routine once I've removed the usage of revoke() from the one and only driver that uses it.

Currently, the low level driver can:

1. Poke into the INQUIRY data for sync speed info
2. Set the queue depth on the device (using the new scsi_adjust_queue_depth function)
3. Allocate any needed structs and such for this device

Future plans include allowing drivers to:

4. Set the default timeout for the device (so RAID controllers can have their 60 second disk timeouts while non-RAID controllers can have their much shorter 5 to 10 second timeouts, really helps responsiveness of system in the face of any error conditions when using the non-RAID controllers)
5. Configure other queueing parameters (queue type, ordering capabilities, etc. as appropriate in light of other changes I have in mind)
6. Anything else that makes sense.

As mentioned, this adds a new scsi_adjust_queue_depth routine that lets the low level driver update the queue depth on a device at any point in time. So, when Justin Gibbs' driver allocates a queue depth of 253 commands on drive X, then finds out that drive X has a hard limit of 64, then we aren't continuing to waste 189 commands worth of allocated kernel memory that can never be effectively used. Future plans here include linking this into the bio queueing so that adjusting the mid layer queue depth does the "right thing" in regards to the bio layer (for all I know it does already, I haven't gotten into the bio->scsi interface yet).

Once this stuff is in place, then my next goal is to get the queueing code in the mid layer up to useable standards (the goal is to be able to rip the buts of the QUEUE_FULL and BUSY handling out of my driver and still have the system do the "right thing" instead of my driver having it's own complete internal queue mechanism including timers and such like it does now).

Side note: the work I'm doing on the aic7xxx_old driver here is simply because that's the easiest sandbox for me to verify my changes, it doesn't imply that I'll be updating the driver in any meaningful way.

Comments?

@@ -46,7 +46,9 @@
 	eh_host_reset_handler: NULL,				\
 	abort: aic7xxx_abort,					\
 	reset: aic7xxx_reset,					\
-	slave_attach: NULL,					\
+	select_queue_depths: NULL,				\
+	slave_attach: aic7xxx_slave_attach,			\
+	slave_detach: aic7xxx_slave_detach,			\
 	bios_param: aic7xxx_biosparam,				\
 	can_queue: 255,		/* max simultaneous cmds      */\
 	this_id: -1,		/* scsi id of host adapter    */\

@@ -64,6 +66,8 @@
 extern int aic7xxx_reset(Scsi_Cmnd *, unsigned int);
 extern int aic7xxx_abort(Scsi_Cmnd *);
 extern int aic7xxx_release(struct Scsi_Host *);
+extern int aic7xxx_slave_attach(Scsi_Device *);
+extern void aic7xxx_slave_detach(Scsi_Device *);
 
 extern const char *aic7xxx_info(struct Scsi_Host *);
 

@@ -485,6 +485,7 @@
 extern void scsi_bottom_half_handler(void);
 extern void scsi_release_commandblocks(Scsi_Device * SDpnt);
 extern void scsi_build_commandblocks(Scsi_Device * SDpnt);
+extern void scsi_adjust_queue_depth(Scsi_Device *, int, int);
 extern void scsi_done(Scsi_Cmnd * SCpnt);
 extern void scsi_finish_command(Scsi_Cmnd *);
 extern int scsi_retry_command(Scsi_Cmnd *);

@@ -562,10 +563,14 @@
 	wait_queue_head_t   scpnt_wait;	/* Used to wait if
 					   device is busy */
 	struct Scsi_Host *host;
+	void *host_device_ptr;	/* host private data ptr, can be alloc'ed on
+				   attach and free'ed on detach */
 	request_queue_t request_queue;
         atomic_t                device_active; /* commands checked out for device */
 	volatile unsigned short device_busy;	/* commands actually active on low-level */
 	Scsi_Cmnd *device_queue;	/* queue of SCSI Command structures */
+	unsigned char queue_depth;	/* How deep a queue to use */
+	unsigned char new_queue_depth;  /* If we need to raise/lower depth */
 
 	unsigned int id, lun, channel;
 

@@ -589,7 +594,6 @@
 	unsigned char current_tag;	/* current tag */
 	unsigned char sync_min_period;	/* Not less than this period */
 	unsigned char sync_max_offset;	/* Not greater than this offset */
-	unsigned char queue_depth;	/* How deep a queue to use */
 
 	unsigned online:1;
 	unsigned writeable:1;

@@ -860,6 +860,17 @@
 
 	scsi_release_commandblocks(SDpnt);
 
+	if(shpnt->hostt->slave_attach != NULL)
+		if((shpnt->hostt->slave_attach)(SDpnt) != 0) {
+			/*
+			 * Low level driver failed to attach this device,
+			 * we've got to kick it back out now :-(
+			 */
+			printk("scsi: scan_scsis_single: slave_attach failed, "
+				"marking OFF-LINE.\n");
+			SDpnt->online = FALSE;
+		}
+
 	/*
 	 * This device was already hooked up to the host in question,
 	 * so at this point we just let go of it and it should be fine.  We do need to

@@ -62,6 +62,7 @@
 EXPORT_SYMBOL(scsi_release_request);
 EXPORT_SYMBOL(scsi_wait_req);
 EXPORT_SYMBOL(scsi_do_req);
+EXPORT_SYMBOL(scsi_adjust_queue_depth);
 
 EXPORT_SYMBOL(scsi_report_bus_reset);
 EXPORT_SYMBOL(scsi_block_requests);

  Doug Ledford <[email protected]>     919-754-3700 x44233
         Red Hat, Inc. 
         1801 Varsity Dr.
         Raleigh, NC 27606

On Tue, Jun 18, 2002 at 08:47:53PM -0400, Doug Ledford wrote:

Doug -

The Scsi_Cmnd initialization should have its own function (maybe including allocation), called from scsi_adjust_queue_depth and your new code.

So do you think the current select_queue_depths should (eventually) be removed?

Not a comment on your patch, but why isn't the queue depth setting handled completely in the mid-layer?

The cmd_per_lun and can_queue fields are in Scsi_Host, and the mid-layer already checks for tagged queueing. I don't see what adavantage there is to having the low level adapters each set the queue depth in their own way. Plus, a lot of common (and not so common) code would go away.

And then scsi_adjust_queue_depth() could be called when not in interrupt context.

-- Patrick Mansfield

I disagree, but see below for why.

Yes, that's my goal. The current method is confusing to some driver writers as they end up seeing the same device multiple times when someone does any scsi-add-single-device magic on thier bus. It only lets you adjust the queue depth when a device is added, not when you need to. It's a bad design and needs to go away.

Because the mid layer doesn't have the information needed to do so. Queue depth settings are limited by two things. 1) the drive's ability to support tagged queue commands. 2) the driver/hardware's ability to send those commands. The driver's often used shared tags amongst all drives for efficiency reasons (my aic7xxx driver does for example). Details such as this impact what can and can not be done in terms of tagged queueing. Only the low level driver does/should have that information, so only the low level driver should control the tagged queueing. The mid layer is, for the most part, totally tag ignorant. It only needs to know that they are supported and it can send X of them, that's all. There isn't any real reason to try and make it control things. (Not yet anyway, but I am planning on adding one specific type of control to the mid layer in regards to tagged queueing, but that's still a few patches down the road)

Each low level driver *must* set the queue depth in its own way. A few examples should illustrate this for you quite nicely:

1) In the aic7xxx driver I have a 255 command limit per card. In host memory I have a set of card/command specific data structures. For each command that comes in, I fill in one of these structures and I then start the command. The tag value used in talking to the device is actually a direct index into this array of command structures. That way, when a device reconnects and tells the card what tag it is reconnecting, the card can download the saved command state immediately. This means that actual tag values used between drives *must* always be unique. Therefore, the mid layer is not in a position to assign the actual tag value for me, but only in a position to say "send this as a tagged command".

2) Currently (and this is broken BTW, but oh well), the aic7xxx driver does most of it's indexing into device specific structs based upon the target ID, but not based upon the lun. That means that when connecting the aic7xxx driver to an external SCSI RAID chassis that exports more than one drive as the same target ID and different luns, we have overlap in the data structures in the driver. Setting the queue depth for these overlapping devices requires internal knowledge of how the overlap effects things. Obviously, the mid layer can't know these details.

3) The aic7xxx driver is capable of setting up a waiting queue on the card itself. In this queue we may have up to 16 commands waiting to be sent to the device. Now, imagine that we already have 64 commands sent to a Seagate drive, and we have just queued up 16 more. When the first of those 16 commands connects to the Seagate drive, it might generate a QUEUE_FULL response. If so, in my interrupt handler, I pull the other 15 commands out of the waiting queue so I don't hit the drive with a bunch of commands when the drive can't take them. I also count exactly how many commands are *actually* on the drive and active so I can track whether or not the QUEUE_FULL messages always happen at the same depth. It is impossible to do this count without knowing intimate hardware details of the controller and its outgoing queue structure. This is also the only reliable way of detecting when a device has a hard queue depth limit or if it generates QUEUE_FULL messages at differing depths based upon current resource allocations on the drive itself. This happens at interrupt time because the command completion, where we get the QUEUE_FULL status, is obviously an interrupt.

This is totally backwards to what you say earlier. Earlier you want the mid layer to handle command queueing so that each driver doesn't have to write specific code for setting things, and here you advocate each driver writing its own method for calling adjust_queue_depth out of the interrupt context when instead this lazy queue depth setting handler is one of the few items that *could* be handled just in one place. I think your take on tagged queueing is about the perfect opposite of most implementation details.

  Doug Ledford <[email protected]>     919-754-3700 x44233
         Red Hat, Inc. 
         1801 Varsity Dr.
         Raleigh, NC 27606