2008-10-01 14:14:24

by Kiyoshi Ueda

[permalink] [raw]
Subject: [PATCH 0/5] block: remove end_{queued|dequeued}_request()

Hi Jens,

This patch-set removes end_queued_request() and end_dequeued_request(),
which became identical by the blk_end_request patch-set:
http://marc.info/?l=linux-kernel&m=119741306502719&w=2

They just calls __blk_end_request() actually, so users of them are
converted to call __blk_end_request() directly.
There should be no functional change.

This patch-set was created on top of the commit below of for-2.6.28.
---------------------------------------------------------------------
commit e857b07acc3f1352224f380fcbf26ef985116cc8
Author: Sven Schuetz <[email protected]>
Date: Fri Sep 26 10:58:02 2008 +0200

include blktrace_api.h in headers_install
---------------------------------------------------------------------

end_request() could be also removed, but it has been used by
many old drivers and auditing all such codes will take a time.
So this patch-set doesn't remove end_request().

Please review and apply.

Thanks,
Kiyoshi Ueda


2008-10-01 14:14:01

by Kiyoshi Ueda

[permalink] [raw]
Subject: [PATCH 1/5] virtio_blk: change to use __blk_end_request()

This patch converts virtio_blk to use __blk_end_request() directly
so that end_{queued|dequeued}_request() can be removed.
Related 'uptodate' argument is converted to 'error'.


Signed-off-by: Kiyoshi Ueda <[email protected]>
Signed-off-by: Jun'ichi Nomura <[email protected]>
Cc: Rusty Russell <[email protected]>
Cc: Jens Axboe <[email protected]>
---
drivers/block/virtio_blk.c | 10 +++++-----
1 files changed, 5 insertions(+), 5 deletions(-)

Index: linux-2.6-block/drivers/block/virtio_blk.c
===================================================================
--- linux-2.6-block.orig/drivers/block/virtio_blk.c
+++ linux-2.6-block/drivers/block/virtio_blk.c
@@ -47,20 +47,20 @@ static void blk_done(struct virtqueue *v

spin_lock_irqsave(&vblk->lock, flags);
while ((vbr = vblk->vq->vq_ops->get_buf(vblk->vq, &len)) != NULL) {
- int uptodate;
+ int error;
switch (vbr->status) {
case VIRTIO_BLK_S_OK:
- uptodate = 1;
+ error = 0;
break;
case VIRTIO_BLK_S_UNSUPP:
- uptodate = -ENOTTY;
+ error = -ENOTTY;
break;
default:
- uptodate = 0;
+ error = -EIO;
break;
}

- end_dequeued_request(vbr->req, uptodate);
+ __blk_end_request(vbr->req, error, blk_rq_bytes(vbr->req));
list_del(&vbr->list);
mempool_free(vbr, vblk->pool);
}

2008-10-01 14:14:55

by Kiyoshi Ueda

[permalink] [raw]
Subject: [PATCH 2/5] memstick: change to use __blk_end_request()

This patch converts memstick to use __blk_end_request() directly
so that end_{queued|dequeued}_request() can be removed.


Signed-off-by: Kiyoshi Ueda <[email protected]>
Signed-off-by: Jun'ichi Nomura <[email protected]>
Cc: Alex Dubov <[email protected]>
Cc: Jens Axboe <[email protected]>
---
drivers/memstick/core/mspro_block.c | 2 +-
1 files changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6-block/drivers/memstick/core/mspro_block.c
===================================================================
--- linux-2.6-block.orig/drivers/memstick/core/mspro_block.c
+++ linux-2.6-block/drivers/memstick/core/mspro_block.c
@@ -828,7 +828,7 @@ static void mspro_block_submit_req(struc

if (msb->eject) {
while ((req = elv_next_request(q)) != NULL)
- end_queued_request(req, -ENODEV);
+ __blk_end_request(req, -ENODEV, blk_rq_bytes(req));

return;
}

2008-10-01 14:16:08

by Kiyoshi Ueda

[permalink] [raw]
Subject: [PATCH 3/5] gdrom: change to use __blk_end_request()

This patch converts gdrom to use __blk_end_request() directly
so that end_{queued|dequeued}_request() can be removed.

gd.transfer is '1' in error cases and '0' in non-error cases,
so gdrom hasn't been propagating any error code to the block layer.
We can just convert error cases to '-EIO'.


Signed-off-by: Kiyoshi Ueda <[email protected]>
Signed-off-by: Jun'ichi Nomura <[email protected]>
Cc: Adrian McMenamin <[email protected]>
Cc: Jens Axboe <[email protected]>
---
drivers/cdrom/gdrom.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

Index: linux-2.6-block/drivers/cdrom/gdrom.c
===================================================================
--- linux-2.6-block.orig/drivers/cdrom/gdrom.c
+++ linux-2.6-block/drivers/cdrom/gdrom.c
@@ -624,14 +624,14 @@ static void gdrom_readdisk_dma(struct wo
ctrl_outb(1, GDROM_DMA_STATUS_REG);
wait_event_interruptible_timeout(request_queue,
gd.transfer == 0, GDROM_DEFAULT_TIMEOUT);
- err = gd.transfer;
+ err = gd.transfer ? -EIO : 0;
gd.transfer = 0;
gd.pending = 0;
/* now seek to take the request spinlock
* before handling ending the request */
spin_lock(&gdrom_lock);
list_del_init(&req->queuelist);
- end_dequeued_request(req, 1 - err);
+ __blk_end_request(req, err, blk_rq_bytes(req));
}
spin_unlock(&gdrom_lock);
kfree(read_command);

2008-10-01 14:16:25

by Kiyoshi Ueda

[permalink] [raw]
Subject: [PATCH 4/5] block: change elevator to use __blk_end_request()

This patch converts elevator to use __blk_end_request() directly
so that end_{queued|dequeued}_request() can be removed.
Related 'uptodate' arguments is converted to 'error'.


Signed-off-by: Kiyoshi Ueda <[email protected]>
Signed-off-by: Jun'ichi Nomura <[email protected]>
Cc: Jens Axboe <[email protected]>
---
block/elevator.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)

Index: linux-2.6-block/block/elevator.c
===================================================================
--- linux-2.6-block.orig/block/elevator.c
+++ linux-2.6-block/block/elevator.c
@@ -754,7 +754,7 @@ struct request *elv_next_request(struct
* not ever see it.
*/
if (blk_empty_barrier(rq)) {
- end_queued_request(rq, 1);
+ __blk_end_request(rq, 0, blk_rq_bytes(rq));
continue;
}
if (!(rq->cmd_flags & REQ_STARTED)) {
@@ -825,7 +825,7 @@ struct request *elv_next_request(struct
break;
} else if (ret == BLKPREP_KILL) {
rq->cmd_flags |= REQ_QUIET;
- end_queued_request(rq, 0);
+ __blk_end_request(rq, -EIO, blk_rq_bytes(rq));
} else {
printk(KERN_ERR "%s: bad return=%d\n", __func__, ret);
break;
@@ -922,7 +922,7 @@ void elv_abort_queue(struct request_queu
rq = list_entry_rq(q->queue_head.next);
rq->cmd_flags |= REQ_QUIET;
blk_add_trace_rq(q, rq, BLK_TA_ABORT);
- end_queued_request(rq, 0);
+ __blk_end_request(rq, -EIO, blk_rq_bytes(rq));
}
}
EXPORT_SYMBOL(elv_abort_queue);

2008-10-01 14:17:55

by Kiyoshi Ueda

[permalink] [raw]
Subject: [PATCH 5/5] block: remove end_{queued|dequeued}_request()

This patch removes end_queued_request() and end_dequeued_request(),
which are no longer used.

As a results, users of __end_request() became only end_request().
So the actual code in __end_request() is moved to end_request()
and __end_request() is removed.


Signed-off-by: Kiyoshi Ueda <[email protected]>
Signed-off-by: Jun'ichi Nomura <[email protected]>
Cc: Jens Axboe <[email protected]>
---
block/blk-core.c | 58 +++++--------------------------------------------
include/linux/blkdev.h | 2 -
2 files changed, 7 insertions(+), 53 deletions(-)

Index: linux-2.6-block/block/blk-core.c
===================================================================
--- linux-2.6-block.orig/block/blk-core.c
+++ linux-2.6-block/block/blk-core.c
@@ -1788,17 +1788,6 @@ static void end_that_request_last(struct
}
}

-static inline void __end_request(struct request *rq, int uptodate,
- unsigned int nr_bytes)
-{
- int error = 0;
-
- if (uptodate <= 0)
- error = uptodate ? uptodate : -EIO;
-
- __blk_end_request(rq, error, nr_bytes);
-}
-
/**
* blk_rq_bytes - Returns bytes left to complete in the entire request
* @rq: the request being processed
@@ -1829,41 +1818,6 @@ unsigned int blk_rq_cur_bytes(struct req
EXPORT_SYMBOL_GPL(blk_rq_cur_bytes);

/**
- * end_queued_request - end all I/O on a queued request
- * @rq: the request being processed
- * @uptodate: error value or %0/%1 uptodate flag
- *
- * Description:
- * Ends all I/O on a request, and removes it from the block layer queues.
- * Not suitable for normal I/O completion, unless the driver still has
- * the request attached to the block layer.
- *
- **/
-void end_queued_request(struct request *rq, int uptodate)
-{
- __end_request(rq, uptodate, blk_rq_bytes(rq));
-}
-EXPORT_SYMBOL(end_queued_request);
-
-/**
- * end_dequeued_request - end all I/O on a dequeued request
- * @rq: the request being processed
- * @uptodate: error value or %0/%1 uptodate flag
- *
- * Description:
- * Ends all I/O on a request. The request must already have been
- * dequeued using blkdev_dequeue_request(), as is normally the case
- * for most drivers.
- *
- **/
-void end_dequeued_request(struct request *rq, int uptodate)
-{
- __end_request(rq, uptodate, blk_rq_bytes(rq));
-}
-EXPORT_SYMBOL(end_dequeued_request);
-
-
-/**
* end_request - end I/O on the current segment of the request
* @req: the request being processed
* @uptodate: error value or %0/%1 uptodate flag
@@ -1877,14 +1831,16 @@ EXPORT_SYMBOL(end_dequeued_request);
* they have a residual value to account for. For that case this function
* isn't really useful, unless the residual just happens to be the
* full current segment. In other words, don't use this function in new
- * code. Use blk_end_request() or __blk_end_request() to end partial parts
- * of a request, or end_dequeued_request() and end_queued_request() to
- * completely end IO on a dequeued/queued request.
- *
+ * code. Use blk_end_request() or __blk_end_request() to end a request.
**/
void end_request(struct request *req, int uptodate)
{
- __end_request(req, uptodate, req->hard_cur_sectors << 9);
+ int error = 0;
+
+ if (uptodate <= 0)
+ error = uptodate ? uptodate : -EIO;
+
+ __blk_end_request(req, error, req->hard_cur_sectors << 9);
}
EXPORT_SYMBOL(end_request);

Index: linux-2.6-block/include/linux/blkdev.h
===================================================================
--- linux-2.6-block.orig/include/linux/blkdev.h
+++ linux-2.6-block/include/linux/blkdev.h
@@ -790,8 +790,6 @@ extern int __blk_end_request(struct requ
extern int blk_end_bidi_request(struct request *rq, int error,
unsigned int nr_bytes, unsigned int bidi_bytes);
extern void end_request(struct request *, int);
-extern void end_queued_request(struct request *, int);
-extern void end_dequeued_request(struct request *, int);
extern int blk_end_request_callback(struct request *rq, int error,
unsigned int nr_bytes,
int (drv_callback)(struct request *));

2008-10-01 14:21:03

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH 0/5] block: remove end_{queued|dequeued}_request()

On Wed, Oct 01 2008, Kiyoshi Ueda wrote:
> Hi Jens,
>
> This patch-set removes end_queued_request() and end_dequeued_request(),
> which became identical by the blk_end_request patch-set:
> http://marc.info/?l=linux-kernel&m=119741306502719&w=2
>
> They just calls __blk_end_request() actually, so users of them are
> converted to call __blk_end_request() directly.
> There should be no functional change.
>
> This patch-set was created on top of the commit below of for-2.6.28.
> ---------------------------------------------------------------------
> commit e857b07acc3f1352224f380fcbf26ef985116cc8
> Author: Sven Schuetz <[email protected]>
> Date: Fri Sep 26 10:58:02 2008 +0200
>
> include blktrace_api.h in headers_install
> ---------------------------------------------------------------------
>
> end_request() could be also removed, but it has been used by
> many old drivers and auditing all such codes will take a time.
> So this patch-set doesn't remove end_request().
>
> Please review and apply.

Looks good, applied to the mix! Thanks.

--
Jens Axboe

2008-10-03 01:31:21

by Rusty Russell

[permalink] [raw]
Subject: Re: [PATCH 1/5] virtio_blk: change to use __blk_end_request()

On Thursday 02 October 2008 00:11:20 Kiyoshi Ueda wrote:
> This patch converts virtio_blk to use __blk_end_request() directly
> so that end_{queued|dequeued}_request() can be removed.
> Related 'uptodate' argument is converted to 'error'.

Hi Kiyoshi,

Nice and clear; now matches common kernel style.

Acked-by: Rusty Russell <[email protected]>

Thanks!
Rusty.