On Tue, Feb 12 2008 at 19:51 +0200, Christoph Hellwig <[EMAIL PROTECTED]> 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 [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to