Re: [PATCH] [v2] aha152x cmnd->device oops

Juergen E. Fischer
2003-10-29T12:24:08Z

Hi Randy,

On Tue, Oct 28, 2003 at 16:26:10 -0800, Randy.Dunlap wrote:
> On Tue, 28 Oct 2003 14:50:28 -0800 Mike Christie <mikenc@us.ibm.com> wrote:
>
> | Hi Randy,
> |
> | > if(!(DONE_SC->SCp.Status & not_issued)) {
> | > - Scsi_Cmnd *cmnd = kmalloc(sizeof(Scsi_Cmnd), GFP_ATOMIC);
> | > + Scsi_Cmnd *cmnd = scsi_get_command(DONE_SC->device, GFP_ATOMIC);
> |
> | Do you need to add a scsi_put_command to balance this get when the
> | command completes?
> |
> | > + cmnd->device = ptr->device;
> |
> | Don't even need this now. scsi_get_command will set the device for you.
> |
> | Mike
>
> Hers's v3 of the aha152x patch. However, I'm not satisfied with it,
> so I'm not asking James to apply it. I don't know the state machine
> or the hardware well enough to understand and patch it.

> Things that I don't like about it:
>
> a. calling aha152x_internal_queue() with cmnd pointer in 2 places.
> Probably the second one should be NULL.
> b. Easy to confuse the state machine by changing just one variable,
> like DONE_SC (i.e., it's risky and I can't test it, although we
> do have a bug report and can ask that reporter to test it).
> c. int result = SCpnt->SCp.Status; /*FIXME*/
> SCp.Status is not valid here AFAIK, and I don't know how to get
> the current command status at this point.

There are 2 specially "commands" which are used internally in the driver
only (so I don't see a problem in using kmalloc instead of
scsi_get_command; but nevertheless ->device needs to be intialized
correctly, which is what scsi_get_command() does and the current code
fails to do).

The first is when a command returns with status CHECK CONDITION and the
driver needs to do a REQUEST SENSE to fetch sense data and add that to
the Ssci_Cmnd which resulted in the CHECK CONDITION. The internally
queued command fills the sense_buffer of the offending command and then
calls its ->scsi_done().

The second it when the controller needs to be resetted. Both are purely
internal to the driver.

I'll test this in the evening.

Jürgen

--
Phase 1: Where do you want to go today?
Phase 2: This is where you want to go today.
Phase 3: You're not going anywhere today.
-- seen on /.
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html