On Fri, May 27 2005, Bartlomiej Zolnierkiewicz wrote:
> Nice work Jens, some feedback below:
>
> On 5/25/05, Jens Axboe <axboe@xxxxxxx> wrote:
>
> > Index: drivers/ide/ide-floppy.c
> > ===================================================================
> > --- 137318b273db26b889675101fbd02d2e84cae5e3/drivers/ide/ide-floppy.c (mode:100644)
> > +++ uncommitted/drivers/ide/ide-floppy.c (mode:100644)
>
> > @@ -1311,9 +1311,9 @@
> > }
> > pc = idefloppy_next_pc_storage(drive);
> > idefloppy_create_rw_cmd(floppy, pc, rq, block);
> > - } else if (rq->flags & REQ_SPECIAL) {
> > + } else if (blk_special_request(rq))
> > pc = (idefloppy_pc_t *) rq->buffer;
> > - } else if (rq->flags & REQ_BLOCK_PC) {
> > + } else if (blk_fs_request(rq)) {
> > pc = idefloppy_next_pc_storage(drive);
> > if (idefloppy_blockpc_cmd(floppy, pc, rq)) {
> > idefloppy_do_end_request(drive, 0, 0);
>
> blk_pc_request(rq)
Indeed, thanks!
> > Index: drivers/ide/ide-io.c
> > ===================================================================
> > --- 137318b273db26b889675101fbd02d2e84cae5e3/drivers/ide/ide-io.c (mode:100644)
> > +++ uncommitted/drivers/ide/ide-io.c (mode:100644)
>
> > @@ -948,9 +948,10 @@
> > if (!drive->special.all) {
> > ide_driver_t *drv;
> >
> > - if (rq->flags & (REQ_DRIVE_CMD | REQ_DRIVE_TASK))
> > + if (rq->cmd_type == REQ_TYPE_ATA_CMD ||
> > + rq->cmd_type == REQ_TYPE_ATA_TASK)
> > return execute_drive_cmd(drive, rq);
> > - else if (rq->flags & REQ_DRIVE_TASKFILE)
> > + else if (rq->cmd_type == REQ_TYPE_ATA_TASKFILE)
> > return execute_drive_cmd(drive, rq);
> > else if (blk_pm_request(rq)) {
> > #ifdef DEBUG_PM
>
> || rq->cmd_type == REQ_TYPE_ATA_TASKFILE while at it
Didn't even notice that, fixed.
> > Index: drivers/scsi/ide-scsi.c
> > ===================================================================
> > --- 137318b273db26b889675101fbd02d2e84cae5e3/drivers/scsi/ide-scsi.c (mode:100644)
> > +++ uncommitted/drivers/scsi/ide-scsi.c (mode:100644)
>
> > @@ -370,12 +370,12 @@
> > u8 *scsi_buf;
> > unsigned long flags;
> >
> > - if (!(rq->flags & (REQ_SPECIAL|REQ_SENSE))) {
> > + if (!blk_special_request(rq) || !blk_sense_request(rq)) {
> > ide_end_request(drive, uptodate, nrsecs);
> > return 0;
> > }
>
> !blk_special_request(rq) && !blk_sense_request(rq)
Thanks.
> > Index: drivers/scsi/scsi_lib.c
> > ===================================================================
> > --- 137318b273db26b889675101fbd02d2e84cae5e3/drivers/scsi/scsi_lib.c (mode:100644)
> > +++ uncommitted/drivers/scsi/scsi_lib.c (mode:100644)
>
> > @@ -922,7 +922,7 @@
> > /*
> > * if this is a rq->data based REQ_BLOCK_PC, setup for a non-sg xfer
> > */
> > - if ((req->flags & REQ_BLOCK_PC) && !req->bio) {
> > + if (blk_pc_request(req) && req->bio) {
> > cmd->request_bufflen = req->data_len;
> > cmd->request_buffer = req->data;
> > req->buffer = req->data;
>
> !req->bio
Yeah, this one I already found while trying to boot the patch, SG_IO
barfed quickly without it.
> I assume they you've verified that there are no odd cases possible like
> rq->flags = REQ_DRIVE_TASKFILE | REQ_BAR_FLUSH or rq->flags = REQ_PC
> (without REQ_BLOCK_PC) which would be obviously broken by this patch?
Not fully verified yet, there's some work to be done there. REQ_PC
never had REQ_BLOCK_PC set, I want to unify those two in ide-cd though.
Should be pretty trivial, I hope to even kill of the old pc_interrupt
handling and just use newpc_interrupt at the same time.
But that really should be on top of this patch, done afterwards!
--
Jens Axboe
-
: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
