Hi, On Thu, Mar 5, 2009 at 1:12 PM, Bartlomiej Zolnierkiewicz <bzolnier@xxxxxxxxx> wrote: > > Hi, > > On Wednesday 04 March 2009, Borislav Petkov wrote: >> ... since some do not transfer any data from the drive and >> rq->buffer is unset leading to an OOPS when checking VM translations >> (CONFIG_DEBUG_VIRTUAL). >> >> Tested-by: Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx> >> Signed-off-by: Borislav Petkov <petkovbb@xxxxxxxxx> > > Thanks for fixing it but I worry that the patch is not entirely correct > (why o why did you have to throw in unrelated changes...?) What unrelated changes? This is the ide-cd fix for when mapping a NULL rq->buffer pointer to an sg. If you mean the [2/3] patch, I had to remove the partial completions for ide-floppy because it was OOPSing on NULL rq-pointer in the interrupt handler. I sent you a pretty detailed OOPS along with backtracking to the offending code line, remember? Also, for reasons of bisectability. > also ideally there should have been two patches: > - minimal bugfix for the buggy patch > - unification/cleanup of PIO sg setup > >> --- >> drivers/ide/ide-atapi.c | 9 ++++++++- >> drivers/ide/ide-cd.c | 4 ---- >> drivers/ide/ide-floppy.c | 4 ---- >> 3 files changed, 8 insertions(+), 9 deletions(-) >> .. >> diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c >> index 091d414..106cacb 100644 >> --- a/drivers/ide/ide-cd.c >> +++ b/drivers/ide/ide-cd.c >> @@ -916,10 +916,6 @@ static ide_startstop_t ide_cd_do_request(ide_drive_t *drive, struct request *rq, >> >> cmd.rq = rq; >> >> - ide_init_sg_cmd(&cmd, >> - blk_fs_request(rq) ? (rq->nr_sectors << 9) : rq->data_len); >> - ide_map_sg(drive, &cmd); >> - >> return ide_issue_pc(drive, &cmd); >> out_end: >> nsectors = rq->hard_nr_sectors; >> diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c >> index 2f8f453..b91deef 100644 >> --- a/drivers/ide/ide-floppy.c >> +++ b/drivers/ide/ide-floppy.c >> @@ -281,10 +281,6 @@ static ide_startstop_t ide_floppy_do_request(ide_drive_t *drive, >> cmd.tf_flags |= IDE_TFLAG_WRITE; >> >> cmd.rq = rq; >> - >> - ide_init_sg_cmd(&cmd, pc->req_xfer); > > There was a reason for using ->req_xfer. Yeah, this is also one of those painful places where I cringe everytime I have to look at. We finally have to sit down and decide which is which; sometimes pc->req_xfer _is_ rq->data_len (idefloppy_blockpc_cmd()) and sometimes obviously not. I'll take a look at that whole mess next and try to come up with something more clean. > I don't see where rq->data_len is set for REQ_TYPE_SPECIAL requests > (ide_queue_pc_tail() and friends)? me neither :(. >> - ide_map_sg(drive, &cmd); >> - >> pc->rq = rq; >> >> return ide_floppy_issue_pc(drive, &cmd, pc); Man, this sucks! -- Regards/Gruss, Boris -- 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
