Hello, Borislav Petkov wrote: > I hit that too when debugging an ide-tape problem a user has > (http://bugzilla.kernel.org/show_bug.cgi?id=12874). However, this is not the > proper solution since, currently, ide-tape stuffs all packet commands in > rq->buffer or rq->special now after your changes. It has to get them out of > there in the ->do_request callback and set drive->pc to point to the current > packet command in flight through the IRQ handler. And since ide_tape_issue_pc() > is called by the ->do_request callback we should have the drive->pc always > valid. > > How about something like that instead: > > diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c > index 4e6181c..171dbcd 100644 > --- a/drivers/ide/ide-tape.c > +++ b/drivers/ide/ide-tape.c > @@ -792,6 +792,9 @@ static ide_startstop_t > idetape_do_request(ide_drive_t *drive, > struct request *postponed_rq = tape->postponed_rq; > u8 stat; > > + if (rq->cmd_type == REQ_TYPE_SPECIAL) > + drive->pc = (struct ide_atapi_pc *) rq->buffer; > + > debug_log(DBG_SENSE, "sector: %llu, nr_sectors: %lu," > " current_nr_sectors: %u\n", > (unsigned long long)rq->sector, rq->nr_sectors, I don't really care one way or the other but the error condition itself looked somewhat pointless to me, so I just killed it. Is the error messasge really worth guaranteeing drive->pc can be accessed after completion? That's a nasty and fragile guarantee. If such check is really necessary, the proper way to do it would be recording whether the last command was request sense in some persistent data structure not trying to access a data structure which the code doesn't own anymore. Thanks. -- tejun -- 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
