Re: [dm-devel] [PATCH 02/23] block: remove the blk_execute_rq return value
On Thu, 2017-04-20 at 16:02 +0200, Christoph Hellwig wrote: > The function only returns -EIO if rq->errors is non-zero, which is not > very useful and lets a large number of callers ignore the return value. > > Just let the callers figure out their error themselves. Reviewed-by: Bart Van Assche -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH 02/23] block: remove the blk_execute_rq return value
The function only returns -EIO if rq->errors is non-zero, which is not very useful and lets a large number of callers ignore the return value. Just let the callers figure out their error themselves. Signed-off-by: Christoph Hellwig Reviewed-by: Johannes Thumshirn --- block/blk-exec.c | 8 +--- block/scsi_ioctl.c | 3 ++- drivers/block/virtio_blk.c | 3 ++- drivers/cdrom/cdrom.c| 3 ++- drivers/ide/ide-atapi.c | 3 ++- drivers/ide/ide-cd.c | 3 ++- drivers/ide/ide-cd_ioctl.c | 3 ++- drivers/ide/ide-devsets.c| 4 ++-- drivers/ide/ide-disk.c | 3 +-- drivers/ide/ide-ioctls.c | 7 --- drivers/ide/ide-park.c | 3 ++- drivers/ide/ide-pm.c | 3 ++- drivers/ide/ide-taskfile.c | 4 ++-- drivers/scsi/osd/osd_initiator.c | 5 - fs/nfsd/blocklayout.c| 5 +++-- include/linux/blkdev.h | 2 +- 16 files changed, 34 insertions(+), 28 deletions(-) diff --git a/block/blk-exec.c b/block/blk-exec.c index 8cd0e9bc8dc8..afa383248c7c 100644 --- a/block/blk-exec.c +++ b/block/blk-exec.c @@ -92,11 +92,10 @@ EXPORT_SYMBOL_GPL(blk_execute_rq_nowait); *Insert a fully prepared request at the back of the I/O scheduler queue *for execution and wait for completion. */ -int blk_execute_rq(struct request_queue *q, struct gendisk *bd_disk, +void blk_execute_rq(struct request_queue *q, struct gendisk *bd_disk, struct request *rq, int at_head) { DECLARE_COMPLETION_ONSTACK(wait); - int err = 0; unsigned long hang_check; rq->end_io_data = &wait; @@ -108,10 +107,5 @@ int blk_execute_rq(struct request_queue *q, struct gendisk *bd_disk, while (!wait_for_completion_io_timeout(&wait, hang_check * (HZ/2))); else wait_for_completion_io(&wait); - - if (rq->errors) - err = -EIO; - - return err; } EXPORT_SYMBOL(blk_execute_rq); diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c index 82a43bb19967..b1352143f12f 100644 --- a/block/scsi_ioctl.c +++ b/block/scsi_ioctl.c @@ -547,7 +547,8 @@ static int __blk_send_generic(struct request_queue *q, struct gendisk *bd_disk, scsi_req(rq)->cmd[0] = cmd; scsi_req(rq)->cmd[4] = data; scsi_req(rq)->cmd_len = 6; - err = blk_execute_rq(q, bd_disk, rq, 0); + blk_execute_rq(q, bd_disk, rq, 0); + err = rq->errors ? -EIO : 0; blk_put_request(rq); return err; diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c index 2d8290169271..eaf99022bdc6 100644 --- a/drivers/block/virtio_blk.c +++ b/drivers/block/virtio_blk.c @@ -310,7 +310,8 @@ static int virtblk_get_id(struct gendisk *disk, char *id_str) if (err) goto out; - err = blk_execute_rq(vblk->disk->queue, vblk->disk, req, false); + blk_execute_rq(vblk->disk->queue, vblk->disk, req, false); + err = req->errors ? -EIO : 0; out: blk_put_request(req); return err; diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c index 87739649eac2..308501730ab3 100644 --- a/drivers/cdrom/cdrom.c +++ b/drivers/cdrom/cdrom.c @@ -2218,7 +2218,8 @@ static int cdrom_read_cdda_bpc(struct cdrom_device_info *cdi, __u8 __user *ubuf, rq->timeout = 60 * HZ; bio = rq->bio; - if (blk_execute_rq(q, cdi->disk, rq, 0)) { + blk_execute_rq(q, cdi->disk, rq, 0); + if (rq->errors) { struct request_sense *s = req->sense; ret = -EIO; cdi->last_sense = s->sense_key; diff --git a/drivers/ide/ide-atapi.c b/drivers/ide/ide-atapi.c index feb30061123b..1524797e1776 100644 --- a/drivers/ide/ide-atapi.c +++ b/drivers/ide/ide-atapi.c @@ -107,7 +107,8 @@ int ide_queue_pc_tail(ide_drive_t *drive, struct gendisk *disk, memcpy(scsi_req(rq)->cmd, pc->c, 12); if (drive->media == ide_tape) scsi_req(rq)->cmd[13] = REQ_IDETAPE_PC1; - error = blk_execute_rq(drive->queue, disk, rq, 0); + blk_execute_rq(drive->queue, disk, rq, 0); + error = rq->errors ? -EIO : 0; put_req: blk_put_request(rq); return error; diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c index 74f1b7dc03f7..95c40afa9120 100644 --- a/drivers/ide/ide-cd.c +++ b/drivers/ide/ide-cd.c @@ -452,7 +452,8 @@ int ide_cd_queue_pc(ide_drive_t *drive, const unsigned char *cmd, } } - error = blk_execute_rq(drive->queue, info->disk, rq, 0); + blk_execute_rq(drive->queue, info->disk, rq, 0); + error = rq->errors ? -EIO : 0; if (buffer) *bufflen = scsi_req(rq)->resid_len; diff --git a/drivers/ide/ide-cd_ioctl.c b/drivers/ide/ide-cd_ioctl.c index 9fcefbc8425e..f1ab726bd430 100644 --- a/drivers/ide/
Re: [dm-devel] [PATCH 02/23] block: remove the blk_execute_rq return value
On Wed, Apr 19, 2017 at 09:07:45PM +, Bart Van Assche wrote: > > + blk_execute_rq(or->request->q, NULL, or->request, 0); > > + error = or->request ? -EIO : 0; > > Hello Christoph, > > Did you perhaps intend or->request->errors instead of or->request? Yes, thanks. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH 02/23] block: remove the blk_execute_rq return value
On Wed, 2017-04-19 at 21:26 +0200, Christoph Hellwig wrote: > --- a/drivers/scsi/osd/osd_initiator.c > +++ b/drivers/scsi/osd/osd_initiator.c > @@ -489,7 +489,10 @@ static void _set_error_resid(struct osd_request *or, > struct request *req, > > int osd_execute_request(struct osd_request *or) > { > - int error = blk_execute_rq(or->request->q, NULL, or->request, 0); > + int error; > + > + blk_execute_rq(or->request->q, NULL, or->request, 0); > + error = or->request ? -EIO : 0; Hello Christoph, Did you perhaps intend or->request->errors instead of or->request? Otherwise this patch looks fine to me. Bart. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH 02/23] block: remove the blk_execute_rq return value
The function only returns -EIO if rq->errors is non-zero, which is not very useful and lets a large number of callers ignore the return value. Just let the callers figure out their error themselves. Signed-off-by: Christoph Hellwig Reviewed-by: Johannes Thumshirn --- block/blk-exec.c | 8 +--- block/scsi_ioctl.c | 3 ++- drivers/block/virtio_blk.c | 3 ++- drivers/cdrom/cdrom.c| 3 ++- drivers/ide/ide-atapi.c | 3 ++- drivers/ide/ide-cd.c | 3 ++- drivers/ide/ide-cd_ioctl.c | 3 ++- drivers/ide/ide-devsets.c| 4 ++-- drivers/ide/ide-disk.c | 3 +-- drivers/ide/ide-ioctls.c | 7 --- drivers/ide/ide-park.c | 3 ++- drivers/ide/ide-pm.c | 3 ++- drivers/ide/ide-taskfile.c | 4 ++-- drivers/scsi/osd/osd_initiator.c | 5 - fs/nfsd/blocklayout.c| 5 +++-- include/linux/blkdev.h | 2 +- 16 files changed, 34 insertions(+), 28 deletions(-) diff --git a/block/blk-exec.c b/block/blk-exec.c index 8cd0e9bc8dc8..afa383248c7c 100644 --- a/block/blk-exec.c +++ b/block/blk-exec.c @@ -92,11 +92,10 @@ EXPORT_SYMBOL_GPL(blk_execute_rq_nowait); *Insert a fully prepared request at the back of the I/O scheduler queue *for execution and wait for completion. */ -int blk_execute_rq(struct request_queue *q, struct gendisk *bd_disk, +void blk_execute_rq(struct request_queue *q, struct gendisk *bd_disk, struct request *rq, int at_head) { DECLARE_COMPLETION_ONSTACK(wait); - int err = 0; unsigned long hang_check; rq->end_io_data = &wait; @@ -108,10 +107,5 @@ int blk_execute_rq(struct request_queue *q, struct gendisk *bd_disk, while (!wait_for_completion_io_timeout(&wait, hang_check * (HZ/2))); else wait_for_completion_io(&wait); - - if (rq->errors) - err = -EIO; - - return err; } EXPORT_SYMBOL(blk_execute_rq); diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c index 82a43bb19967..b1352143f12f 100644 --- a/block/scsi_ioctl.c +++ b/block/scsi_ioctl.c @@ -547,7 +547,8 @@ static int __blk_send_generic(struct request_queue *q, struct gendisk *bd_disk, scsi_req(rq)->cmd[0] = cmd; scsi_req(rq)->cmd[4] = data; scsi_req(rq)->cmd_len = 6; - err = blk_execute_rq(q, bd_disk, rq, 0); + blk_execute_rq(q, bd_disk, rq, 0); + err = rq->errors ? -EIO : 0; blk_put_request(rq); return err; diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c index 2d8290169271..eaf99022bdc6 100644 --- a/drivers/block/virtio_blk.c +++ b/drivers/block/virtio_blk.c @@ -310,7 +310,8 @@ static int virtblk_get_id(struct gendisk *disk, char *id_str) if (err) goto out; - err = blk_execute_rq(vblk->disk->queue, vblk->disk, req, false); + blk_execute_rq(vblk->disk->queue, vblk->disk, req, false); + err = req->errors ? -EIO : 0; out: blk_put_request(req); return err; diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c index 87739649eac2..308501730ab3 100644 --- a/drivers/cdrom/cdrom.c +++ b/drivers/cdrom/cdrom.c @@ -2218,7 +2218,8 @@ static int cdrom_read_cdda_bpc(struct cdrom_device_info *cdi, __u8 __user *ubuf, rq->timeout = 60 * HZ; bio = rq->bio; - if (blk_execute_rq(q, cdi->disk, rq, 0)) { + blk_execute_rq(q, cdi->disk, rq, 0); + if (rq->errors) { struct request_sense *s = req->sense; ret = -EIO; cdi->last_sense = s->sense_key; diff --git a/drivers/ide/ide-atapi.c b/drivers/ide/ide-atapi.c index feb30061123b..1524797e1776 100644 --- a/drivers/ide/ide-atapi.c +++ b/drivers/ide/ide-atapi.c @@ -107,7 +107,8 @@ int ide_queue_pc_tail(ide_drive_t *drive, struct gendisk *disk, memcpy(scsi_req(rq)->cmd, pc->c, 12); if (drive->media == ide_tape) scsi_req(rq)->cmd[13] = REQ_IDETAPE_PC1; - error = blk_execute_rq(drive->queue, disk, rq, 0); + blk_execute_rq(drive->queue, disk, rq, 0); + error = rq->errors ? -EIO : 0; put_req: blk_put_request(rq); return error; diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c index 74f1b7dc03f7..95c40afa9120 100644 --- a/drivers/ide/ide-cd.c +++ b/drivers/ide/ide-cd.c @@ -452,7 +452,8 @@ int ide_cd_queue_pc(ide_drive_t *drive, const unsigned char *cmd, } } - error = blk_execute_rq(drive->queue, info->disk, rq, 0); + blk_execute_rq(drive->queue, info->disk, rq, 0); + error = rq->errors ? -EIO : 0; if (buffer) *bufflen = scsi_req(rq)->resid_len; diff --git a/drivers/ide/ide-cd_ioctl.c b/drivers/ide/ide-cd_ioctl.c index 9fcefbc8425e..f1ab726bd430 100644 --- a/drivers/ide/