Re: [PATCH 04/40] ide-scsi: fix Interrupt Reason checking inidescsi_pc_intr()

From: Borislav Petkov
Date: Mon May 19 2008 - 01:10:56 EST


On Sun, May 18, 2008 at 08:55:03PM +0200, Bartlomiej Zolnierkiewicz wrote:
> Set PC_FLAG_WRITING pc flag in idescsi_queue() (if needed)
> and then fix Interrupt Reason checking in idescsi_pc_intr().
>
> Cc: Borislav Petkov <petkovbb@xxxxxxxxx>
> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@xxxxxxxxx>
> ---
> drivers/scsi/ide-scsi.c | 17 +++++++++++------
> 1 file changed, 11 insertions(+), 6 deletions(-)
>
> Index: b/drivers/scsi/ide-scsi.c
> ===================================================================
> --- a/drivers/scsi/ide-scsi.c
> +++ b/drivers/scsi/ide-scsi.c
> @@ -427,7 +427,15 @@ static ide_startstop_t idescsi_pc_intr (
> printk(KERN_ERR "ide-scsi: CoD != 0 in idescsi_pc_intr\n");
> return ide_do_reset (drive);
> }
> - if (ireason & IO) {
> + if (((ireason & IO) == IO) == !!(pc->flags & PC_FLAG_WRITING)) {
> + /* Hopefully, we will never get here */
> + printk(KERN_ERR "%s: We wanted to %s, but the device wants us "
> + "to %s!\n", drive->name,
> + (ireason & IO) ? "Write" : "Read",
> + (ireason & IO) ? "Read" : "Write");
> + return ide_do_reset(drive);
> + }
> + if (!(pc->flags & PC_FLAG_WRITING)) {
> temp = pc->xferred + bcount;
> if (temp > pc->req_xfer) {
> if (temp > pc->buf_size) {
> @@ -436,7 +444,6 @@ static ide_startstop_t idescsi_pc_intr (
> "- discarding data\n");
> temp = pc->buf_size - pc->xferred;
> if (temp) {
> - pc->flags &= ~PC_FLAG_WRITING;
> if (pc->sg)
> idescsi_input_buffers(drive, pc,
> temp);
> @@ -457,15 +464,11 @@ static ide_startstop_t idescsi_pc_intr (
> printk (KERN_NOTICE "ide-scsi: The scsi wants to send us more data than expected - allowing transfer\n");
> #endif /* IDESCSI_DEBUG_LOG */
> }
> - }
> - if (ireason & IO) {
> - pc->flags &= ~PC_FLAG_WRITING;
> if (pc->sg)
> idescsi_input_buffers(drive, pc, bcount);
> else
> hwif->input_data(drive, NULL, pc->cur_pos, bcount);
> } else {
> - pc->flags |= PC_FLAG_WRITING;

Yeah, what was the driver doing turning on/off that flag in the irq handler? Are
those pc's getting reused in ide-scsi... I guess this is one of the bugs you
were talking about :).

> if (pc->sg)
> idescsi_output_buffers(drive, pc, bcount);
> else
> @@ -777,6 +780,8 @@ static int idescsi_queue (struct scsi_cm
>
> memset (pc->c, 0, 12);
> pc->flags = 0;
> + if (cmd->sc_data_direction == DMA_TO_DEVICE)
> + pc->flags |= PC_FLAG_WRITING;
> pc->rq = rq;
> memcpy (pc->c, cmd->cmnd, cmd->cmd_len);
> pc->buf = NULL;

--
Regards/Gruß,
Boris.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/