On Mon, Apr 10, 2006 at 03:04:47PM -0500, Brian King wrote: > Patrick Mansfield wrote: > > I should not have used "untagged", that is misleading and a problem with > > current scsi core, where we reference tcq, tags, and don't seem to mention > > task attributes. > > > > But LLDD can override anything in slave_configure. > > I guess my biggest problem with this part of the patch is that it prevents an > LLDD that wants this behavior from being able to use scsi_adjust_queue_depth > to set the queue depth, whether it be in slave_configure, change_queue_depth, etc. > > > Also, it looks like we could safely use cmd_per_lun as the default > > queue_depth, rather than setting it to 1 as done in my previous > > post/patch. > > Ok. If we do that and also allow scsi_adjust_queue_depth > to adjust the queue depth when tagged == 0, as is allowed today, > then I think most of my objections to the patch should disappear, > although it may require me to make a couple ipr changes. Yeh, and then there is not really anything left of my patch, small as it was, just the check for BQUE and calling scsi_activate_tcq(). And calling scsi_activate_tcq if tagged_supported should be a separate patch that also includes any required driver changes (like with ipr to call scsi_deactivate_tcq, and an audit of all other drivers ...). Really I was confused by scsi_mid_low_api.txt and code mentioning tags along with cmd_per_lun and LLD abilities, like this: The mid level invokes scsi_adjust_queue_depth() with tagged queuing off and "cmd_per_lun" for that host as the queue length. These settings can be overridden by a slave_configure() supplied by the LLD. But it makes no sense to set "tagged queuing off" on transports like FCP. So, it is fine to use cmd_per_lun as the default queue_depth, without any adjustments (in slave_configure) via scsi_adjust_queue_depth or scsi_activate_tcq. -- Patrick Mansfield - : send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
