Re: [PATCH] libata: device suspend/resume

[Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>]

Nice work Jens, some feedback below:

On 5/25/05, Jens Axboe <axboe@suse.de> 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)

> 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

> 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)

> 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


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?

Bartlomiej