* [PATCH 5/6] libata: Per device max command length checking
@ 2006-06-07 16:25 Brian King
2006-06-12 3:29 ` Jeff Garzik
0 siblings, 1 reply; 3+ messages in thread
From: Brian King @ 2006-06-07 16:25 UTC (permalink / raw)
To: jgarzik; +Cc: linux-ide, linux-scsi, brking
Currently, libata sets the scsi_host's max_cmd_len to
be the minimum of the max command lengths of all devices
attached to the same ATA port. This patch moves this checking
into libata so libata can check this on a per device
basis and still allows an ATA host to implement its
own host limit.
Signed-off-by: Brian King <brking@us.ibm.com>
---
libata-dev-bjking1/drivers/scsi/libata-core.c | 8 +-------
libata-dev-bjking1/drivers/scsi/libata-scsi.c | 6 ++++++
2 files changed, 7 insertions(+), 7 deletions(-)
diff -puN drivers/scsi/libata-core.c~libata_max_cmd_len drivers/scsi/libata-core.c
--- libata-dev/drivers/scsi/libata-core.c~libata_max_cmd_len 2006-06-07 10:54:09.000000000 -0500
+++ libata-dev-bjking1/drivers/scsi/libata-core.c 2006-06-07 10:54:09.000000000 -0500
@@ -1415,12 +1415,6 @@ static int ata_dev_configure(struct ata_
cdb_intr_string);
}
- ap->host->max_cmd_len = 0;
- for (i = 0; i < ATA_MAX_DEVICES; i++)
- ap->host->max_cmd_len = max_t(unsigned int,
- ap->host->max_cmd_len,
- ap->device[i].cdb_len);
-
/* limit bridge transfers to udma5, 200 sectors */
if (ata_dev_knobble(dev)) {
if (print_info)
@@ -5166,7 +5160,7 @@ static void ata_host_init(struct ata_por
host->max_lun = 1;
host->max_channel = 1;
host->unique_id = ata_unique_id++;
- host->max_cmd_len = 12;
+ host->max_cmd_len = ATAPI_CDB_LEN;
ap->lock = &host_set->lock;
ap->flags = ATA_FLAG_DISABLED;
diff -puN drivers/scsi/libata-scsi.c~libata_max_cmd_len drivers/scsi/libata-scsi.c
--- libata-dev/drivers/scsi/libata-scsi.c~libata_max_cmd_len 2006-06-07 10:54:09.000000000 -0500
+++ libata-dev-bjking1/drivers/scsi/libata-scsi.c 2006-06-07 10:54:09.000000000 -0500
@@ -2607,6 +2607,12 @@ static inline int __ata_scsi_queuecmd(st
{
int rc = 0;
+ if (unlikely(cmd->cmd_len > dev->cdb_len)) {
+ cmd->result = (DID_ABORT << 16);
+ done(cmd);
+ return 0;
+ }
+
if (dev->class == ATA_DEV_ATA) {
ata_xlat_func_t xlat_func = ata_get_xlat_func(dev,
cmd->cmnd[0]);
_
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH 5/6] libata: Per device max command length checking
2006-06-07 16:25 [PATCH 5/6] libata: Per device max command length checking Brian King
@ 2006-06-12 3:29 ` Jeff Garzik
2006-06-12 18:22 ` Brian King
0 siblings, 1 reply; 3+ messages in thread
From: Jeff Garzik @ 2006-06-12 3:29 UTC (permalink / raw)
To: Brian King; +Cc: linux-ide, linux-scsi
Brian King wrote:
> Currently, libata sets the scsi_host's max_cmd_len to
> be the minimum of the max command lengths of all devices
> attached to the same ATA port. This patch moves this checking
> into libata so libata can check this on a per device
> basis and still allows an ATA host to implement its
> own host limit.
>
> Signed-off-by: Brian King <brking@us.ibm.com>
> ---
>
> libata-dev-bjking1/drivers/scsi/libata-core.c | 8 +-------
> libata-dev-bjking1/drivers/scsi/libata-scsi.c | 6 ++++++
> 2 files changed, 7 insertions(+), 7 deletions(-)
>
> diff -puN drivers/scsi/libata-core.c~libata_max_cmd_len drivers/scsi/libata-core.c
> --- libata-dev/drivers/scsi/libata-core.c~libata_max_cmd_len 2006-06-07 10:54:09.000000000 -0500
> +++ libata-dev-bjking1/drivers/scsi/libata-core.c 2006-06-07 10:54:09.000000000 -0500
> @@ -1415,12 +1415,6 @@ static int ata_dev_configure(struct ata_
> cdb_intr_string);
> }
>
> - ap->host->max_cmd_len = 0;
> - for (i = 0; i < ATA_MAX_DEVICES; i++)
> - ap->host->max_cmd_len = max_t(unsigned int,
> - ap->host->max_cmd_len,
> - ap->device[i].cdb_len);
> -
> /* limit bridge transfers to udma5, 200 sectors */
> if (ata_dev_knobble(dev)) {
> if (print_info)
> @@ -5166,7 +5160,7 @@ static void ata_host_init(struct ata_por
> host->max_lun = 1;
> host->max_channel = 1;
> host->unique_id = ata_unique_id++;
> - host->max_cmd_len = 12;
> + host->max_cmd_len = ATAPI_CDB_LEN;
>
> ap->lock = &host_set->lock;
> ap->flags = ATA_FLAG_DISABLED;
> diff -puN drivers/scsi/libata-scsi.c~libata_max_cmd_len drivers/scsi/libata-scsi.c
> --- libata-dev/drivers/scsi/libata-scsi.c~libata_max_cmd_len 2006-06-07 10:54:09.000000000 -0500
> +++ libata-dev-bjking1/drivers/scsi/libata-scsi.c 2006-06-07 10:54:09.000000000 -0500
> @@ -2607,6 +2607,12 @@ static inline int __ata_scsi_queuecmd(st
> {
> int rc = 0;
>
> + if (unlikely(cmd->cmd_len > dev->cdb_len)) {
> + cmd->result = (DID_ABORT << 16);
> + done(cmd);
> + return 0;
> + }
:( do we really need to be adding this to the hot path, for such an
unlikely case?
Further, I wonder why this matters? Doesn't ipr present devices as one
per port?
Jeff
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH 5/6] libata: Per device max command length checking
2006-06-12 3:29 ` Jeff Garzik
@ 2006-06-12 18:22 ` Brian King
0 siblings, 0 replies; 3+ messages in thread
From: Brian King @ 2006-06-12 18:22 UTC (permalink / raw)
To: Jeff Garzik; +Cc: linux-ide, linux-scsi
Jeff Garzik wrote:
> Brian King wrote:
>> Currently, libata sets the scsi_host's max_cmd_len to
>> be the minimum of the max command lengths of all devices
>> attached to the same ATA port. This patch moves this checking
>> into libata so libata can check this on a per device
>> basis and still allows an ATA host to implement its
>> own host limit.
>>
>> Signed-off-by: Brian King <brking@us.ibm.com>
>> ---
>>
>> libata-dev-bjking1/drivers/scsi/libata-core.c | 8 +-------
>> libata-dev-bjking1/drivers/scsi/libata-scsi.c | 6 ++++++
>> 2 files changed, 7 insertions(+), 7 deletions(-)
>>
>> diff -puN drivers/scsi/libata-core.c~libata_max_cmd_len drivers/scsi/libata-core.c
>> --- libata-dev/drivers/scsi/libata-core.c~libata_max_cmd_len 2006-06-07 10:54:09.000000000 -0500
>> +++ libata-dev-bjking1/drivers/scsi/libata-core.c 2006-06-07 10:54:09.000000000 -0500
>> @@ -1415,12 +1415,6 @@ static int ata_dev_configure(struct ata_
>> cdb_intr_string);
>> }
>>
>> - ap->host->max_cmd_len = 0;
>> - for (i = 0; i < ATA_MAX_DEVICES; i++)
>> - ap->host->max_cmd_len = max_t(unsigned int,
>> - ap->host->max_cmd_len,
>> - ap->device[i].cdb_len);
>> -
>> /* limit bridge transfers to udma5, 200 sectors */
>> if (ata_dev_knobble(dev)) {
>> if (print_info)
>> @@ -5166,7 +5160,7 @@ static void ata_host_init(struct ata_por
>> host->max_lun = 1;
>> host->max_channel = 1;
>> host->unique_id = ata_unique_id++;
>> - host->max_cmd_len = 12;
>> + host->max_cmd_len = ATAPI_CDB_LEN;
>>
>> ap->lock = &host_set->lock;
>> ap->flags = ATA_FLAG_DISABLED;
>> diff -puN drivers/scsi/libata-scsi.c~libata_max_cmd_len drivers/scsi/libata-scsi.c
>> --- libata-dev/drivers/scsi/libata-scsi.c~libata_max_cmd_len 2006-06-07 10:54:09.000000000 -0500
>> +++ libata-dev-bjking1/drivers/scsi/libata-scsi.c 2006-06-07 10:54:09.000000000 -0500
>> @@ -2607,6 +2607,12 @@ static inline int __ata_scsi_queuecmd(st
>> {
>> int rc = 0;
>>
>> + if (unlikely(cmd->cmd_len > dev->cdb_len)) {
>> + cmd->result = (DID_ABORT << 16);
>> + done(cmd);
>> + return 0;
>> + }
>
> :( do we really need to be adding this to the hot path, for such an
> unlikely case?
>
> Further, I wonder why this matters? Doesn't ipr present devices as one
> per port?
ipr does present devices as one per port, but ipr can also have 'n' ports
per host adapter, so it cannot simply use the scsi_host's max_cmd_len.
The other option would be to move this check into the SAS only path
since it would be the only user that would need this since SATA
users can use scsi_host->max_cmd_len. I'll roll this into my patchset
so you can take a look...
Brian
--
Brian King
eServer Storage I/O
IBM Linux Technology Center
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2006-06-12 18:22 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-06-07 16:25 [PATCH 5/6] libata: Per device max command length checking Brian King
2006-06-12 3:29 ` Jeff Garzik
2006-06-12 18:22 ` Brian King
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).