On Thu, Mar 03 2005, [EMAIL PROTECTED] wrote:
> Jens Axboe wrote:
> > On Thu, Mar 03 2005, Stuart Hayes wrote:
> >> I sent in this patch a few weeks ago, and never saw a response... I
> >> was wondering if there was a problem with it, or if I need to supply
> >> more info...? This patch is against 2.6.11-rc3. It makes sure that
> >> when
> >> ide_atapi_error() tries to end a failing ATAPI request after 2 reset
> >> attempts by calling drive->driver->end_request(), it will really be
> >> ended. Right now, a request that has a non-null rq->bio will not get
> >> ended,
> >> nor will rq->errors get cleared, so it will get retried forever with
> >> no more reset attempts.
> >
> > Indeed a problem. Would be nice to switch the ->end_request() to be
> > byte based like SCSI, would make it cleaner.
> >
> > But do you really need the !nsectors check? If ->data_len is 0, there
> > should not be a need to pass a non-zero sector count.
> >
>
> I wondered about the !nsectors check myself. I copied that code from
> the cdrom_end_request() function, and I assumed that it must have been
> there for some reason that wasn't immediately clear to me, so I didn't
> take it out. I figured performance in that code path wasn't critical.
>
> I can take it out if that's the only problem!
Lets just keep it, it sure doesn't hurt :)
> >> My fingers are crossed that this patch won't be mangled... I seem to
> >> have a problem with that...
> >
> > No such luck, it's mangled :). Looks like tabs turned to a single
> > space.
> > Maybe you have better luck with attaching the file.
>
> Here's another try at the patch. This time it is against 2.6.11, and I
> shortened the comments so I could send it from this email address
> without it getting mangled...
>
>
>
> --- ide-io.c.orig 2005-03-03 10:57:05.468669136 -0500
> +++ ide-io.c 2005-03-03 11:01:33.982848776 -0500
> @@ -486,9 +486,21 @@ static ide_startstop_t ide_ata_error(ide
> /* force an abort */
> hwif->OUTB(WIN_IDLEIMMEDIATE, IDE_COMMAND_REG);
>
> - if (rq->errors >= ERROR_MAX || blk_noretry_request(rq))
> - drive->driver->end_request(drive, 0, 0);
> - else {
> + if (rq->errors >= ERROR_MAX || blk_noretry_request(rq)) {
> + /*
> + * make sure request is fully ended--otherwise bio might
> get
> + * updated and the command will be retried without
> + * rq->errors getting reset to zero, which could cause
> us to
> + * get stuckin a loop with infinite retries without any
> more
> + * reset attempts (borrowed from cdrom_end_request)
> + */
> + int nsectors;
> + if (blk_pc_request(rq))
> + nsectors = (rq->data_len + 511) >> 9;
> + if (!nsectors)
> + nsectors = 1;
> + drive->driver->end_request(drive, 0, nsectors);
There's still a problem here, you are not initializing nsectors for
non-pc requests. And your comments wrap :)
int nsectors = rq->hard_nr_sectors;
if (blk_pc_request(rq))
nsectors = (rq->data_len + 511) >> 9;
if (!nsectors)
nsectors = 1;
...
Can you resend with that fixed up and with a Signed-off-by header?
--
Jens Axboe
-
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
