On Tue, Feb 12 2008 at 19:51 +0200, Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote:
> On Sun, Feb 10, 2008 at 09:12:02PM +0200, Boaz Harrosh wrote:
>> @@ -525,6 +516,7 @@ int scsi_dispatch_cmd(struct scsi_cmnd *cmd)
>> unsigned long flags = 0;
>> unsigned long timeout;
>> int rtn = 0;
>> + unsigned cmd_len;
>>
>> /* check if the device is still usable */
>> if (unlikely(cmd->device->sdev_state == SDEV_DEL)) {
>> @@ -606,9 +598,17 @@ int scsi_dispatch_cmd(struct scsi_cmnd *cmd)
>> * Before we queue this command, check if the command
>> * length exceeds what the host adapter can handle.
>> */
>> - if (CDB_SIZE(cmd) > cmd->device->host->max_cmd_len) {
>> + cmd_len = cmd->cmd_len;
>> + if (!cmd_len) {
>> + BUG_ON(cmd->cmnd[0] == VARIABLE_LENGTH_CMD);
>> + cmd_len = COMMAND_SIZE((cmd)->cmnd[0]);
>> + }
>
> This looks odd to me. Shouldn't we make sure cmd_len is always
> initialized in a single place for either varlen or fixed length
> commands and not have things like this?
>
I used to have a BUG_ON(!cmd_len) here at around the 2.6.20 kernels
And it would trigger. I'm not sure exactly how. Through a retransmit
or something.
Reinspecting all command submission paths, I agree with you that it
should not happen anymore. I will look at it some more and run tests.
Perhaps if this code will sit in -mm tree for a while I can put the
BUG_ON back and see if it triggers again. What do you recommend?
Boaz
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
