http://bugzilla.kernel.org/show_bug.cgi?id=13399 --- Comment #24 from Anonymous Emailer <anonymous@xxxxxxxxxxxxxxxxxxxx> 2009-06-14 13:02:45 --- Reply-To: petkovbb@xxxxxxxxxxxxxx Hi, On Sun, Jun 14, 2009 at 02:32:10PM +0200, Bartlomiej Zolnierkiewicz wrote: > On Sunday 14 June 2009 12:06:06 Borislav Petkov wrote: > > Hi, > > > > On Sat, Jun 13, 2009 at 06:59:53PM +0200, Bartlomiej Zolnierkiewicz wrote: > > > > --- Comment #20 from Borislav Petkov <bbpetkov@xxxxxxxx> 2009-06-13 16:29:05 --- > > > > Hi Bart, > > > > > > > > thanks for analyzing this. > > > > > > > > I'm staring at the ATA_DRQ == 0 part in cdrom_newpc_intr: > > > > > > > > } else if (!blk_pc_request(rq)) { > > > > ide_cd_request_sense_fixup(drive, cmd); > > > > /* complain if we still have data left to transfer */ > > > > uptodate = cmd->nleft ? 0 : 1; > > > > if (uptodate == 0) > > > > rq->cmd_flags |= REQ_FAILED; > > > > } > > > > goto out_end; > > > > } > > > > > > > > so, in our case ide_cd_error_cmd() kills the rq prematurely and that's > > > > why ide_complete_rq() oopses later. And this is caused by uptodate == > > > > > > Nope, it is block layer that kills it prematurely. > > > > ok, I still need to understand the whole code flow properly so please > > bare with me. > > > > I got misled by the __blk_end_request() thing: am I right to assume that > > you were using it to give a better example where it is more clear that > > the block layer really kills rq->bio-less requests? > > > > Because we don't hit __blk_end_request from ide_cd_error_cmd() (or > > ide_complete_rq() too, for that matter) - we do rather: > > > > ide_cd_error_cmd() does ide_complete_rq(), which ends up doing > > blk_end_request(), then blk_end_io() and the rq->bio thing is checked > > in end_that_request_data(). Which is actually the same thing but done > > slightly differently. > > Yes, you're seeing block layer updates from 2 days ago. Nope, I'm staring at .30-rc7 sources. I guess those updates are a bit older. > > > There are two issues here: > > > > > > * OOPS (*the*regression* which should be taken care of first (cause: > > > unexpected interaction between ide/block) > > > > I'm thinking something like > > > > if (uptodate == 0 && !(OK_STAT(stat, 0, BAD_STAT))) > > ide_cd_error_cmd(drive, cmd); > > Those are two separate issues, please stop mixing them. > > AFAICS ide-cd has been always failing !uptodate requests so the latter > issue is nothing new. Which means that it is really not the right time > to be scratching our heads about it while the former problem has been > seriously affecting people for weeks now and also managed to slip into > the final 2.6.30 release.. Ok, let's do that: @Hans: can you apply the following ontop of the debugging patch and test and send us the whole dmesg, thanks. -- diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c index 6f728da..219c303 100644 --- a/drivers/ide/ide-cd.c +++ b/drivers/ide/ide-cd.c @@ -764,7 +764,7 @@ out_end: rq->errors = -EIO; } - if (uptodate == 0) + if (uptodate == 0 && rq->bio) ide_cd_error_cmd(drive, cmd); /* make sure it's fully ended */ -- > > > * handling of non-fully completed requests with "good" status (cause: > > > stupid hardware) > > > > The non-intrusive solution, IMHO, would be to add another quirk flag for > > such a device (SONY DVD-ROM DDU1615) and do not complete the rq in that > > case, aka no partial completion for packet commands to that device. I'm > > wondering what else is broken with it especially if you're requiring > > bigger buffers like the capacities page. > > > > So, how about an ide_cd_complete_rq() helper which hides such special > > cases and is called as an indirection at the end of the irq handler? > > I can't tell much without seeing the concept with more details. > > IOW please send patches, that is the best way to discuss things. Ok. -- Configure bugmail: http://bugzilla.kernel.org/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching the assignee of the bug.-- 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
