Hello,
Upon ack, please pull from the following git tree, which pulls in
linux-2.6-ide#for-next into linux-2.6-block#for-next and applies this
patchset on top of it.
git://git.kernel.org/pub/scm/linux/kernel/git/tj/misc.git block-cleanup
This is the third take of block-cleanup-patches. Changes from the
last take[L] are
* All ide changes including L0001-0003 are separated out and merged
through linux-2.6-ide#for-next and pulled into
linux-2.6-block#for-next.
* L0015-block-kill-end_request.patch replaced with
0012-block-replace-end_request-with-__-blk_end_reques.patch.
* L0016-ubd-simplify-block-request-completion.patch
L0017-block-clean-up-unnecessary-stuff-from-block-drivers.patch are
postponed to later patchset.
* 0013-0014 to kill rq->data added.
* Updated to the current #for-next.
0001-block-merge-blk_invoke_request_fn-into-__blk_run_.patch
0002-block-kill-blk_start_queueing.patch
0003-block-don-t-set-REQ_NOMERGE-unnecessarily.patch
0004-block-cleanup-REQ_SOFTBARRIER-usages.patch
0005-block-clean-up-misc-stuff-after-block-layer-timeout.patch
0006-block-reorder-request-completion-functions.patch
0007-block-reorganize-request-fetching-functions.patch
0008-block-kill-blk_end_request_callback.patch
0009-block-clean-up-request-completion-API.patch
0010-block-move-rq-start_time-initialization-to-blk_rq_.patch
0011-block-implement-and-use-__-blk_end_request_all.patch
0012-block-replace-end_request-with-__-blk_end_reques.patch
0013-block-don-t-abuse-rq-data.patch
0014-block-kill-data.patch
0001-0011 are mostly identical to L0004-0014. L0015 incorrectly
replaced end_request() with __blk_end_request_all(). 0012 adds
[__]blk_end_request_cur() and replaces end_request() with it.
With ide changes pulled in, omap is the only user of rq->data, 0013
makes it use only rq->special and 0014 kills rq->data.
This patchset is on top of
linux-2.6-block#for-next(a95320f3ede3f028837a621715cd352e83ffe7c2)
+ linux-2.6-ide#for-next(2ecf0a57c60dcb588f310d94412118e15c510532)
and contains the following changes.
arch/arm/plat-omap/mailbox.c | 54 ++--
block/as-iosched.c | 6
block/blk-barrier.c | 9
block/blk-core.c | 486 ++++++++++++++----------------------
block/blk-exec.c | 1
block/blk-map.c | 6
block/blk-timeout.c | 22 -
block/blk.h | 37 ++
block/cfq-iosched.c | 6
block/elevator.c | 137 ----------
block/scsi_ioctl.c | 1
drivers/block/amiflop.c | 10
drivers/block/ataflop.c | 14 -
drivers/block/cpqarray.c | 3
drivers/block/hd.c | 14 -
drivers/block/mg_disk.c | 16 -
drivers/block/paride/pcd.c | 12
drivers/block/paride/pd.c | 5
drivers/block/paride/pf.c | 28 +-
drivers/block/ps3disk.c | 6
drivers/block/swim.c | 14 -
drivers/block/swim3.c | 26 -
drivers/block/sx8.c | 3
drivers/block/virtio_blk.c | 2
drivers/block/xd.c | 15 -
drivers/block/xen-blkfront.c | 6
drivers/block/xsysace.c | 4
drivers/block/z2ram.c | 4
drivers/cdrom/gdrom.c | 8
drivers/cdrom/viocd.c | 25 -
drivers/memstick/core/mspro_block.c | 2
drivers/message/i2o/i2o_block.c | 2
drivers/mtd/mtd_blkdevs.c | 22 -
drivers/s390/block/dasd.c | 17 -
drivers/s390/char/tape_block.c | 15 -
drivers/sbus/char/jsflash.c | 8
drivers/scsi/scsi_lib.c | 3
include/linux/blkdev.h | 173 ++++++++++--
38 files changed, 559 insertions(+), 663 deletions(-)
Thanks.
--
tejun
[L] http://thread.gmane.org/gmane.linux.kernel/807554
__blk_run_queue wraps blk_invoke_request_fn() such that it
additionally removes plug and bails out early if the queue is empty.
Both extra operations have their own pending mechanisms and don't
cause any harm correctness-wise when they are done superflously.
The only user of blk_invoke_request_fn() being blk_start_queue(),
there isn't much reason to keep both functions around. Merge
blk_invoke_request_fn() into __blk_run_queue() and make
blk_start_queue() use __blk_run_queue() instead.
[ Impact: merge two subtly different internal functions ]
Signed-off-by: Tejun Heo <[email protected]>
---
block/blk-core.c | 35 ++++++++++++++---------------------
1 files changed, 14 insertions(+), 21 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index cce7a88..3de653d 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -333,24 +333,6 @@ void blk_unplug(struct request_queue *q)
}
EXPORT_SYMBOL(blk_unplug);
-static void blk_invoke_request_fn(struct request_queue *q)
-{
- if (unlikely(blk_queue_stopped(q)))
- return;
-
- /*
- * one level of recursion is ok and is much faster than kicking
- * the unplug handling
- */
- if (!queue_flag_test_and_set(QUEUE_FLAG_REENTER, q)) {
- q->request_fn(q);
- queue_flag_clear(QUEUE_FLAG_REENTER, q);
- } else {
- queue_flag_set(QUEUE_FLAG_PLUGGED, q);
- kblockd_schedule_work(q, &q->unplug_work);
- }
-}
-
/**
* blk_start_queue - restart a previously stopped queue
* @q: The &struct request_queue in question
@@ -365,7 +347,7 @@ void blk_start_queue(struct request_queue *q)
WARN_ON(!irqs_disabled());
queue_flag_clear(QUEUE_FLAG_STOPPED, q);
- blk_invoke_request_fn(q);
+ __blk_run_queue(q);
}
EXPORT_SYMBOL(blk_start_queue);
@@ -425,12 +407,23 @@ void __blk_run_queue(struct request_queue *q)
{
blk_remove_plug(q);
+ if (unlikely(blk_queue_stopped(q)))
+ return;
+
+ if (elv_queue_empty(q))
+ return;
+
/*
* Only recurse once to avoid overrunning the stack, let the unplug
* handling reinvoke the handler shortly if we already got there.
*/
- if (!elv_queue_empty(q))
- blk_invoke_request_fn(q);
+ if (!queue_flag_test_and_set(QUEUE_FLAG_REENTER, q)) {
+ q->request_fn(q);
+ queue_flag_clear(QUEUE_FLAG_REENTER, q);
+ } else {
+ queue_flag_set(QUEUE_FLAG_PLUGGED, q);
+ kblockd_schedule_work(q, &q->unplug_work);
+ }
}
EXPORT_SYMBOL(__blk_run_queue);
--
1.6.0.2
RQ_NOMERGE_FLAGS already clears defines which REQ flags aren't
mergeable. There is no reason to specify it superflously. It only
adds to confusion. Don't set REQ_NOMERGE for barriers and requests
with specific queueing directive. REQ_NOMERGE is now exclusively used
by the merging code.
[ Impact: cleanup ]
Signed-off-by: Tejun Heo <[email protected]>
---
block/blk-core.c | 5 +----
block/blk-exec.c | 1 -
2 files changed, 1 insertions(+), 5 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index d4c7260..bcc85ec 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1080,16 +1080,13 @@ void init_request_from_bio(struct request *req, struct bio *bio)
if (bio_failfast_driver(bio))
req->cmd_flags |= REQ_FAILFAST_DRIVER;
- /*
- * REQ_BARRIER implies no merging, but lets make it explicit
- */
if (unlikely(bio_discard(bio))) {
req->cmd_flags |= REQ_DISCARD;
if (bio_barrier(bio))
req->cmd_flags |= REQ_SOFTBARRIER;
req->q->prepare_discard_fn(req->q, req);
} else if (unlikely(bio_barrier(bio)))
- req->cmd_flags |= (REQ_HARDBARRIER | REQ_NOMERGE);
+ req->cmd_flags |= REQ_HARDBARRIER;
if (bio_sync(bio))
req->cmd_flags |= REQ_RW_SYNC;
diff --git a/block/blk-exec.c b/block/blk-exec.c
index 6af716d..49557e9 100644
--- a/block/blk-exec.c
+++ b/block/blk-exec.c
@@ -51,7 +51,6 @@ void blk_execute_rq_nowait(struct request_queue *q, struct gendisk *bd_disk,
int where = at_head ? ELEVATOR_INSERT_FRONT : ELEVATOR_INSERT_BACK;
rq->rq_disk = bd_disk;
- rq->cmd_flags |= REQ_NOMERGE;
rq->end_io = done;
WARN_ON(irqs_disabled());
spin_lock_irq(q->queue_lock);
--
1.6.0.2
blk_start_queueing() is identical to __blk_run_queue() except that it
doesn't check for recursion. None of the current users depends on
blk_start_queueing() running request_fn directly. Replace usages of
blk_start_queueing() with [__]blk_run_queue() and kill it.
[ Impact: removal of mostly duplicate interface function ]
Signed-off-by: Tejun Heo <[email protected]>
---
block/as-iosched.c | 6 +-----
block/blk-core.c | 28 ++--------------------------
block/cfq-iosched.c | 6 +++---
block/elevator.c | 7 +++----
include/linux/blkdev.h | 1 -
5 files changed, 9 insertions(+), 39 deletions(-)
diff --git a/block/as-iosched.c b/block/as-iosched.c
index c48fa67..45bd070 100644
--- a/block/as-iosched.c
+++ b/block/as-iosched.c
@@ -1312,12 +1312,8 @@ static void as_merged_requests(struct request_queue *q, struct request *req,
static void as_work_handler(struct work_struct *work)
{
struct as_data *ad = container_of(work, struct as_data, antic_work);
- struct request_queue *q = ad->q;
- unsigned long flags;
- spin_lock_irqsave(q->queue_lock, flags);
- blk_start_queueing(q);
- spin_unlock_irqrestore(q->queue_lock, flags);
+ blk_run_queue(ad->q);
}
static int as_may_queue(struct request_queue *q, int rw)
diff --git a/block/blk-core.c b/block/blk-core.c
index 3de653d..d4c7260 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -433,9 +433,7 @@ EXPORT_SYMBOL(__blk_run_queue);
*
* Description:
* Invoke request handling on this queue, if it has pending work to do.
- * May be used to restart queueing when a request has completed. Also
- * See @blk_start_queueing.
- *
+ * May be used to restart queueing when a request has completed.
*/
void blk_run_queue(struct request_queue *q)
{
@@ -893,28 +891,6 @@ struct request *blk_get_request(struct request_queue *q, int rw, gfp_t gfp_mask)
EXPORT_SYMBOL(blk_get_request);
/**
- * blk_start_queueing - initiate dispatch of requests to device
- * @q: request queue to kick into gear
- *
- * This is basically a helper to remove the need to know whether a queue
- * is plugged or not if someone just wants to initiate dispatch of requests
- * for this queue. Should be used to start queueing on a device outside
- * of ->request_fn() context. Also see @blk_run_queue.
- *
- * The queue lock must be held with interrupts disabled.
- */
-void blk_start_queueing(struct request_queue *q)
-{
- if (!blk_queue_plugged(q)) {
- if (unlikely(blk_queue_stopped(q)))
- return;
- q->request_fn(q);
- } else
- __generic_unplug_device(q);
-}
-EXPORT_SYMBOL(blk_start_queueing);
-
-/**
* blk_requeue_request - put a request back on queue
* @q: request queue where request should be inserted
* @rq: request to be inserted
@@ -982,7 +958,7 @@ void blk_insert_request(struct request_queue *q, struct request *rq,
drive_stat_acct(rq, 1);
__elv_add_request(q, rq, where, 0);
- blk_start_queueing(q);
+ __blk_run_queue(q);
spin_unlock_irqrestore(q->queue_lock, flags);
}
EXPORT_SYMBOL(blk_insert_request);
diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
index 0d3b70d..581f84c 100644
--- a/block/cfq-iosched.c
+++ b/block/cfq-iosched.c
@@ -2078,7 +2078,7 @@ cfq_rq_enqueued(struct cfq_data *cfqd, struct cfq_queue *cfqq,
if (blk_rq_bytes(rq) > PAGE_CACHE_SIZE ||
cfqd->busy_queues > 1) {
del_timer(&cfqd->idle_slice_timer);
- blk_start_queueing(cfqd->queue);
+ __blk_run_queue(cfqd->queue);
}
cfq_mark_cfqq_must_dispatch(cfqq);
}
@@ -2090,7 +2090,7 @@ cfq_rq_enqueued(struct cfq_data *cfqd, struct cfq_queue *cfqq,
* this new queue is RT and the current one is BE
*/
cfq_preempt_queue(cfqd, cfqq);
- blk_start_queueing(cfqd->queue);
+ __blk_run_queue(cfqd->queue);
}
}
@@ -2335,7 +2335,7 @@ static void cfq_kick_queue(struct work_struct *work)
struct request_queue *q = cfqd->queue;
spin_lock_irq(q->queue_lock);
- blk_start_queueing(q);
+ __blk_run_queue(cfqd->queue);
spin_unlock_irq(q->queue_lock);
}
diff --git a/block/elevator.c b/block/elevator.c
index 7073a90..2e0fb21 100644
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -599,7 +599,7 @@ void elv_quiesce_start(struct request_queue *q)
*/
elv_drain_elevator(q);
while (q->rq.elvpriv) {
- blk_start_queueing(q);
+ __blk_run_queue(q);
spin_unlock_irq(q->queue_lock);
msleep(10);
spin_lock_irq(q->queue_lock);
@@ -643,8 +643,7 @@ void elv_insert(struct request_queue *q, struct request *rq, int where)
* with anything. There's no point in delaying queue
* processing.
*/
- blk_remove_plug(q);
- blk_start_queueing(q);
+ __blk_run_queue(q);
break;
case ELEVATOR_INSERT_SORT:
@@ -971,7 +970,7 @@ void elv_completed_request(struct request_queue *q, struct request *rq)
blk_ordered_cur_seq(q) == QUEUE_ORDSEQ_DRAIN &&
(!next || blk_ordered_req_seq(next) > QUEUE_ORDSEQ_DRAIN)) {
blk_ordered_complete_seq(q, QUEUE_ORDSEQ_DRAIN, 0);
- blk_start_queueing(q);
+ __blk_run_queue(q);
}
}
}
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index ba54c83..6e03aef 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -794,7 +794,6 @@ extern void blk_sync_queue(struct request_queue *q);
extern void __blk_stop_queue(struct request_queue *q);
extern void __blk_run_queue(struct request_queue *);
extern void blk_run_queue(struct request_queue *);
-extern void blk_start_queueing(struct request_queue *);
extern int blk_rq_map_user(struct request_queue *, struct request *,
struct rq_map_data *, void __user *, unsigned long,
gfp_t);
--
1.6.0.2
blk_insert_request() doesn't need to worry about REQ_SOFTBARRIER.
Don't set it. Combined with recent ide updates, REQ_SOFTBARRIER is
now only used in elevator proper and for discard requests.
[ Impact: cleanup ]
Signed-off-by: Tejun Heo <[email protected]>
---
block/blk-core.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index bcc85ec..c632e6d 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -944,7 +944,6 @@ void blk_insert_request(struct request_queue *q, struct request *rq,
* barrier
*/
rq->cmd_type = REQ_TYPE_SPECIAL;
- rq->cmd_flags |= REQ_SOFTBARRIER;
rq->special = data;
--
1.6.0.2
* In blk_rq_timed_out_timer(), else { if } to else if
* In blk_add_timer(), simplify if/else block
[ Impact: cleanup ]
Signed-off-by: Tejun Heo <[email protected]>
---
block/blk-timeout.c | 22 +++++++++-------------
1 files changed, 9 insertions(+), 13 deletions(-)
diff --git a/block/blk-timeout.c b/block/blk-timeout.c
index 8f570c4..590d1eb 100644
--- a/block/blk-timeout.c
+++ b/block/blk-timeout.c
@@ -122,10 +122,8 @@ void blk_rq_timed_out_timer(unsigned long data)
if (blk_mark_rq_complete(rq))
continue;
blk_rq_timed_out(rq);
- } else {
- if (!next || time_after(next, rq->deadline))
- next = rq->deadline;
- }
+ } else if (!next || time_after(next, rq->deadline))
+ next = rq->deadline;
}
/*
@@ -176,16 +174,14 @@ void blk_add_timer(struct request *req)
BUG_ON(!list_empty(&req->timeout_list));
BUG_ON(test_bit(REQ_ATOM_COMPLETE, &req->atomic_flags));
- if (req->timeout)
- req->deadline = jiffies + req->timeout;
- else {
- req->deadline = jiffies + q->rq_timeout;
- /*
- * Some LLDs, like scsi, peek at the timeout to prevent
- * a command from being retried forever.
- */
+ /*
+ * Some LLDs, like scsi, peek at the timeout to prevent a
+ * command from being retried forever.
+ */
+ if (!req->timeout)
req->timeout = q->rq_timeout;
- }
+
+ req->deadline = jiffies + req->timeout;
list_add_tail(&req->timeout_list, &q->timeout_list);
/*
--
1.6.0.2
Reorder request completion functions such that
* All request completion functions are located together.
* Functions which are used by only one caller is put right above the
caller.
* end_request() is put after other completion functions but before
blk_update_request().
This change is for completion function cleanup which will follow.
[ Impact: cleanup, code reorganization ]
Signed-off-by: Tejun Heo <[email protected]>
---
block/blk-core.c | 144 ++++++++++++++++++++++++------------------------
include/linux/blkdev.h | 16 +++---
2 files changed, 80 insertions(+), 80 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index c632e6d..08f09c5 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1682,6 +1682,35 @@ static void blk_account_io_done(struct request *req)
}
/**
+ * blk_rq_bytes - Returns bytes left to complete in the entire request
+ * @rq: the request being processed
+ **/
+unsigned int blk_rq_bytes(struct request *rq)
+{
+ if (blk_fs_request(rq))
+ return rq->hard_nr_sectors << 9;
+
+ return rq->data_len;
+}
+EXPORT_SYMBOL_GPL(blk_rq_bytes);
+
+/**
+ * blk_rq_cur_bytes - Returns bytes left to complete in the current segment
+ * @rq: the request being processed
+ **/
+unsigned int blk_rq_cur_bytes(struct request *rq)
+{
+ if (blk_fs_request(rq))
+ return rq->current_nr_sectors << 9;
+
+ if (rq->bio)
+ return rq->bio->bi_size;
+
+ return rq->data_len;
+}
+EXPORT_SYMBOL_GPL(blk_rq_cur_bytes);
+
+/**
* __end_that_request_first - end I/O on a request
* @req: the request being processed
* @error: %0 for success, < %0 for error
@@ -1795,6 +1824,22 @@ static int __end_that_request_first(struct request *req, int error,
return 1;
}
+static int end_that_request_data(struct request *rq, int error,
+ unsigned int nr_bytes, unsigned int bidi_bytes)
+{
+ if (rq->bio) {
+ if (__end_that_request_first(rq, error, nr_bytes))
+ return 1;
+
+ /* Bidi request must be completed as a whole */
+ if (blk_bidi_rq(rq) &&
+ __end_that_request_first(rq->next_rq, error, bidi_bytes))
+ return 1;
+ }
+
+ return 0;
+}
+
/*
* queue lock must be held
*/
@@ -1824,78 +1869,6 @@ static void end_that_request_last(struct request *req, int error)
}
/**
- * blk_rq_bytes - Returns bytes left to complete in the entire request
- * @rq: the request being processed
- **/
-unsigned int blk_rq_bytes(struct request *rq)
-{
- if (blk_fs_request(rq))
- return rq->hard_nr_sectors << 9;
-
- return rq->data_len;
-}
-EXPORT_SYMBOL_GPL(blk_rq_bytes);
-
-/**
- * blk_rq_cur_bytes - Returns bytes left to complete in the current segment
- * @rq: the request being processed
- **/
-unsigned int blk_rq_cur_bytes(struct request *rq)
-{
- if (blk_fs_request(rq))
- return rq->current_nr_sectors << 9;
-
- if (rq->bio)
- return rq->bio->bi_size;
-
- return rq->data_len;
-}
-EXPORT_SYMBOL_GPL(blk_rq_cur_bytes);
-
-/**
- * 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
- *
- * Description:
- * Ends I/O on the current segment of a request. If that is the only
- * remaining segment, the request is also completed and freed.
- *
- * This is a remnant of how older block drivers handled I/O completions.
- * Modern drivers typically end I/O on the full request in one go, unless
- * 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 a request.
- **/
-void end_request(struct request *req, int uptodate)
-{
- int error = 0;
-
- if (uptodate <= 0)
- error = uptodate ? uptodate : -EIO;
-
- __blk_end_request(req, error, req->hard_cur_sectors << 9);
-}
-EXPORT_SYMBOL(end_request);
-
-static int end_that_request_data(struct request *rq, int error,
- unsigned int nr_bytes, unsigned int bidi_bytes)
-{
- if (rq->bio) {
- if (__end_that_request_first(rq, error, nr_bytes))
- return 1;
-
- /* Bidi request must be completed as a whole */
- if (blk_bidi_rq(rq) &&
- __end_that_request_first(rq->next_rq, error, bidi_bytes))
- return 1;
- }
-
- return 0;
-}
-
-/**
* blk_end_io - Generic end_io function to complete a request.
* @rq: the request being processed
* @error: %0 for success, < %0 for error
@@ -2005,6 +1978,33 @@ int blk_end_bidi_request(struct request *rq, int error, unsigned int nr_bytes,
EXPORT_SYMBOL_GPL(blk_end_bidi_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
+ *
+ * Description:
+ * Ends I/O on the current segment of a request. If that is the only
+ * remaining segment, the request is also completed and freed.
+ *
+ * This is a remnant of how older block drivers handled I/O completions.
+ * Modern drivers typically end I/O on the full request in one go, unless
+ * 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 a request.
+ **/
+void end_request(struct request *req, int uptodate)
+{
+ int error = 0;
+
+ if (uptodate <= 0)
+ error = uptodate ? uptodate : -EIO;
+
+ __blk_end_request(req, error, req->hard_cur_sectors << 9);
+}
+EXPORT_SYMBOL(end_request);
+
+/**
* blk_update_request - Special helper function for request stacking drivers
* @rq: the request being processed
* @error: %0 for success, < %0 for error
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 6e03aef..8d12dba 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -829,6 +829,14 @@ static inline void blk_run_address_space(struct address_space *mapping)
extern void blkdev_dequeue_request(struct request *req);
/*
+ * blk_end_request() takes bytes instead of sectors as a complete size.
+ * blk_rq_bytes() returns bytes left to complete in the entire request.
+ * blk_rq_cur_bytes() returns bytes left to complete in the current segment.
+ */
+extern unsigned int blk_rq_bytes(struct request *rq);
+extern unsigned int blk_rq_cur_bytes(struct request *rq);
+
+/*
* blk_end_request() and friends.
* __blk_end_request() and end_request() must be called with
* the request queue spinlock acquired.
@@ -855,14 +863,6 @@ extern void blk_update_request(struct request *rq, int error,
unsigned int nr_bytes);
/*
- * blk_end_request() takes bytes instead of sectors as a complete size.
- * blk_rq_bytes() returns bytes left to complete in the entire request.
- * blk_rq_cur_bytes() returns bytes left to complete in the current segment.
- */
-extern unsigned int blk_rq_bytes(struct request *rq);
-extern unsigned int blk_rq_cur_bytes(struct request *rq);
-
-/*
* Access functions for manipulating queue properties
*/
extern struct request_queue *blk_init_queue_node(request_fn_proc *rfn,
--
1.6.0.2
Impact: code reorganization
elv_next_request() and elv_dequeue_request() are public block layer
interface than actual elevator implementation. They mostly deal with
how requests interact with block layer and low level drivers at the
beginning of rqeuest processing whereas __elv_next_request() is the
actual eleveator request fetching interface.
Move the two functions to blk-core.c. This prepares for further
interface cleanup.
Signed-off-by: Tejun Heo <[email protected]>
---
block/blk-core.c | 95 ++++++++++++++++++++++++++++++++++++++++
block/blk.h | 37 ++++++++++++++++
block/elevator.c | 128 ------------------------------------------------------
3 files changed, 132 insertions(+), 128 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index 08f09c5..148492e 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1710,6 +1710,101 @@ unsigned int blk_rq_cur_bytes(struct request *rq)
}
EXPORT_SYMBOL_GPL(blk_rq_cur_bytes);
+struct request *elv_next_request(struct request_queue *q)
+{
+ struct request *rq;
+ int ret;
+
+ while ((rq = __elv_next_request(q)) != NULL) {
+ if (!(rq->cmd_flags & REQ_STARTED)) {
+ /*
+ * This is the first time the device driver
+ * sees this request (possibly after
+ * requeueing). Notify IO scheduler.
+ */
+ if (blk_sorted_rq(rq))
+ elv_activate_rq(q, rq);
+
+ /*
+ * just mark as started even if we don't start
+ * it, a request that has been delayed should
+ * not be passed by new incoming requests
+ */
+ rq->cmd_flags |= REQ_STARTED;
+ trace_block_rq_issue(q, rq);
+ }
+
+ if (!q->boundary_rq || q->boundary_rq == rq) {
+ q->end_sector = rq_end_sector(rq);
+ q->boundary_rq = NULL;
+ }
+
+ if (rq->cmd_flags & REQ_DONTPREP)
+ break;
+
+ if (q->dma_drain_size && rq->data_len) {
+ /*
+ * make sure space for the drain appears we
+ * know we can do this because max_hw_segments
+ * has been adjusted to be one fewer than the
+ * device can handle
+ */
+ rq->nr_phys_segments++;
+ }
+
+ if (!q->prep_rq_fn)
+ break;
+
+ ret = q->prep_rq_fn(q, rq);
+ if (ret == BLKPREP_OK) {
+ break;
+ } else if (ret == BLKPREP_DEFER) {
+ /*
+ * the request may have been (partially) prepped.
+ * we need to keep this request in the front to
+ * avoid resource deadlock. REQ_STARTED will
+ * prevent other fs requests from passing this one.
+ */
+ if (q->dma_drain_size && rq->data_len &&
+ !(rq->cmd_flags & REQ_DONTPREP)) {
+ /*
+ * remove the space for the drain we added
+ * so that we don't add it again
+ */
+ --rq->nr_phys_segments;
+ }
+
+ rq = NULL;
+ break;
+ } else if (ret == BLKPREP_KILL) {
+ rq->cmd_flags |= REQ_QUIET;
+ __blk_end_request(rq, -EIO, blk_rq_bytes(rq));
+ } else {
+ printk(KERN_ERR "%s: bad return=%d\n", __func__, ret);
+ break;
+ }
+ }
+
+ return rq;
+}
+EXPORT_SYMBOL(elv_next_request);
+
+void elv_dequeue_request(struct request_queue *q, struct request *rq)
+{
+ BUG_ON(list_empty(&rq->queuelist));
+ BUG_ON(ELV_ON_HASH(rq));
+
+ list_del_init(&rq->queuelist);
+
+ /*
+ * the time frame between a request being removed from the lists
+ * and to it is freed is accounted as io that is in progress at
+ * the driver side.
+ */
+ if (blk_account_rq(rq))
+ q->in_flight++;
+}
+
/**
* __end_that_request_first - end I/O on a request
* @req: the request being processed
diff --git a/block/blk.h b/block/blk.h
index 5dfc412..69a1087 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -43,6 +43,43 @@ static inline void blk_clear_rq_complete(struct request *rq)
clear_bit(REQ_ATOM_COMPLETE, &rq->atomic_flags);
}
+/*
+ * Internal elevator interface
+ */
+#define ELV_ON_HASH(rq) (!hlist_unhashed(&(rq)->hash))
+
+static inline struct request *__elv_next_request(struct request_queue *q)
+{
+ struct request *rq;
+
+ while (1) {
+ while (!list_empty(&q->queue_head)) {
+ rq = list_entry_rq(q->queue_head.next);
+ if (blk_do_ordered(q, &rq))
+ return rq;
+ }
+
+ if (!q->elevator->ops->elevator_dispatch_fn(q, 0))
+ return NULL;
+ }
+}
+
+static inline void elv_activate_rq(struct request_queue *q, struct request *rq)
+{
+ struct elevator_queue *e = q->elevator;
+
+ if (e->ops->elevator_activate_req_fn)
+ e->ops->elevator_activate_req_fn(q, rq);
+}
+
+static inline void elv_deactivate_rq(struct request_queue *q, struct request *rq)
+{
+ struct elevator_queue *e = q->elevator;
+
+ if (e->ops->elevator_deactivate_req_fn)
+ e->ops->elevator_deactivate_req_fn(q, rq);
+}
+
#ifdef CONFIG_FAIL_IO_TIMEOUT
int blk_should_fake_timeout(struct request_queue *);
ssize_t part_timeout_show(struct device *, struct device_attribute *, char *);
diff --git a/block/elevator.c b/block/elevator.c
index 2e0fb21..b03b875 100644
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -53,7 +53,6 @@ static const int elv_hash_shift = 6;
(hash_long(ELV_HASH_BLOCK((sec)), elv_hash_shift))
#define ELV_HASH_ENTRIES (1 << elv_hash_shift)
#define rq_hash_key(rq) ((rq)->sector + (rq)->nr_sectors)
-#define ELV_ON_HASH(rq) (!hlist_unhashed(&(rq)->hash))
DEFINE_TRACE(block_rq_insert);
DEFINE_TRACE(block_rq_issue);
@@ -310,22 +309,6 @@ void elevator_exit(struct elevator_queue *e)
}
EXPORT_SYMBOL(elevator_exit);
-static void elv_activate_rq(struct request_queue *q, struct request *rq)
-{
- struct elevator_queue *e = q->elevator;
-
- if (e->ops->elevator_activate_req_fn)
- e->ops->elevator_activate_req_fn(q, rq);
-}
-
-static void elv_deactivate_rq(struct request_queue *q, struct request *rq)
-{
- struct elevator_queue *e = q->elevator;
-
- if (e->ops->elevator_deactivate_req_fn)
- e->ops->elevator_deactivate_req_fn(q, rq);
-}
-
static inline void __elv_rqhash_del(struct request *rq)
{
hlist_del_init(&rq->hash);
@@ -758,117 +741,6 @@ void elv_add_request(struct request_queue *q, struct request *rq, int where,
}
EXPORT_SYMBOL(elv_add_request);
-static inline struct request *__elv_next_request(struct request_queue *q)
-{
- struct request *rq;
-
- while (1) {
- while (!list_empty(&q->queue_head)) {
- rq = list_entry_rq(q->queue_head.next);
- if (blk_do_ordered(q, &rq))
- return rq;
- }
-
- if (!q->elevator->ops->elevator_dispatch_fn(q, 0))
- return NULL;
- }
-}
-
-struct request *elv_next_request(struct request_queue *q)
-{
- struct request *rq;
- int ret;
-
- while ((rq = __elv_next_request(q)) != NULL) {
- if (!(rq->cmd_flags & REQ_STARTED)) {
- /*
- * This is the first time the device driver
- * sees this request (possibly after
- * requeueing). Notify IO scheduler.
- */
- if (blk_sorted_rq(rq))
- elv_activate_rq(q, rq);
-
- /*
- * just mark as started even if we don't start
- * it, a request that has been delayed should
- * not be passed by new incoming requests
- */
- rq->cmd_flags |= REQ_STARTED;
- trace_block_rq_issue(q, rq);
- }
-
- if (!q->boundary_rq || q->boundary_rq == rq) {
- q->end_sector = rq_end_sector(rq);
- q->boundary_rq = NULL;
- }
-
- if (rq->cmd_flags & REQ_DONTPREP)
- break;
-
- if (q->dma_drain_size && rq->data_len) {
- /*
- * make sure space for the drain appears we
- * know we can do this because max_hw_segments
- * has been adjusted to be one fewer than the
- * device can handle
- */
- rq->nr_phys_segments++;
- }
-
- if (!q->prep_rq_fn)
- break;
-
- ret = q->prep_rq_fn(q, rq);
- if (ret == BLKPREP_OK) {
- break;
- } else if (ret == BLKPREP_DEFER) {
- /*
- * the request may have been (partially) prepped.
- * we need to keep this request in the front to
- * avoid resource deadlock. REQ_STARTED will
- * prevent other fs requests from passing this one.
- */
- if (q->dma_drain_size && rq->data_len &&
- !(rq->cmd_flags & REQ_DONTPREP)) {
- /*
- * remove the space for the drain we added
- * so that we don't add it again
- */
- --rq->nr_phys_segments;
- }
-
- rq = NULL;
- break;
- } else if (ret == BLKPREP_KILL) {
- rq->cmd_flags |= REQ_QUIET;
- __blk_end_request(rq, -EIO, blk_rq_bytes(rq));
- } else {
- printk(KERN_ERR "%s: bad return=%d\n", __func__, ret);
- break;
- }
- }
-
- return rq;
-}
-EXPORT_SYMBOL(elv_next_request);
-
-void elv_dequeue_request(struct request_queue *q, struct request *rq)
-{
- BUG_ON(list_empty(&rq->queuelist));
- BUG_ON(ELV_ON_HASH(rq));
-
- list_del_init(&rq->queuelist);
-
- /*
- * the time frame between a request being removed from the lists
- * and to it is freed is accounted as io that is in progress at
- * the driver side.
- */
- if (blk_account_rq(rq))
- q->in_flight++;
-}
-
int elv_queue_empty(struct request_queue *q)
{
struct elevator_queue *e = q->elevator;
--
1.6.0.2
rq->start_time was initialized in init_request_from_bio() so special
requests didn't have start_time set. This has been okay as start_time
has been used only for fs requests; however, there is no indication of
this actually is the case or not. Set rq->start_time in blk_rq_init()
and guarantee that all initialized rq's have its start_time set. This
improves consistency at virtually no cost and future changes will make
use of the timestamp for !bio requests.
[ Impact: rq->start_time is valid for all requests ]
Signed-off-by: Tejun Heo <[email protected]>
---
block/blk-core.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index 274888b..4b39e9e 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -134,6 +134,7 @@ void blk_rq_init(struct request_queue *q, struct request *rq)
rq->cmd_len = BLK_MAX_CDB;
rq->tag = -1;
rq->ref_count = 1;
+ rq->start_time = jiffies;
}
EXPORT_SYMBOL(blk_rq_init);
@@ -1097,7 +1098,6 @@ void init_request_from_bio(struct request *req, struct bio *bio)
req->errors = 0;
req->hard_sector = req->sector = bio->bi_sector;
req->ioprio = bio_prio(bio);
- req->start_time = jiffies;
blk_rq_bio_prep(req->q, req, bio);
}
--
1.6.0.2
Request completion has gone through several changes and became a bit
messy over the time. Clean it up.
1. end_that_request_data() is a thin wrapper around
end_that_request_data_first() which checks whether bio is NULL
before doing anything and handles bidi completion.
blk_update_request() is a thin wrapper around
end_that_request_data() which clears nr_sectors on the last
iteration but doesn't use the bidi completion.
Clean it up by moving the initial bio NULL check and nr_sectors
clearing on the last iteration into end_that_request_data() and
renaming it to blk_update_request(), which makes blk_end_io() the
only user of end_that_request_data(). Collapse
end_that_request_data() into blk_end_io().
2. There are four visible completion variants - blk_end_request(),
__blk_end_request(), blk_end_bidi_request() and end_request().
blk_end_request() and blk_end_bidi_request() uses blk_end_request()
as the backend but __blk_end_request() and end_request() use
separate implementation in __blk_end_request() due to different
locking rules.
Make blk_end_io() handle both cases thus all four public completion
functions are thin wrappers around blk_end_io(). Rename
blk_end_io() to __blk_end_io() and export it and inline all public
completion functions.
3. As the whole request issue/completion usages are about to be
modified and audited, it's a good chance to convert completion
functions return bool which better indicates the intended meaning
of return values.
4. The function name end_that_request_last() is from the days when it
was a public interface and slighly confusing. Give it a proper
internal name - finish_request().
5. Add description explaning that blk_end_bidi_request() can be safely
used for uni requests as suggested by Boaz Harrosh.
The only visible behavior change is from #1. nr_sectors counts are
cleared after the final iteration no matter which function is used to
complete the request. I couldn't find any place where the code
assumes those nr_sectors counters contain the values for the last
segment and this change is good as it makes the API much more
consistent as the end result is now same whether a request is
completed using [__]blk_end_request() alone or in combination with
blk_update_request().
[ Impact: cleanup, rq->*nr_sectors always updated after req completion ]
Signed-off-by: Tejun Heo <[email protected]>
Reviewed-by: Boaz Harrosh <[email protected]>
---
block/blk-core.c | 215 ++++++++++++------------------------------------
include/linux/blkdev.h | 117 +++++++++++++++++++++++---
2 files changed, 157 insertions(+), 175 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index 0f402c2..274888b 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1806,25 +1806,35 @@ void elv_dequeue_request(struct request_queue *q, struct request *rq)
}
/**
- * __end_that_request_first - end I/O on a request
- * @req: the request being processed
+ * blk_update_request - Special helper function for request stacking drivers
+ * @rq: the request being processed
* @error: %0 for success, < %0 for error
- * @nr_bytes: number of bytes to complete
+ * @nr_bytes: number of bytes to complete @rq
*
* Description:
- * Ends I/O on a number of bytes attached to @req, and sets it up
- * for the next range of segments (if any) in the cluster.
+ * Ends I/O on a number of bytes attached to @rq, but doesn't complete
+ * the request structure even if @rq doesn't have leftover.
+ * If @rq has leftover, sets it up for the next range of segments.
+ *
+ * This special helper function is only for request stacking drivers
+ * (e.g. request-based dm) so that they can handle partial completion.
+ * Actual device drivers should use blk_end_request instead.
+ *
+ * Passing the result of blk_rq_bytes() as @nr_bytes guarantees
+ * %false return from this function.
*
* Return:
- * %0 - we are done with this request, call end_that_request_last()
- * %1 - still buffers pending for this request
+ * %false - this request doesn't have any more data
+ * %true - this request has more data
**/
-static int __end_that_request_first(struct request *req, int error,
- int nr_bytes)
+bool blk_update_request(struct request *req, int error, unsigned int nr_bytes)
{
int total_bytes, bio_nbytes, next_idx = 0;
struct bio *bio;
+ if (!req->bio)
+ return false;
+
trace_block_rq_complete(req->q, req);
/*
@@ -1901,8 +1911,16 @@ static int __end_that_request_first(struct request *req, int error,
/*
* completely done
*/
- if (!req->bio)
- return 0;
+ if (!req->bio) {
+ /*
+ * Reset counters so that the request stacking driver
+ * can find how many bytes remain in the request
+ * later.
+ */
+ req->nr_sectors = req->hard_nr_sectors = 0;
+ req->current_nr_sectors = req->hard_cur_sectors = 0;
+ return false;
+ }
/*
* if the request wasn't completed, update state
@@ -1916,29 +1934,14 @@ static int __end_that_request_first(struct request *req, int error,
blk_recalc_rq_sectors(req, total_bytes >> 9);
blk_recalc_rq_segments(req);
- return 1;
-}
-
-static int end_that_request_data(struct request *rq, int error,
- unsigned int nr_bytes, unsigned int bidi_bytes)
-{
- if (rq->bio) {
- if (__end_that_request_first(rq, error, nr_bytes))
- return 1;
-
- /* Bidi request must be completed as a whole */
- if (blk_bidi_rq(rq) &&
- __end_that_request_first(rq->next_rq, error, bidi_bytes))
- return 1;
- }
-
- return 0;
+ return true;
}
+EXPORT_SYMBOL_GPL(blk_update_request);
/*
* queue lock must be held
*/
-static void end_that_request_last(struct request *req, int error)
+static void finish_request(struct request *req, int error)
{
if (blk_rq_tagged(req))
blk_queue_end_tag(req->q, req);
@@ -1964,161 +1967,47 @@ static void end_that_request_last(struct request *req, int error)
}
/**
- * blk_end_io - Generic end_io function to complete a request.
+ * __blk_end_io - Generic end_io function to complete a request.
* @rq: the request being processed
* @error: %0 for success, < %0 for error
* @nr_bytes: number of bytes to complete @rq
* @bidi_bytes: number of bytes to complete @rq->next_rq
+ * @locked: whether rq->q->queue_lock is held on entry
*
* Description:
* Ends I/O on a number of bytes attached to @rq and @rq->next_rq.
* If @rq has leftover, sets it up for the next range of segments.
*
* Return:
- * %0 - we are done with this request
- * %1 - this request is not freed yet, it still has pending buffers.
+ * %false - we are done with this request
+ * %true - this request is not freed yet, it still has pending buffers.
**/
-static int blk_end_io(struct request *rq, int error, unsigned int nr_bytes,
- unsigned int bidi_bytes)
+bool __blk_end_io(struct request *rq, int error, unsigned int nr_bytes,
+ unsigned int bidi_bytes, bool locked)
{
struct request_queue *q = rq->q;
unsigned long flags = 0UL;
- if (end_that_request_data(rq, error, nr_bytes, bidi_bytes))
- return 1;
-
- add_disk_randomness(rq->rq_disk);
-
- spin_lock_irqsave(q->queue_lock, flags);
- end_that_request_last(rq, error);
- spin_unlock_irqrestore(q->queue_lock, flags);
-
- return 0;
-}
+ if (blk_update_request(rq, error, nr_bytes))
+ return true;
-/**
- * blk_end_request - Helper function for drivers to complete the request.
- * @rq: the request being processed
- * @error: %0 for success, < %0 for error
- * @nr_bytes: number of bytes to complete
- *
- * Description:
- * Ends I/O on a number of bytes attached to @rq.
- * If @rq has leftover, sets it up for the next range of segments.
- *
- * Return:
- * %0 - we are done with this request
- * %1 - still buffers pending for this request
- **/
-int blk_end_request(struct request *rq, int error, unsigned int nr_bytes)
-{
- return blk_end_io(rq, error, nr_bytes, 0);
-}
-EXPORT_SYMBOL_GPL(blk_end_request);
-
-/**
- * __blk_end_request - Helper function for drivers to complete the request.
- * @rq: the request being processed
- * @error: %0 for success, < %0 for error
- * @nr_bytes: number of bytes to complete
- *
- * Description:
- * Must be called with queue lock held unlike blk_end_request().
- *
- * Return:
- * %0 - we are done with this request
- * %1 - still buffers pending for this request
- **/
-int __blk_end_request(struct request *rq, int error, unsigned int nr_bytes)
-{
- if (rq->bio && __end_that_request_first(rq, error, nr_bytes))
- return 1;
+ /* Bidi request must be completed as a whole */
+ if (unlikely(blk_bidi_rq(rq)) &&
+ blk_update_request(rq->next_rq, error, bidi_bytes))
+ return true;
add_disk_randomness(rq->rq_disk);
- end_that_request_last(rq, error);
-
- return 0;
-}
-EXPORT_SYMBOL_GPL(__blk_end_request);
-
-/**
- * blk_end_bidi_request - Helper function for drivers to complete bidi request.
- * @rq: the bidi request being processed
- * @error: %0 for success, < %0 for error
- * @nr_bytes: number of bytes to complete @rq
- * @bidi_bytes: number of bytes to complete @rq->next_rq
- *
- * Description:
- * Ends I/O on a number of bytes attached to @rq and @rq->next_rq.
- *
- * Return:
- * %0 - we are done with this request
- * %1 - still buffers pending for this request
- **/
-int blk_end_bidi_request(struct request *rq, int error, unsigned int nr_bytes,
- unsigned int bidi_bytes)
-{
- return blk_end_io(rq, error, nr_bytes, bidi_bytes);
-}
-EXPORT_SYMBOL_GPL(blk_end_bidi_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
- *
- * Description:
- * Ends I/O on the current segment of a request. If that is the only
- * remaining segment, the request is also completed and freed.
- *
- * This is a remnant of how older block drivers handled I/O completions.
- * Modern drivers typically end I/O on the full request in one go, unless
- * 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 a request.
- **/
-void end_request(struct request *req, int uptodate)
-{
- int error = 0;
-
- if (uptodate <= 0)
- error = uptodate ? uptodate : -EIO;
-
- __blk_end_request(req, error, req->hard_cur_sectors << 9);
-}
-EXPORT_SYMBOL(end_request);
+ if (!locked) {
+ spin_lock_irqsave(q->queue_lock, flags);
+ finish_request(rq, error);
+ spin_unlock_irqrestore(q->queue_lock, flags);
+ } else
+ finish_request(rq, error);
-/**
- * blk_update_request - Special helper function for request stacking drivers
- * @rq: the request being processed
- * @error: %0 for success, < %0 for error
- * @nr_bytes: number of bytes to complete @rq
- *
- * Description:
- * Ends I/O on a number of bytes attached to @rq, but doesn't complete
- * the request structure even if @rq doesn't have leftover.
- * If @rq has leftover, sets it up for the next range of segments.
- *
- * This special helper function is only for request stacking drivers
- * (e.g. request-based dm) so that they can handle partial completion.
- * Actual device drivers should use blk_end_request instead.
- */
-void blk_update_request(struct request *rq, int error, unsigned int nr_bytes)
-{
- if (!end_that_request_data(rq, error, nr_bytes, 0)) {
- /*
- * These members are not updated in end_that_request_data()
- * when all bios are completed.
- * Update them so that the request stacking driver can find
- * how many bytes remain in the request later.
- */
- rq->nr_sectors = rq->hard_nr_sectors = 0;
- rq->current_nr_sectors = rq->hard_cur_sectors = 0;
- }
+ return false;
}
-EXPORT_SYMBOL_GPL(blk_update_request);
+EXPORT_SYMBOL_GPL(__blk_end_io);
void blk_rq_bio_prep(struct request_queue *q, struct request *rq,
struct bio *bio)
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index e24f56d..0d94ad6 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -837,27 +837,120 @@ extern unsigned int blk_rq_bytes(struct request *rq);
extern unsigned int blk_rq_cur_bytes(struct request *rq);
/*
- * blk_end_request() and friends.
- * __blk_end_request() and end_request() must be called with
- * the request queue spinlock acquired.
+ * Request completion related functions.
+ *
+ * blk_update_request() completes given number of bytes and updates
+ * the request without completing it.
+ *
+ * blk_end_request() and friends. __blk_end_request() and
+ * end_request() must be called with the request queue spinlock
+ * acquired.
*
* Several drivers define their own end_request and call
* blk_end_request() for parts of the original function.
* This prevents code duplication in drivers.
*/
-extern int blk_end_request(struct request *rq, int error,
- unsigned int nr_bytes);
-extern int __blk_end_request(struct request *rq, int error,
- unsigned int nr_bytes);
-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 bool blk_update_request(struct request *rq, int error,
+ unsigned int nr_bytes);
+
+/* internal function, subject to change, don't ever use directly */
+extern bool __blk_end_io(struct request *rq, int error,
+ unsigned int nr_bytes, unsigned int bidi_bytes,
+ bool locked);
+
+/**
+ * blk_end_request - Helper function for drivers to complete the request.
+ * @rq: the request being processed
+ * @error: %0 for success, < %0 for error
+ * @nr_bytes: number of bytes to complete
+ *
+ * Description:
+ * Ends I/O on a number of bytes attached to @rq.
+ * If @rq has leftover, sets it up for the next range of segments.
+ *
+ * Return:
+ * %false - we are done with this request
+ * %true - still buffers pending for this request
+ **/
+static inline bool blk_end_request(struct request *rq, int error,
+ unsigned int nr_bytes)
+{
+ return __blk_end_io(rq, error, nr_bytes, 0, false);
+}
+
+/**
+ * __blk_end_request - Helper function for drivers to complete the request.
+ * @rq: the request being processed
+ * @error: %0 for success, < %0 for error
+ * @nr_bytes: number of bytes to complete
+ *
+ * Description:
+ * Must be called with queue lock held unlike blk_end_request().
+ *
+ * Return:
+ * %false - we are done with this request
+ * %true - still buffers pending for this request
+ **/
+static inline bool __blk_end_request(struct request *rq, int error,
+ unsigned int nr_bytes)
+{
+ return __blk_end_io(rq, error, nr_bytes, 0, true);
+}
+
+/**
+ * blk_end_bidi_request - Helper function for drivers to complete bidi request.
+ * @rq: the bidi request being processed
+ * @error: %0 for success, < %0 for error
+ * @nr_bytes: number of bytes to complete @rq
+ * @bidi_bytes: number of bytes to complete @rq->next_rq
+ *
+ * Description:
+ * Ends I/O on a number of bytes attached to @rq and @rq->next_rq.
+ * Drivers that supports bidi can safely call this member for any
+ * type of request, bidi or uni. In the later case @bidi_bytes is
+ * just ignored.
+ *
+ * Return:
+ * %false - we are done with this request
+ * %true - still buffers pending for this request
+ **/
+static inline bool blk_end_bidi_request(struct request *rq, int error,
+ unsigned int nr_bytes,
+ unsigned int bidi_bytes)
+{
+ return __blk_end_io(rq, error, nr_bytes, bidi_bytes, false);
+}
+
+/**
+ * end_request - end I/O on the current segment of the request
+ * @rq: the request being processed
+ * @uptodate: error value or %0/%1 uptodate flag
+ *
+ * Description:
+ * Ends I/O on the current segment of a request. If that is the only
+ * remaining segment, the request is also completed and freed.
+ *
+ * This is a remnant of how older block drivers handled I/O completions.
+ * Modern drivers typically end I/O on the full request in one go, unless
+ * 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 a request.
+ **/
+static inline void end_request(struct request *rq, int uptodate)
+{
+ int error = 0;
+
+ if (uptodate <= 0)
+ error = uptodate ? uptodate : -EIO;
+
+ __blk_end_io(rq, error, rq->hard_cur_sectors << 9, 0, true);
+}
+
extern void blk_complete_request(struct request *);
extern void __blk_complete_request(struct request *);
extern void blk_abort_request(struct request *);
extern void blk_abort_queue(struct request_queue *);
-extern void blk_update_request(struct request *rq, int error,
- unsigned int nr_bytes);
/*
* Access functions for manipulating queue properties
--
1.6.0.2
With recent IDE updates, blk_end_request_callback() doesn't have any
user now. Kill it.
[ Impact: removal of unused convoluted interface ]
Signed-off-by: Tejun Heo <[email protected]>
---
block/blk-core.c | 48 +++---------------------------------------------
include/linux/blkdev.h | 3 ---
2 files changed, 3 insertions(+), 48 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index 148492e..0f402c2 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1969,10 +1969,6 @@ static void end_that_request_last(struct request *req, int error)
* @error: %0 for success, < %0 for error
* @nr_bytes: number of bytes to complete @rq
* @bidi_bytes: number of bytes to complete @rq->next_rq
- * @drv_callback: function called between completion of bios in the request
- * and completion of the request.
- * If the callback returns non %0, this helper returns without
- * completion of the request.
*
* Description:
* Ends I/O on a number of bytes attached to @rq and @rq->next_rq.
@@ -1983,8 +1979,7 @@ static void end_that_request_last(struct request *req, int error)
* %1 - this request is not freed yet, it still has pending buffers.
**/
static int blk_end_io(struct request *rq, int error, unsigned int nr_bytes,
- unsigned int bidi_bytes,
- int (drv_callback)(struct request *))
+ unsigned int bidi_bytes)
{
struct request_queue *q = rq->q;
unsigned long flags = 0UL;
@@ -1992,10 +1987,6 @@ static int blk_end_io(struct request *rq, int error, unsigned int nr_bytes,
if (end_that_request_data(rq, error, nr_bytes, bidi_bytes))
return 1;
- /* Special feature for tricky drivers */
- if (drv_callback && drv_callback(rq))
- return 1;
-
add_disk_randomness(rq->rq_disk);
spin_lock_irqsave(q->queue_lock, flags);
@@ -2021,7 +2012,7 @@ static int blk_end_io(struct request *rq, int error, unsigned int nr_bytes,
**/
int blk_end_request(struct request *rq, int error, unsigned int nr_bytes)
{
- return blk_end_io(rq, error, nr_bytes, 0, NULL);
+ return blk_end_io(rq, error, nr_bytes, 0);
}
EXPORT_SYMBOL_GPL(blk_end_request);
@@ -2068,7 +2059,7 @@ EXPORT_SYMBOL_GPL(__blk_end_request);
int blk_end_bidi_request(struct request *rq, int error, unsigned int nr_bytes,
unsigned int bidi_bytes)
{
- return blk_end_io(rq, error, nr_bytes, bidi_bytes, NULL);
+ return blk_end_io(rq, error, nr_bytes, bidi_bytes);
}
EXPORT_SYMBOL_GPL(blk_end_bidi_request);
@@ -2129,39 +2120,6 @@ void blk_update_request(struct request *rq, int error, unsigned int nr_bytes)
}
EXPORT_SYMBOL_GPL(blk_update_request);
-/**
- * blk_end_request_callback - Special helper function for tricky drivers
- * @rq: the request being processed
- * @error: %0 for success, < %0 for error
- * @nr_bytes: number of bytes to complete
- * @drv_callback: function called between completion of bios in the request
- * and completion of the request.
- * If the callback returns non %0, this helper returns without
- * completion of the request.
- *
- * Description:
- * Ends I/O on a number of bytes attached to @rq.
- * If @rq has leftover, sets it up for the next range of segments.
- *
- * This special helper function is used only for existing tricky drivers.
- * (e.g. cdrom_newpc_intr() of ide-cd)
- * This interface will be removed when such drivers are rewritten.
- * Don't use this interface in other places anymore.
- *
- * Return:
- * %0 - we are done with this request
- * %1 - this request is not freed yet.
- * this request still has pending buffers or
- * the driver doesn't want to finish this request yet.
- **/
-int blk_end_request_callback(struct request *rq, int error,
- unsigned int nr_bytes,
- int (drv_callback)(struct request *))
-{
- return blk_end_io(rq, error, nr_bytes, 0, drv_callback);
-}
-EXPORT_SYMBOL_GPL(blk_end_request_callback);
-
void blk_rq_bio_prep(struct request_queue *q, struct request *rq,
struct bio *bio)
{
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 8d12dba..e24f56d 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -852,9 +852,6 @@ extern int __blk_end_request(struct request *rq, int error,
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 int blk_end_request_callback(struct request *rq, int error,
- unsigned int nr_bytes,
- int (drv_callback)(struct request *));
extern void blk_complete_request(struct request *);
extern void __blk_complete_request(struct request *);
extern void blk_abort_request(struct request *);
--
1.6.0.2
omap mailbox uses rq->data as the second opaque pointer to carry
mbox_msg_t and rq->special message argument which is needed only for
tx. Add and use omap_msg_tx_data struct for tx and use rq->special
for mbox_msg_t for rx such that only rq->special is used as opaque
pointer.
[ Impact: cleanup rq->data usage, extra kmalloc in msg_send ]
Signed-off-by: Tejun Heo <[email protected]>
Cc: Russell King <[email protected]>
---
arch/arm/plat-omap/mailbox.c | 43 ++++++++++++++++++++++++++++++-----------
1 files changed, 31 insertions(+), 12 deletions(-)
diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index cf81bad..538ba75 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -147,24 +147,40 @@ static int __mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg, void *arg)
return ret;
}
+struct omap_msg_tx_data {
+ mbox_msg_t msg;
+ void *arg;
+};
+
+static void omap_msg_tx_end_io(struct request *rq, int error)
+{
+ kfree(rq->special);
+ __blk_put_request(rq->q, rq);
+}
+
int omap_mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg, void* arg)
{
+ struct omap_msg_tx_data *tx_data;
struct request *rq;
struct request_queue *q = mbox->txq->queue;
- int ret = 0;
+
+ tx_data = kmalloc(sizeof(*tx_data), GFP_ATOMIC);
+ if (unlikely(!tx_data))
+ return -ENOMEM;
rq = blk_get_request(q, WRITE, GFP_ATOMIC);
if (unlikely(!rq)) {
- ret = -ENOMEM;
- goto fail;
+ kfree(tx_data);
+ return -ENOMEM;
}
- rq->data = (void *)msg;
- blk_insert_request(q, rq, 0, arg);
+ tx_data->msg = msg;
+ tx_data->arg = arg;
+ rq->end_io = omap_msg_tx_end_io;
+ blk_insert_request(q, rq, 0, tx_data);
schedule_work(&mbox->txq->work);
- fail:
- return ret;
+ return 0;
}
EXPORT_SYMBOL(omap_mbox_msg_send);
@@ -178,6 +194,8 @@ static void mbox_tx_work(struct work_struct *work)
struct request_queue *q = mbox->txq->queue;
while (1) {
+ struct omap_msg_tx_data *tx_data;
+
spin_lock(q->queue_lock);
rq = elv_next_request(q);
spin_unlock(q->queue_lock);
@@ -185,7 +203,9 @@ static void mbox_tx_work(struct work_struct *work)
if (!rq)
break;
- ret = __mbox_msg_send(mbox, (mbox_msg_t) rq->data, rq->special);
+ tx_data = rq->special;
+
+ ret = __mbox_msg_send(mbox, tx_data->msg, tx_data->arg);
if (ret) {
enable_mbox_irq(mbox, IRQ_TX);
return;
@@ -222,7 +242,7 @@ static void mbox_rx_work(struct work_struct *work)
if (!rq)
break;
- msg = (mbox_msg_t) rq->data;
+ msg = (mbox_msg_t)rq->special;
blk_end_request_all(rq, 0);
mbox->rxq->callback((void *)msg);
}
@@ -260,7 +280,6 @@ static void __mbox_rx_interrupt(struct omap_mbox *mbox)
goto nomem;
msg = mbox_fifo_read(mbox);
- rq->data = (void *)msg;
if (unlikely(mbox_seq_test(mbox, msg))) {
pr_info("mbox: Illegal seq bit!(%08x)\n", msg);
@@ -268,7 +287,7 @@ static void __mbox_rx_interrupt(struct omap_mbox *mbox)
mbox->err_notify();
}
- blk_insert_request(q, rq, 0, NULL);
+ blk_insert_request(q, rq, 0, (void *)msg);
if (mbox->ops->type == OMAP_MBOX_TYPE1)
break;
}
@@ -331,7 +350,7 @@ omap_mbox_read(struct device *dev, struct device_attribute *attr, char *buf)
if (!rq)
break;
- *p = (mbox_msg_t) rq->data;
+ *p = (mbox_msg_t)rq->special;
blk_end_request_all(rq, 0);
--
1.6.0.2
Now that all block request data transfer is done via bio, rq->data
isn't used. Kill it.
While at it, make the roles of rq->special and buffer clear.
[ Impact: drop now unncessary field from struct request ]
Signed-off-by: Tejun Heo <[email protected]>
Cc: Boaz Harrosh <[email protected]>
---
block/blk-core.c | 5 ++---
block/blk-map.c | 6 +++---
block/scsi_ioctl.c | 1 -
drivers/scsi/scsi_lib.c | 1 -
include/linux/blkdev.h | 5 ++---
5 files changed, 7 insertions(+), 11 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index b7c9464..339e5e9 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -189,10 +189,9 @@ void blk_dump_rq_flags(struct request *rq, char *msg)
(unsigned long long)rq->sector,
rq->nr_sectors,
rq->current_nr_sectors);
- printk(KERN_INFO " bio %p, biotail %p, buffer %p, data %p, len %u\n",
+ printk(KERN_INFO " bio %p, biotail %p, buffer %p, len %u\n",
rq->bio, rq->biotail,
- rq->buffer, rq->data,
- rq->data_len);
+ rq->buffer, rq->data_len);
if (blk_pc_request(rq)) {
printk(KERN_INFO " cdb: ");
diff --git a/block/blk-map.c b/block/blk-map.c
index f103729..694fefa 100644
--- a/block/blk-map.c
+++ b/block/blk-map.c
@@ -156,7 +156,7 @@ int blk_rq_map_user(struct request_queue *q, struct request *rq,
if (!bio_flagged(bio, BIO_USER_MAPPED))
rq->cmd_flags |= REQ_COPY_USER;
- rq->buffer = rq->data = NULL;
+ rq->buffer = NULL;
return 0;
unmap_rq:
blk_rq_unmap_user(bio);
@@ -235,7 +235,7 @@ int blk_rq_map_user_iov(struct request_queue *q, struct request *rq,
blk_queue_bounce(q, &bio);
bio_get(bio);
blk_rq_bio_prep(q, rq, bio);
- rq->buffer = rq->data = NULL;
+ rq->buffer = NULL;
return 0;
}
EXPORT_SYMBOL(blk_rq_map_user_iov);
@@ -313,7 +313,7 @@ int blk_rq_map_kern(struct request_queue *q, struct request *rq, void *kbuf,
blk_rq_bio_prep(q, rq, bio);
blk_queue_bounce(q, &rq->bio);
- rq->buffer = rq->data = NULL;
+ rq->buffer = NULL;
return 0;
}
EXPORT_SYMBOL(blk_rq_map_kern);
diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c
index 82a0ca2..bb9aa43 100644
--- a/block/scsi_ioctl.c
+++ b/block/scsi_ioctl.c
@@ -500,7 +500,6 @@ static int __blk_send_generic(struct request_queue *q, struct gendisk *bd_disk,
rq = blk_get_request(q, WRITE, __GFP_WAIT);
rq->cmd_type = REQ_TYPE_BLOCK_PC;
- rq->data = NULL;
rq->data_len = 0;
rq->extra_len = 0;
rq->timeout = BLK_DEFAULT_SG_TIMEOUT;
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 756ac7c..aa9fc57 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1088,7 +1088,6 @@ int scsi_setup_blk_pc_cmnd(struct scsi_device *sdev, struct request *req)
return ret;
} else {
BUG_ON(req->data_len);
- BUG_ON(req->data);
memset(&cmd->sdb, 0, sizeof(cmd->sdb));
req->buffer = NULL;
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 0bb3ed4..b1de769 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -209,8 +209,8 @@ struct request {
unsigned short ioprio;
- void *special;
- char *buffer;
+ void *special; /* opaque pointer available for LLD use */
+ char *buffer; /* kaddr of the current segment if available */
int tag;
int errors;
@@ -227,7 +227,6 @@ struct request {
unsigned int data_len;
unsigned int extra_len; /* length of alignment and padding */
unsigned int sense_len;
- void *data;
void *sense;
unsigned long deadline;
--
1.6.0.2
There are many [__]blk_end_request() call sites which call it with
full request length and expect full completion. Many of them ensure
that the request actually completes by doing BUG_ON() the return
value, which is awkward and error-prone.
This patch adds [__]blk_end_request_all() which takes @rq and @error
and fully completes the request. BUG_ON() is added to to ensure that
this actually happens.
Most conversions are simple but there are a few noteworthy ones.
* cdrom/viocd: viocd_end_request() replaced with direct calls to
__blk_end_request_all().
* s390/block/dasd: dasd_end_request() replaced with direct calls to
__blk_end_request_all().
* s390/char/tape_block: tapeblock_end_request() replaced with direct
calls to blk_end_request_all().
[ Impact: cleanup ]
Signed-off-by: Tejun Heo <[email protected]>
Cc: Russell King <[email protected]>
Cc: Stephen Rothwell <[email protected]>
Cc: Mike Miller <[email protected]>
Cc: Martin Schwidefsky <[email protected]>
Cc: Jeff Garzik <[email protected]>
Cc: Rusty Russell <[email protected]>
Cc: Jeremy Fitzhardinge <[email protected]>
Cc: Alex Dubov <[email protected]>
Cc: James Bottomley <[email protected]>
---
arch/arm/plat-omap/mailbox.c | 11 +++--------
block/blk-barrier.c | 9 ++-------
block/blk-core.c | 2 +-
block/elevator.c | 2 +-
drivers/block/cpqarray.c | 3 +--
drivers/block/sx8.c | 3 +--
drivers/block/virtio_blk.c | 2 +-
drivers/block/xen-blkfront.c | 4 +---
drivers/cdrom/gdrom.c | 2 +-
drivers/cdrom/viocd.c | 25 ++++---------------------
drivers/memstick/core/mspro_block.c | 2 +-
drivers/s390/block/dasd.c | 17 ++++-------------
drivers/s390/char/tape_block.c | 15 ++++-----------
drivers/scsi/scsi_lib.c | 2 +-
include/linux/blkdev.h | 32 ++++++++++++++++++++++++++++++++
15 files changed, 58 insertions(+), 73 deletions(-)
diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index 0abfbaa..cf81bad 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -192,8 +192,7 @@ static void mbox_tx_work(struct work_struct *work)
}
spin_lock(q->queue_lock);
- if (__blk_end_request(rq, 0, 0))
- BUG();
+ __blk_end_request_all(rq, 0);
spin_unlock(q->queue_lock);
}
}
@@ -224,10 +223,7 @@ static void mbox_rx_work(struct work_struct *work)
break;
msg = (mbox_msg_t) rq->data;
-
- if (blk_end_request(rq, 0, 0))
- BUG();
-
+ blk_end_request_all(rq, 0);
mbox->rxq->callback((void *)msg);
}
}
@@ -337,8 +333,7 @@ omap_mbox_read(struct device *dev, struct device_attribute *attr, char *buf)
*p = (mbox_msg_t) rq->data;
- if (blk_end_request(rq, 0, 0))
- BUG();
+ blk_end_request_all(rq, 0);
if (unlikely(mbox_seq_test(mbox, *p))) {
pr_info("mbox: Illegal seq bit!(%08x) ignored\n", *p);
diff --git a/block/blk-barrier.c b/block/blk-barrier.c
index 20b4111..c8d0876 100644
--- a/block/blk-barrier.c
+++ b/block/blk-barrier.c
@@ -106,10 +106,7 @@ bool blk_ordered_complete_seq(struct request_queue *q, unsigned seq, int error)
*/
q->ordseq = 0;
rq = q->orig_bar_rq;
-
- if (__blk_end_request(rq, q->orderr, blk_rq_bytes(rq)))
- BUG();
-
+ __blk_end_request_all(rq, q->orderr);
return true;
}
@@ -252,9 +249,7 @@ bool blk_do_ordered(struct request_queue *q, struct request **rqp)
* with prejudice.
*/
elv_dequeue_request(q, rq);
- if (__blk_end_request(rq, -EOPNOTSUPP,
- blk_rq_bytes(rq)))
- BUG();
+ __blk_end_request_all(rq, -EOPNOTSUPP);
*rqp = NULL;
return false;
}
diff --git a/block/blk-core.c b/block/blk-core.c
index 4b39e9e..b7c9464 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1778,7 +1778,7 @@ struct request *elv_next_request(struct request_queue *q)
break;
} else if (ret == BLKPREP_KILL) {
rq->cmd_flags |= REQ_QUIET;
- __blk_end_request(rq, -EIO, blk_rq_bytes(rq));
+ __blk_end_request_all(rq, -EIO);
} else {
printk(KERN_ERR "%s: bad return=%d\n", __func__, ret);
break;
diff --git a/block/elevator.c b/block/elevator.c
index b03b875..1af5d9f 100644
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -810,7 +810,7 @@ void elv_abort_queue(struct request_queue *q)
rq = list_entry_rq(q->queue_head.next);
rq->cmd_flags |= REQ_QUIET;
trace_block_rq_abort(q, rq);
- __blk_end_request(rq, -EIO, blk_rq_bytes(rq));
+ __blk_end_request_all(rq, -EIO);
}
}
EXPORT_SYMBOL(elv_abort_queue);
diff --git a/drivers/block/cpqarray.c b/drivers/block/cpqarray.c
index ca268ca..488a8f4 100644
--- a/drivers/block/cpqarray.c
+++ b/drivers/block/cpqarray.c
@@ -1024,8 +1024,7 @@ static inline void complete_command(cmdlist_t *cmd, int timeout)
cmd->req.sg[i].size, ddir);
DBGPX(printk("Done with %p\n", rq););
- if (__blk_end_request(rq, error, blk_rq_bytes(rq)))
- BUG();
+ __blk_end_request_all(rq, error);
}
/*
diff --git a/drivers/block/sx8.c b/drivers/block/sx8.c
index ff0448e..60e85bb 100644
--- a/drivers/block/sx8.c
+++ b/drivers/block/sx8.c
@@ -749,8 +749,7 @@ static inline void carm_end_request_queued(struct carm_host *host,
struct request *req = crq->rq;
int rc;
- rc = __blk_end_request(req, error, blk_rq_bytes(req));
- assert(rc == 0);
+ __blk_end_request_all(req, error);
rc = carm_put_request(host, crq);
assert(rc == 0);
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 5d34764..50745e6 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -62,7 +62,7 @@ static void blk_done(struct virtqueue *vq)
break;
}
- __blk_end_request(vbr->req, error, blk_rq_bytes(vbr->req));
+ __blk_end_request_all(vbr->req, error);
list_del(&vbr->list);
mempool_free(vbr, vblk->pool);
}
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 8f90508..cd6cfe3 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -551,7 +551,6 @@ static irqreturn_t blkif_interrupt(int irq, void *dev_id)
for (i = info->ring.rsp_cons; i != rp; i++) {
unsigned long id;
- int ret;
bret = RING_GET_RESPONSE(&info->ring, i);
id = bret->id;
@@ -578,8 +577,7 @@ static irqreturn_t blkif_interrupt(int irq, void *dev_id)
dev_dbg(&info->xbdev->dev, "Bad return from blkdev data "
"request: %x\n", bret->status);
- ret = __blk_end_request(req, error, blk_rq_bytes(req));
- BUG_ON(ret);
+ __blk_end_request_all(req, error);
break;
default:
BUG();
diff --git a/drivers/cdrom/gdrom.c b/drivers/cdrom/gdrom.c
index 2eecb77..fee9a9e 100644
--- a/drivers/cdrom/gdrom.c
+++ b/drivers/cdrom/gdrom.c
@@ -632,7 +632,7 @@ static void gdrom_readdisk_dma(struct work_struct *work)
* before handling ending the request */
spin_lock(&gdrom_lock);
list_del_init(&req->queuelist);
- __blk_end_request(req, err, blk_rq_bytes(req));
+ __blk_end_request_all(req, err);
}
spin_unlock(&gdrom_lock);
kfree(read_command);
diff --git a/drivers/cdrom/viocd.c b/drivers/cdrom/viocd.c
index 1392935..cc3efa0 100644
--- a/drivers/cdrom/viocd.c
+++ b/drivers/cdrom/viocd.c
@@ -291,23 +291,6 @@ static int send_request(struct request *req)
return 0;
}
-static void viocd_end_request(struct request *req, int error)
-{
- int nsectors = req->hard_nr_sectors;
-
- /*
- * Make sure it's fully ended, and ensure that we process
- * at least one sector.
- */
- if (blk_pc_request(req))
- nsectors = (req->data_len + 511) >> 9;
- if (!nsectors)
- nsectors = 1;
-
- if (__blk_end_request(req, error, nsectors << 9))
- BUG();
-}
-
static int rwreq;
static void do_viocd_request(struct request_queue *q)
@@ -316,11 +299,11 @@ static void do_viocd_request(struct request_queue *q)
while ((rwreq == 0) && ((req = elv_next_request(q)) != NULL)) {
if (!blk_fs_request(req))
- viocd_end_request(req, -EIO);
+ __blk_end_request_all(req, -EIO);
else if (send_request(req) < 0) {
printk(VIOCD_KERN_WARNING
"unable to send message to OS/400!");
- viocd_end_request(req, -EIO);
+ __blk_end_request_all(req, -EIO);
} else
rwreq++;
}
@@ -531,9 +514,9 @@ return_complete:
"with rc %d:0x%04X: %s\n",
req, event->xRc,
bevent->sub_result, err->msg);
- viocd_end_request(req, -EIO);
+ __blk_end_request_all(req, -EIO);
} else
- viocd_end_request(req, 0);
+ __blk_end_request_all(req, 0);
/* restart handling of incoming requests */
spin_unlock_irqrestore(&viocd_reqlock, flags);
diff --git a/drivers/memstick/core/mspro_block.c b/drivers/memstick/core/mspro_block.c
index de143de..a416346 100644
--- a/drivers/memstick/core/mspro_block.c
+++ b/drivers/memstick/core/mspro_block.c
@@ -826,7 +826,7 @@ static void mspro_block_submit_req(struct request_queue *q)
if (msb->eject) {
while ((req = elv_next_request(q)) != NULL)
- __blk_end_request(req, -ENODEV, blk_rq_bytes(req));
+ __blk_end_request_all(req, -ENODEV);
return;
}
diff --git a/drivers/s390/block/dasd.c b/drivers/s390/block/dasd.c
index d181527..fabec95 100644
--- a/drivers/s390/block/dasd.c
+++ b/drivers/s390/block/dasd.c
@@ -1614,15 +1614,6 @@ void dasd_block_clear_timer(struct dasd_block *block)
}
/*
- * posts the buffer_cache about a finalized request
- */
-static inline void dasd_end_request(struct request *req, int error)
-{
- if (__blk_end_request(req, error, blk_rq_bytes(req)))
- BUG();
-}
-
-/*
* Process finished error recovery ccw.
*/
static inline void __dasd_block_process_erp(struct dasd_block *block,
@@ -1676,7 +1667,7 @@ static void __dasd_process_request_queue(struct dasd_block *block)
"Rejecting write request %p",
req);
blkdev_dequeue_request(req);
- dasd_end_request(req, -EIO);
+ __blk_end_request_all(req, -EIO);
continue;
}
cqr = basedev->discipline->build_cp(basedev, block, req);
@@ -1705,7 +1696,7 @@ static void __dasd_process_request_queue(struct dasd_block *block)
"on request %p",
PTR_ERR(cqr), req);
blkdev_dequeue_request(req);
- dasd_end_request(req, -EIO);
+ __blk_end_request_all(req, -EIO);
continue;
}
/*
@@ -1731,7 +1722,7 @@ static void __dasd_cleanup_cqr(struct dasd_ccw_req *cqr)
status = cqr->block->base->discipline->free_cp(cqr, req);
if (status <= 0)
error = status ? status : -EIO;
- dasd_end_request(req, error);
+ __blk_end_request_all(req, error);
}
/*
@@ -2040,7 +2031,7 @@ static void dasd_flush_request_queue(struct dasd_block *block)
spin_lock_irq(&block->request_queue_lock);
while ((req = elv_next_request(block->request_queue))) {
blkdev_dequeue_request(req);
- dasd_end_request(req, -EIO);
+ __blk_end_request_all(req, -EIO);
}
spin_unlock_irq(&block->request_queue_lock);
}
diff --git a/drivers/s390/char/tape_block.c b/drivers/s390/char/tape_block.c
index f32e89e..86596d3 100644
--- a/drivers/s390/char/tape_block.c
+++ b/drivers/s390/char/tape_block.c
@@ -74,13 +74,6 @@ tapeblock_trigger_requeue(struct tape_device *device)
* Post finished request.
*/
static void
-tapeblock_end_request(struct request *req, int error)
-{
- if (blk_end_request(req, error, blk_rq_bytes(req)))
- BUG();
-}
-
-static void
__tapeblock_end_request(struct tape_request *ccw_req, void *data)
{
struct tape_device *device;
@@ -90,7 +83,7 @@ __tapeblock_end_request(struct tape_request *ccw_req, void *data)
device = ccw_req->device;
req = (struct request *) data;
- tapeblock_end_request(req, (ccw_req->rc == 0) ? 0 : -EIO);
+ blk_end_request_all(req, (ccw_req->rc == 0) ? 0 : -EIO);
if (ccw_req->rc == 0)
/* Update position. */
device->blk_data.block_position =
@@ -118,7 +111,7 @@ tapeblock_start_request(struct tape_device *device, struct request *req)
ccw_req = device->discipline->bread(device, req);
if (IS_ERR(ccw_req)) {
DBF_EVENT(1, "TBLOCK: bread failed\n");
- tapeblock_end_request(req, -EIO);
+ blk_end_request_all(req, -EIO);
return PTR_ERR(ccw_req);
}
ccw_req->callback = __tapeblock_end_request;
@@ -131,7 +124,7 @@ tapeblock_start_request(struct tape_device *device, struct request *req)
* Start/enqueueing failed. No retries in
* this case.
*/
- tapeblock_end_request(req, -EIO);
+ blk_end_request_all(req, -EIO);
device->discipline->free_bread(ccw_req);
}
@@ -177,7 +170,7 @@ tapeblock_requeue(struct work_struct *work) {
DBF_EVENT(1, "TBLOCK: Rejecting write request\n");
blkdev_dequeue_request(req);
spin_unlock_irq(&device->blk_data.request_queue_lock);
- tapeblock_end_request(req, -EIO);
+ blk_end_request_all(req, -EIO);
spin_lock_irq(&device->blk_data.request_queue_lock);
continue;
}
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index d1cb64a..756ac7c 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -922,7 +922,7 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
if (driver_byte(result) & DRIVER_SENSE)
scsi_print_sense("", cmd);
}
- blk_end_request(req, -EIO, blk_rq_bytes(req));
+ blk_end_request_all(req, -EIO);
scsi_next_command(cmd);
break;
case ACTION_REPREP:
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 0d94ad6..0a5f383 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -879,6 +879,22 @@ static inline bool blk_end_request(struct request *rq, int error,
}
/**
+ * blk_end_request_all - Helper function for drives to finish the request.
+ * @rq: the request to finish
+ * @err: %0 for success, < %0 for error
+ *
+ * Description:
+ * Completely finish @rq.
+ */
+static inline void blk_end_request_all(struct request *rq, int error)
+{
+ bool pending;
+
+ pending = blk_end_request(rq, error, blk_rq_bytes(rq));
+ BUG_ON(pending);
+}
+
+/**
* __blk_end_request - Helper function for drivers to complete the request.
* @rq: the request being processed
* @error: %0 for success, < %0 for error
@@ -898,6 +914,22 @@ static inline bool __blk_end_request(struct request *rq, int error,
}
/**
+ * __blk_end_request_all - Helper function for drives to finish the request.
+ * @rq: the request to finish
+ * @err: %0 for success, < %0 for error
+ *
+ * Description:
+ * Completely finish @rq. Must be called with queue lock held.
+ */
+static inline void __blk_end_request_all(struct request *rq, int error)
+{
+ bool pending;
+
+ pending = __blk_end_request(rq, error, blk_rq_bytes(rq));
+ BUG_ON(pending);
+}
+
+/**
* blk_end_bidi_request - Helper function for drivers to complete bidi request.
* @rq: the bidi request being processed
* @error: %0 for success, < %0 for error
--
1.6.0.2
Tejun Heo wrote:
> Now that all block request data transfer is done via bio, rq->data
> isn't used. Kill it.
>
> While at it, make the roles of rq->special and buffer clear.
>
> [ Impact: drop now unncessary field from struct request ]
Subject line was wrong. It should have been "block: kill rq->data".
Git tree updated accordingly.
Thanks.
--
tejun
end_request() has been kept around for backward compatibility;
however, it's about time for it to go away.
* There aren't too many users left.
* Its use of @updtodate is pretty confusing.
* In some cases, newer code ends up using mixture of end_request() and
[__]blk_end_request[_all](), which is way too confusing.
So, add [__]blk_end_request_cur() and replace end_request() with it.
Most conversions are straightforward. Noteworthy ones are...
* paride/pcd: next_request() updated to take 0/-errno instead of 1/0.
* paride/pf: pf_end_request() and next_request() updated to take
0/-errno instead of 1/0.
* xd: xd_readwrite() updated to return 0/-errno instead of 1/0.
* mtd/mtd_blkdevs: blktrans_discard_request() updated to return
0/-errno instead of 1/0. Unnecessary local variable res
initialization removed from mtd_blktrans_thread().
[ Impact: cleanup ]
Signed-off-by: Tejun Heo <[email protected]>
Cc: Joerg Dorchain <[email protected]>
Cc: Geert Uytterhoeven <[email protected]>
Cc: Tim Waugh <[email protected]>
Cc: Stephen Rothwell <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Jeremy Fitzhardinge <[email protected]>
Cc: Grant Likely <[email protected]>
Cc: Markus Lidel <[email protected]>
Cc: David Woodhouse <[email protected]>
Cc: Pete Zaitcev <[email protected]>
Cc: Laurent Vivier <[email protected]>
Cc: unsik Kim <[email protected]>
---
drivers/block/amiflop.c | 10 +++---
drivers/block/ataflop.c | 14 +++++-----
drivers/block/hd.c | 14 +++++-----
drivers/block/mg_disk.c | 16 +++++-----
drivers/block/paride/pcd.c | 12 ++++----
drivers/block/paride/pd.c | 5 ++-
drivers/block/paride/pf.c | 28 +++++++++---------
drivers/block/ps3disk.c | 6 ++--
drivers/block/swim.c | 14 +++++-----
drivers/block/swim3.c | 26 +++++++++---------
drivers/block/xd.c | 15 +++++-----
drivers/block/xen-blkfront.c | 2 +-
drivers/block/xsysace.c | 4 +-
drivers/block/z2ram.c | 4 +-
drivers/cdrom/gdrom.c | 6 ++--
drivers/message/i2o/i2o_block.c | 2 +-
drivers/mtd/mtd_blkdevs.c | 22 +++++++-------
drivers/sbus/char/jsflash.c | 8 +++---
include/linux/blkdev.h | 57 +++++++++++++++++++--------------------
19 files changed, 133 insertions(+), 132 deletions(-)
diff --git a/drivers/block/amiflop.c b/drivers/block/amiflop.c
index 8df436f..b99a2a6 100644
--- a/drivers/block/amiflop.c
+++ b/drivers/block/amiflop.c
@@ -1359,7 +1359,7 @@ static void redo_fd_request(void)
#endif
block = CURRENT->sector + cnt;
if ((int)block > floppy->blocks) {
- end_request(CURRENT, 0);
+ __blk_end_request_cur(CURRENT, -EIO);
goto repeat;
}
@@ -1373,11 +1373,11 @@ static void redo_fd_request(void)
if ((rq_data_dir(CURRENT) != READ) && (rq_data_dir(CURRENT) != WRITE)) {
printk(KERN_WARNING "do_fd_request: unknown command\n");
- end_request(CURRENT, 0);
+ __blk_end_request_cur(CURRENT, -EIO);
goto repeat;
}
if (get_track(drive, track) == -1) {
- end_request(CURRENT, 0);
+ __blk_end_request_cur(CURRENT, -EIO);
goto repeat;
}
@@ -1391,7 +1391,7 @@ static void redo_fd_request(void)
/* keep the drive spinning while writes are scheduled */
if (!fd_motor_on(drive)) {
- end_request(CURRENT, 0);
+ __blk_end_request_cur(CURRENT, -EIO);
goto repeat;
}
/*
@@ -1410,7 +1410,7 @@ static void redo_fd_request(void)
CURRENT->nr_sectors -= CURRENT->current_nr_sectors;
CURRENT->sector += CURRENT->current_nr_sectors;
- end_request(CURRENT, 1);
+ __blk_end_request_cur(CURRENT, 0);
goto repeat;
}
diff --git a/drivers/block/ataflop.c b/drivers/block/ataflop.c
index 4234c11..44a8702 100644
--- a/drivers/block/ataflop.c
+++ b/drivers/block/ataflop.c
@@ -612,7 +612,7 @@ static void fd_error( void )
CURRENT->errors++;
if (CURRENT->errors >= MAX_ERRORS) {
printk(KERN_ERR "fd%d: too many errors.\n", SelectedDrive );
- end_request(CURRENT, 0);
+ __blk_end_request_cur(CURRENT, -EIO);
}
else if (CURRENT->errors == RECALIBRATE_ERRORS) {
printk(KERN_WARNING "fd%d: recalibrating\n", SelectedDrive );
@@ -734,7 +734,7 @@ static void do_fd_action( int drive )
/* all sectors finished */
CURRENT->nr_sectors -= CURRENT->current_nr_sectors;
CURRENT->sector += CURRENT->current_nr_sectors;
- end_request(CURRENT, 1);
+ __blk_end_request_cur(CURRENT, 0);
redo_fd_request();
return;
}
@@ -1141,7 +1141,7 @@ static void fd_rwsec_done1(int status)
/* all sectors finished */
CURRENT->nr_sectors -= CURRENT->current_nr_sectors;
CURRENT->sector += CURRENT->current_nr_sectors;
- end_request(CURRENT, 1);
+ __blk_end_request_cur(CURRENT, 0);
redo_fd_request();
}
return;
@@ -1414,7 +1414,7 @@ repeat:
if (!UD.connected) {
/* drive not connected */
printk(KERN_ERR "Unknown Device: fd%d\n", drive );
- end_request(CURRENT, 0);
+ __blk_end_request_cur(CURRENT, -EIO);
goto repeat;
}
@@ -1430,12 +1430,12 @@ repeat:
/* user supplied disk type */
if (--type >= NUM_DISK_MINORS) {
printk(KERN_WARNING "fd%d: invalid disk format", drive );
- end_request(CURRENT, 0);
+ __blk_end_request_cur(CURRENT, -EIO);
goto repeat;
}
if (minor2disktype[type].drive_types > DriveType) {
printk(KERN_WARNING "fd%d: unsupported disk format", drive );
- end_request(CURRENT, 0);
+ __blk_end_request_cur(CURRENT, -EIO);
goto repeat;
}
type = minor2disktype[type].index;
@@ -1445,7 +1445,7 @@ repeat:
}
if (CURRENT->sector + 1 > UDT->blocks) {
- end_request(CURRENT, 0);
+ __blk_end_request_cur(CURRENT, -EIO);
goto repeat;
}
diff --git a/drivers/block/hd.c b/drivers/block/hd.c
index 3c11f06..f3dbc96 100644
--- a/drivers/block/hd.c
+++ b/drivers/block/hd.c
@@ -410,7 +410,7 @@ static void bad_rw_intr(void)
if (req != NULL) {
struct hd_i_struct *disk = req->rq_disk->private_data;
if (++req->errors >= MAX_ERRORS || (hd_error & BBD_ERR)) {
- end_request(req, 0);
+ __blk_end_request_cur(req, -EIO);
disk->special_op = disk->recalibrate = 1;
} else if (req->errors % RESET_FREQ == 0)
reset = 1;
@@ -466,7 +466,7 @@ ok_to_read:
req->buffer+512);
#endif
if (req->current_nr_sectors <= 0)
- end_request(req, 1);
+ __blk_end_request_cur(req, 0);
if (i > 0) {
SET_HANDLER(&read_intr);
return;
@@ -505,7 +505,7 @@ ok_to_write:
--req->current_nr_sectors;
req->buffer += 512;
if (!i || (req->bio && req->current_nr_sectors <= 0))
- end_request(req, 1);
+ __blk_end_request_cur(req, 0);
if (i > 0) {
SET_HANDLER(&write_intr);
outsw(HD_DATA, req->buffer, 256);
@@ -550,7 +550,7 @@ static void hd_times_out(unsigned long dummy)
#ifdef DEBUG
printk("%s: too many errors\n", name);
#endif
- end_request(CURRENT, 0);
+ __blk_end_request_cur(CURRENT, -EIO);
}
local_irq_disable();
hd_request();
@@ -566,7 +566,7 @@ static int do_special_op(struct hd_i_struct *disk, struct request *req)
}
if (disk->head > 16) {
printk("%s: cannot handle device with more than 16 heads - giving up\n", req->rq_disk->disk_name);
- end_request(req, 0);
+ __blk_end_request_cur(req, -EIO);
}
disk->special_op = 0;
return 1;
@@ -612,7 +612,7 @@ repeat:
((block+nsect) > get_capacity(req->rq_disk))) {
printk("%s: bad access: block=%d, count=%d\n",
req->rq_disk->disk_name, block, nsect);
- end_request(req, 0);
+ __blk_end_request_cur(req, -EIO);
goto repeat;
}
@@ -652,7 +652,7 @@ repeat:
break;
default:
printk("unknown hd-command\n");
- end_request(req, 0);
+ __blk_end_request_cur(req, -EIO);
break;
}
}
diff --git a/drivers/block/mg_disk.c b/drivers/block/mg_disk.c
index fb39d9a..f2a9692 100644
--- a/drivers/block/mg_disk.c
+++ b/drivers/block/mg_disk.c
@@ -280,7 +280,7 @@ static void mg_bad_rw_intr(struct mg_host *host)
if (req != NULL)
if (++req->errors >= MG_MAX_ERRORS ||
host->error == MG_ERR_TIMEOUT)
- end_request(req, 0);
+ __blk_end_request_cur(req, -EIO);
}
static unsigned int mg_out(struct mg_host *host,
@@ -346,7 +346,7 @@ static void mg_read(struct request *req)
if (req->current_nr_sectors <= 0) {
MG_DBG("remain : %d sects\n", remains);
- end_request(req, 1);
+ __blk_end_request_cur(req, 0);
if (remains > 0)
req = elv_next_request(host->breq);
}
@@ -390,7 +390,7 @@ static void mg_write(struct request *req)
if (req->current_nr_sectors <= 0) {
MG_DBG("remain : %d sects\n", remains);
- end_request(req, 1);
+ __blk_end_request_cur(req, 0);
if (remains > 0)
req = elv_next_request(host->breq);
}
@@ -443,7 +443,7 @@ ok_to_read:
/* let know if current segment done */
if (req->current_nr_sectors <= 0)
- end_request(req, 1);
+ __blk_end_request_cur(req, 0);
/* set handler if read remains */
if (i > 0) {
@@ -492,7 +492,7 @@ ok_to_write:
/* let know if current segment or all done */
if (!i || (req->bio && req->current_nr_sectors <= 0))
- end_request(req, 1);
+ __blk_end_request_cur(req, 0);
/* write 1 sector and set handler if remains */
if (i > 0) {
@@ -554,7 +554,7 @@ static void mg_request_poll(struct request_queue *q)
default:
printk(KERN_WARNING "%s:%d unknown command\n",
__func__, __LINE__);
- end_request(req, 0);
+ __blk_end_request_cur(req, -EIO);
break;
}
}
@@ -608,7 +608,7 @@ static unsigned int mg_issue_req(struct request *req,
default:
printk(KERN_WARNING "%s:%d unknown command\n",
__func__, __LINE__);
- end_request(req, 0);
+ __blk_end_request_cur(req, -EIO);
break;
}
return MG_ERR_NONE;
@@ -646,7 +646,7 @@ static void mg_request(struct request_queue *q)
"%s: bad access: sector=%d, count=%d\n",
req->rq_disk->disk_name,
sect_num, sect_cnt);
- end_request(req, 0);
+ __blk_end_request_cur(req, -EIO);
continue;
}
diff --git a/drivers/block/paride/pcd.c b/drivers/block/paride/pcd.c
index e91d4b4..9fd57c2 100644
--- a/drivers/block/paride/pcd.c
+++ b/drivers/block/paride/pcd.c
@@ -735,16 +735,16 @@ static void do_pcd_request(struct request_queue * q)
ps_set_intr(do_pcd_read, NULL, 0, nice);
return;
} else
- end_request(pcd_req, 0);
+ __blk_end_request_cur(pcd_req, -EIO);
}
}
-static inline void next_request(int success)
+static inline void next_request(int err)
{
unsigned long saved_flags;
spin_lock_irqsave(&pcd_lock, saved_flags);
- end_request(pcd_req, success);
+ __blk_end_request_cur(pcd_req, err);
pcd_busy = 0;
do_pcd_request(pcd_queue);
spin_unlock_irqrestore(&pcd_lock, saved_flags);
@@ -781,7 +781,7 @@ static void pcd_start(void)
if (pcd_command(pcd_current, rd_cmd, 2048, "read block")) {
pcd_bufblk = -1;
- next_request(0);
+ next_request(-EIO);
return;
}
@@ -796,7 +796,7 @@ static void do_pcd_read(void)
pcd_retries = 0;
pcd_transfer();
if (!pcd_count) {
- next_request(1);
+ next_request(0);
return;
}
@@ -815,7 +815,7 @@ static void do_pcd_read_drq(void)
return;
}
pcd_bufblk = -1;
- next_request(0);
+ next_request(-EIO);
return;
}
diff --git a/drivers/block/paride/pd.c b/drivers/block/paride/pd.c
index 9299455..0732df4 100644
--- a/drivers/block/paride/pd.c
+++ b/drivers/block/paride/pd.c
@@ -410,7 +410,8 @@ static void run_fsm(void)
pd_claimed = 0;
phase = NULL;
spin_lock_irqsave(&pd_lock, saved_flags);
- end_request(pd_req, res);
+ __blk_end_request_cur(pd_req,
+ res == Ok ? 0 : -EIO);
pd_req = elv_next_request(pd_queue);
if (!pd_req)
stop = 1;
@@ -477,7 +478,7 @@ static int pd_next_buf(void)
if (pd_count)
return 0;
spin_lock_irqsave(&pd_lock, saved_flags);
- end_request(pd_req, 1);
+ __blk_end_request_cur(pd_req, 0);
pd_count = pd_req->current_nr_sectors;
pd_buf = pd_req->buffer;
spin_unlock_irqrestore(&pd_lock, saved_flags);
diff --git a/drivers/block/paride/pf.c b/drivers/block/paride/pf.c
index bef3b99..3871e35 100644
--- a/drivers/block/paride/pf.c
+++ b/drivers/block/paride/pf.c
@@ -750,10 +750,10 @@ static int pf_ready(void)
static struct request_queue *pf_queue;
-static void pf_end_request(int uptodate)
+static void pf_end_request(int err)
{
if (pf_req) {
- end_request(pf_req, uptodate);
+ __blk_end_request_cur(pf_req, err);
pf_req = NULL;
}
}
@@ -773,7 +773,7 @@ repeat:
pf_count = pf_req->current_nr_sectors;
if (pf_block + pf_count > get_capacity(pf_req->rq_disk)) {
- pf_end_request(0);
+ pf_end_request(-EIO);
goto repeat;
}
@@ -788,7 +788,7 @@ repeat:
pi_do_claimed(pf_current->pi, do_pf_write);
else {
pf_busy = 0;
- pf_end_request(0);
+ pf_end_request(-EIO);
goto repeat;
}
}
@@ -805,7 +805,7 @@ static int pf_next_buf(void)
return 1;
if (!pf_count) {
spin_lock_irqsave(&pf_spin_lock, saved_flags);
- pf_end_request(1);
+ pf_end_request(0);
pf_req = elv_next_request(pf_queue);
spin_unlock_irqrestore(&pf_spin_lock, saved_flags);
if (!pf_req)
@@ -816,12 +816,12 @@ static int pf_next_buf(void)
return 0;
}
-static inline void next_request(int success)
+static inline void next_request(int err)
{
unsigned long saved_flags;
spin_lock_irqsave(&pf_spin_lock, saved_flags);
- pf_end_request(success);
+ pf_end_request(err);
pf_busy = 0;
do_pf_request(pf_queue);
spin_unlock_irqrestore(&pf_spin_lock, saved_flags);
@@ -844,7 +844,7 @@ static void do_pf_read_start(void)
pi_do_claimed(pf_current->pi, do_pf_read_start);
return;
}
- next_request(0);
+ next_request(-EIO);
return;
}
pf_mask = STAT_DRQ;
@@ -863,7 +863,7 @@ static void do_pf_read_drq(void)
pi_do_claimed(pf_current->pi, do_pf_read_start);
return;
}
- next_request(0);
+ next_request(-EIO);
return;
}
pi_read_block(pf_current->pi, pf_buf, 512);
@@ -871,7 +871,7 @@ static void do_pf_read_drq(void)
break;
}
pi_disconnect(pf_current->pi);
- next_request(1);
+ next_request(0);
}
static void do_pf_write(void)
@@ -890,7 +890,7 @@ static void do_pf_write_start(void)
pi_do_claimed(pf_current->pi, do_pf_write_start);
return;
}
- next_request(0);
+ next_request(-EIO);
return;
}
@@ -903,7 +903,7 @@ static void do_pf_write_start(void)
pi_do_claimed(pf_current->pi, do_pf_write_start);
return;
}
- next_request(0);
+ next_request(-EIO);
return;
}
pi_write_block(pf_current->pi, pf_buf, 512);
@@ -923,11 +923,11 @@ static void do_pf_write_done(void)
pi_do_claimed(pf_current->pi, do_pf_write_start);
return;
}
- next_request(0);
+ next_request(-EIO);
return;
}
pi_disconnect(pf_current->pi);
- next_request(1);
+ next_request(0);
}
static int __init pf_init(void)
diff --git a/drivers/block/ps3disk.c b/drivers/block/ps3disk.c
index bccc42b..d23b54b 100644
--- a/drivers/block/ps3disk.c
+++ b/drivers/block/ps3disk.c
@@ -158,7 +158,7 @@ static int ps3disk_submit_request_sg(struct ps3_storage_device *dev,
if (res) {
dev_err(&dev->sbd.core, "%s:%u: %s failed %d\n", __func__,
__LINE__, op, res);
- end_request(req, 0);
+ __blk_end_request_cur(req, -EIO);
return 0;
}
@@ -180,7 +180,7 @@ static int ps3disk_submit_flush_request(struct ps3_storage_device *dev,
if (res) {
dev_err(&dev->sbd.core, "%s:%u: sync cache failed 0x%llx\n",
__func__, __LINE__, res);
- end_request(req, 0);
+ __blk_end_request_cur(req, -EIO);
return 0;
}
@@ -205,7 +205,7 @@ static void ps3disk_do_request(struct ps3_storage_device *dev,
break;
} else {
blk_dump_rq_flags(req, DEVICE_NAME " bad request");
- end_request(req, 0);
+ __blk_end_request_cur(req, -EIO);
continue;
}
}
diff --git a/drivers/block/swim.c b/drivers/block/swim.c
index d22cc38..6544a7b 100644
--- a/drivers/block/swim.c
+++ b/drivers/block/swim.c
@@ -532,39 +532,39 @@ static void redo_fd_request(struct request_queue *q)
fs = req->rq_disk->private_data;
if (req->sector < 0 || req->sector >= fs->total_secs) {
- end_request(req, 0);
+ __blk_end_request_cur(req, -EIO);
continue;
}
if (req->current_nr_sectors == 0) {
- end_request(req, 1);
+ __blk_end_request_cur(req, 0);
continue;
}
if (!fs->disk_in) {
- end_request(req, 0);
+ __blk_end_request_cur(req, -EIO);
continue;
}
if (rq_data_dir(req) == WRITE) {
if (fs->write_protected) {
- end_request(req, 0);
+ __blk_end_request_cur(req, -EIO);
continue;
}
}
switch (rq_data_dir(req)) {
case WRITE:
/* NOT IMPLEMENTED */
- end_request(req, 0);
+ __blk_end_request_cur(req, -EIO);
break;
case READ:
if (floppy_read_sectors(fs, req->sector,
req->current_nr_sectors,
req->buffer)) {
- end_request(req, 0);
+ __blk_end_request_cur(req, -EIO);
continue;
}
req->nr_sectors -= req->current_nr_sectors;
req->sector += req->current_nr_sectors;
req->buffer += req->current_nr_sectors * 512;
- end_request(req, 1);
+ __blk_end_request_cur(req, 0);
break;
}
}
diff --git a/drivers/block/swim3.c b/drivers/block/swim3.c
index 6129653..5904f7b 100644
--- a/drivers/block/swim3.c
+++ b/drivers/block/swim3.c
@@ -320,15 +320,15 @@ static void start_request(struct floppy_state *fs)
#endif
if (req->sector < 0 || req->sector >= fs->total_secs) {
- end_request(req, 0);
+ __blk_end_request_cur(req, -EIO);
continue;
}
if (req->current_nr_sectors == 0) {
- end_request(req, 1);
+ __blk_end_request_cur(req, 0);
continue;
}
if (fs->ejected) {
- end_request(req, 0);
+ __blk_end_request_cur(req, -EIO);
continue;
}
@@ -336,7 +336,7 @@ static void start_request(struct floppy_state *fs)
if (fs->write_prot < 0)
fs->write_prot = swim3_readbit(fs, WRITE_PROT);
if (fs->write_prot) {
- end_request(req, 0);
+ __blk_end_request_cur(req, -EIO);
continue;
}
}
@@ -508,7 +508,7 @@ static void act(struct floppy_state *fs)
case do_transfer:
if (fs->cur_cyl != fs->req_cyl) {
if (fs->retries > 5) {
- end_request(fd_req, 0);
+ __blk_end_request_cur(fd_req, -EIO);
fs->state = idle;
return;
}
@@ -540,7 +540,7 @@ static void scan_timeout(unsigned long data)
out_8(&sw->intr_enable, 0);
fs->cur_cyl = -1;
if (fs->retries > 5) {
- end_request(fd_req, 0);
+ __blk_end_request_cur(fd_req, -EIO);
fs->state = idle;
start_request(fs);
} else {
@@ -559,7 +559,7 @@ static void seek_timeout(unsigned long data)
out_8(&sw->select, RELAX);
out_8(&sw->intr_enable, 0);
printk(KERN_ERR "swim3: seek timeout\n");
- end_request(fd_req, 0);
+ __blk_end_request_cur(fd_req, -EIO);
fs->state = idle;
start_request(fs);
}
@@ -583,7 +583,7 @@ static void settle_timeout(unsigned long data)
return;
}
printk(KERN_ERR "swim3: seek settle timeout\n");
- end_request(fd_req, 0);
+ __blk_end_request_cur(fd_req, -EIO);
fs->state = idle;
start_request(fs);
}
@@ -615,7 +615,7 @@ static void xfer_timeout(unsigned long data)
fd_req->current_nr_sectors -= s;
printk(KERN_ERR "swim3: timeout %sing sector %ld\n",
(rq_data_dir(fd_req)==WRITE? "writ": "read"), (long)fd_req->sector);
- end_request(fd_req, 0);
+ __blk_end_request_cur(fd_req, -EIO);
fs->state = idle;
start_request(fs);
}
@@ -646,7 +646,7 @@ static irqreturn_t swim3_interrupt(int irq, void *dev_id)
printk(KERN_ERR "swim3: seen sector but cyl=ff?\n");
fs->cur_cyl = -1;
if (fs->retries > 5) {
- end_request(fd_req, 0);
+ __blk_end_request_cur(fd_req, -EIO);
fs->state = idle;
start_request(fs);
} else {
@@ -731,7 +731,7 @@ static irqreturn_t swim3_interrupt(int irq, void *dev_id)
printk("swim3: error %sing block %ld (err=%x)\n",
rq_data_dir(fd_req) == WRITE? "writ": "read",
(long)fd_req->sector, err);
- end_request(fd_req, 0);
+ __blk_end_request_cur(fd_req, -EIO);
fs->state = idle;
}
} else {
@@ -740,7 +740,7 @@ static irqreturn_t swim3_interrupt(int irq, void *dev_id)
printk(KERN_ERR "swim3: fd dma: stat=%x resid=%d\n", stat, resid);
printk(KERN_ERR " state=%d, dir=%x, intr=%x, err=%x\n",
fs->state, rq_data_dir(fd_req), intr, err);
- end_request(fd_req, 0);
+ __blk_end_request_cur(fd_req, -EIO);
fs->state = idle;
start_request(fs);
break;
@@ -749,7 +749,7 @@ static irqreturn_t swim3_interrupt(int irq, void *dev_id)
fd_req->current_nr_sectors -= fs->scount;
fd_req->buffer += fs->scount * 512;
if (fd_req->current_nr_sectors <= 0) {
- end_request(fd_req, 1);
+ __blk_end_request_cur(fd_req, 0);
fs->state = idle;
} else {
fs->req_sector += fs->scount;
diff --git a/drivers/block/xd.c b/drivers/block/xd.c
index 64b496f..6f6ad82 100644
--- a/drivers/block/xd.c
+++ b/drivers/block/xd.c
@@ -314,21 +314,22 @@ static void do_xd_request (struct request_queue * q)
int retry;
if (!blk_fs_request(req)) {
- end_request(req, 0);
+ __blk_end_request_cur(req, -EIO);
continue;
}
if (block + count > get_capacity(req->rq_disk)) {
- end_request(req, 0);
+ __blk_end_request_cur(req, -EIO);
continue;
}
if (rw != READ && rw != WRITE) {
printk("do_xd_request: unknown request\n");
- end_request(req, 0);
+ __blk_end_request_cur(req, -EIO);
continue;
}
for (retry = 0; (retry < XD_RETRIES) && !res; retry++)
res = xd_readwrite(rw, disk, req->buffer, block, count);
- end_request(req, res); /* wrap up, 0 = fail, 1 = success */
+ /* wrap up, 0 = success, -errno = fail */
+ __blk_end_request_cur(req, res);
}
}
@@ -418,7 +419,7 @@ static int xd_readwrite (u_char operation,XD_INFO *p,char *buffer,u_int block,u_
printk("xd%c: %s timeout, recalibrating drive\n",'a'+drive,(operation == READ ? "read" : "write"));
xd_recalibrate(drive);
spin_lock_irq(&xd_lock);
- return (0);
+ return -EIO;
case 2:
if (sense[0] & 0x30) {
printk("xd%c: %s - ",'a'+drive,(operation == READ ? "reading" : "writing"));
@@ -439,7 +440,7 @@ static int xd_readwrite (u_char operation,XD_INFO *p,char *buffer,u_int block,u_
else
printk(" - no valid disk address\n");
spin_lock_irq(&xd_lock);
- return (0);
+ return -EIO;
}
if (xd_dma_buffer)
for (i=0; i < (temp * 0x200); i++)
@@ -448,7 +449,7 @@ static int xd_readwrite (u_char operation,XD_INFO *p,char *buffer,u_int block,u_
count -= temp, buffer += temp * 0x200, block += temp;
}
spin_lock_irq(&xd_lock);
- return (1);
+ return 0;
}
/* xd_recalibrate: recalibrate a given drive and reset controller if necessary */
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index cd6cfe3..b456447 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -302,7 +302,7 @@ static void do_blkif_request(struct request_queue *rq)
while ((req = elv_next_request(rq)) != NULL) {
info = req->rq_disk->private_data;
if (!blk_fs_request(req)) {
- end_request(req, 0);
+ __blk_end_request_cur(req, -EIO);
continue;
}
diff --git a/drivers/block/xsysace.c b/drivers/block/xsysace.c
index 4aecf5d..b1e1d7e 100644
--- a/drivers/block/xsysace.c
+++ b/drivers/block/xsysace.c
@@ -466,7 +466,7 @@ struct request *ace_get_next_request(struct request_queue * q)
while ((req = elv_next_request(q)) != NULL) {
if (blk_fs_request(req))
break;
- end_request(req, 0);
+ __blk_end_request_cur(req, -EIO);
}
return req;
}
@@ -494,7 +494,7 @@ static void ace_fsm_dostate(struct ace_device *ace)
/* Drop all pending requests */
while ((req = elv_next_request(ace->queue)) != NULL)
- end_request(req, 0);
+ __blk_end_request_cur(req, -EIO);
/* Drop back to IDLE state and notify waiters */
ace->fsm_state = ACE_FSM_STATE_IDLE;
diff --git a/drivers/block/z2ram.c b/drivers/block/z2ram.c
index 80754cd..b66ad58 100644
--- a/drivers/block/z2ram.c
+++ b/drivers/block/z2ram.c
@@ -77,7 +77,7 @@ static void do_z2_request(struct request_queue *q)
if (start + len > z2ram_size) {
printk( KERN_ERR DEVICE_NAME ": bad access: block=%lu, count=%u\n",
req->sector, req->current_nr_sectors);
- end_request(req, 0);
+ __blk_end_request_cur(req, -EIO);
continue;
}
while (len) {
@@ -93,7 +93,7 @@ static void do_z2_request(struct request_queue *q)
start += size;
len -= size;
}
- end_request(req, 1);
+ __blk_end_request_cur(req, 0);
}
}
diff --git a/drivers/cdrom/gdrom.c b/drivers/cdrom/gdrom.c
index fee9a9e..cab2b1f 100644
--- a/drivers/cdrom/gdrom.c
+++ b/drivers/cdrom/gdrom.c
@@ -654,17 +654,17 @@ static void gdrom_request(struct request_queue *rq)
while ((req = elv_next_request(rq)) != NULL) {
if (!blk_fs_request(req)) {
printk(KERN_DEBUG "GDROM: Non-fs request ignored\n");
- end_request(req, 0);
+ __blk_end_request_cur(req, -EIO);
}
if (rq_data_dir(req) != READ) {
printk(KERN_NOTICE "GDROM: Read only device -");
printk(" write request ignored\n");
- end_request(req, 0);
+ __blk_end_request_cur(req, -EIO);
}
if (req->nr_sectors)
gdrom_request_handler_dma(req);
else
- end_request(req, 0);
+ __blk_end_request_cur(req, -EIO);
}
}
diff --git a/drivers/message/i2o/i2o_block.c b/drivers/message/i2o/i2o_block.c
index a443e13..221317e 100644
--- a/drivers/message/i2o/i2o_block.c
+++ b/drivers/message/i2o/i2o_block.c
@@ -923,7 +923,7 @@ static void i2o_block_request_fn(struct request_queue *q)
break;
}
} else
- end_request(req, 0);
+ __blk_end_request_cur(req, -EIO);
}
};
diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
index a49a9c8..76c4c8d 100644
--- a/drivers/mtd/mtd_blkdevs.c
+++ b/drivers/mtd/mtd_blkdevs.c
@@ -54,33 +54,33 @@ static int do_blktrans_request(struct mtd_blktrans_ops *tr,
if (req->cmd_type == REQ_TYPE_LINUX_BLOCK &&
req->cmd[0] == REQ_LB_OP_DISCARD)
- return !tr->discard(dev, block, nsect);
+ return tr->discard(dev, block, nsect);
if (!blk_fs_request(req))
- return 0;
+ return -EIO;
if (req->sector + req->current_nr_sectors > get_capacity(req->rq_disk))
- return 0;
+ return -EIO;
switch(rq_data_dir(req)) {
case READ:
for (; nsect > 0; nsect--, block++, buf += tr->blksize)
if (tr->readsect(dev, block, buf))
- return 0;
- return 1;
+ return -EIO;
+ return 0;
case WRITE:
if (!tr->writesect)
- return 0;
+ return -EIO;
for (; nsect > 0; nsect--, block++, buf += tr->blksize)
if (tr->writesect(dev, block, buf))
- return 0;
- return 1;
+ return -EIO;
+ return 0;
default:
printk(KERN_NOTICE "Unknown request %u\n", rq_data_dir(req));
- return 0;
+ return -EIO;
}
}
@@ -96,7 +96,7 @@ static int mtd_blktrans_thread(void *arg)
while (!kthread_should_stop()) {
struct request *req;
struct mtd_blktrans_dev *dev;
- int res = 0;
+ int res;
req = elv_next_request(rq);
@@ -119,7 +119,7 @@ static int mtd_blktrans_thread(void *arg)
spin_lock_irq(rq->queue_lock);
- end_request(req, res);
+ __blk_end_request_cur(req, res);
}
spin_unlock_irq(rq->queue_lock);
diff --git a/drivers/sbus/char/jsflash.c b/drivers/sbus/char/jsflash.c
index a85ad05..0961788 100644
--- a/drivers/sbus/char/jsflash.c
+++ b/drivers/sbus/char/jsflash.c
@@ -192,25 +192,25 @@ static void jsfd_do_request(struct request_queue *q)
size_t len = req->current_nr_sectors << 9;
if ((offset + len) > jdp->dsize) {
- end_request(req, 0);
+ __blk_end_request_cur(req, -EIO);
continue;
}
if (rq_data_dir(req) != READ) {
printk(KERN_ERR "jsfd: write\n");
- end_request(req, 0);
+ __blk_end_request_cur(req, -EIO);
continue;
}
if ((jdp->dbase & 0xff000000) != 0x20000000) {
printk(KERN_ERR "jsfd: bad base %x\n", (int)jdp->dbase);
- end_request(req, 0);
+ __blk_end_request_cur(req, -EIO);
continue;
}
jsfd_read(req->buffer, jdp->dbase + offset, len);
- end_request(req, 1);
+ __blk_end_request_cur(req, 0);
}
}
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 0a5f383..0bb3ed4 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -842,9 +842,8 @@ extern unsigned int blk_rq_cur_bytes(struct request *rq);
* blk_update_request() completes given number of bytes and updates
* the request without completing it.
*
- * blk_end_request() and friends. __blk_end_request() and
- * end_request() must be called with the request queue spinlock
- * acquired.
+ * blk_end_request() and friends. __blk_end_request() must be called
+ * with the request queue spinlock acquired.
*
* Several drivers define their own end_request and call
* blk_end_request() for parts of the original function.
@@ -895,6 +894,19 @@ static inline void blk_end_request_all(struct request *rq, int error)
}
/**
+ * blk_end_request_cur - Helper function to finish the current request chunk.
+ * @rq: the request to finish the current chunk for
+ * @err: %0 for success, < %0 for error
+ *
+ * Description:
+ * Complete the current consecutively mapped chunk from @rq.
+ */
+static inline void blk_end_request_cur(struct request *rq, int error)
+{
+ blk_end_request(rq, error, rq->hard_cur_sectors << 9);
+}
+
+/**
* __blk_end_request - Helper function for drivers to complete the request.
* @rq: the request being processed
* @error: %0 for success, < %0 for error
@@ -930,6 +942,19 @@ static inline void __blk_end_request_all(struct request *rq, int error)
}
/**
+ * __blk_end_request_cur - Helper function to finish the current request chunk.
+ * @rq: the request to finish the current chunk for
+ * @err: %0 for success, < %0 for error
+ *
+ * Description:
+ * Complete the current consecutively mapped chunk from @rq. Must
+ * be called with queue lock held.
+ */
+static inline void __blk_end_request_cur(struct request *rq, int error)
+{
+ __blk_end_request(rq, error, rq->hard_cur_sectors << 9);
+}
+/**
* blk_end_bidi_request - Helper function for drivers to complete bidi request.
* @rq: the bidi request being processed
* @error: %0 for success, < %0 for error
@@ -953,32 +978,6 @@ static inline bool blk_end_bidi_request(struct request *rq, int error,
return __blk_end_io(rq, error, nr_bytes, bidi_bytes, false);
}
-/**
- * end_request - end I/O on the current segment of the request
- * @rq: the request being processed
- * @uptodate: error value or %0/%1 uptodate flag
- *
- * Description:
- * Ends I/O on the current segment of a request. If that is the only
- * remaining segment, the request is also completed and freed.
- *
- * This is a remnant of how older block drivers handled I/O completions.
- * Modern drivers typically end I/O on the full request in one go, unless
- * 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 a request.
- **/
-static inline void end_request(struct request *rq, int uptodate)
-{
- int error = 0;
-
- if (uptodate <= 0)
- error = uptodate ? uptodate : -EIO;
-
- __blk_end_io(rq, error, rq->hard_cur_sectors << 9, 0, true);
-}
-
extern void blk_complete_request(struct request *);
extern void __blk_complete_request(struct request *);
extern void blk_abort_request(struct request *);
--
1.6.0.2
On Wed, Apr 22, 2009 at 01:37:54AM +0900, Tejun Heo wrote:
> Impact: code reorganization
>
> elv_next_request() and elv_dequeue_request() are public block layer
> interface than actual elevator implementation. They mostly deal with
> how requests interact with block layer and low level drivers at the
> beginning of rqeuest processing whereas __elv_next_request() is the
> actual eleveator request fetching interface.
>
> Move the two functions to blk-core.c. This prepares for further
> interface cleanup.
As a follow-up patch you might want to rename them to have a more
descriptive prefix than elv_ and add some kerneldoc comments for them.
> /**
> + * __blk_end_io - Generic end_io function to complete a request.
> * @rq: the request being processed
> * @error: %0 for success, < %0 for error
> * @nr_bytes: number of bytes to complete @rq
> * @bidi_bytes: number of bytes to complete @rq->next_rq
> + * @locked: whether rq->q->queue_lock is held on entry
> *
> * Description:
> * Ends I/O on a number of bytes attached to @rq and @rq->next_rq.
> * If @rq has leftover, sets it up for the next range of segments.
> *
> * Return:
> + * %false - we are done with this request
> + * %true - this request is not freed yet, it still has pending buffers.
> **/
> +bool __blk_end_io(struct request *rq, int error, unsigned int nr_bytes,
> + unsigned int bidi_bytes, bool locked)
> {
> struct request_queue *q = rq->q;
> unsigned long flags = 0UL;
>
> + if (blk_update_request(rq, error, nr_bytes))
> + return true;
>
> + /* Bidi request must be completed as a whole */
> + if (unlikely(blk_bidi_rq(rq)) &&
> + blk_update_request(rq->next_rq, error, bidi_bytes))
> + return true;
>
> add_disk_randomness(rq->rq_disk);
>
> + if (!locked) {
> + spin_lock_irqsave(q->queue_lock, flags);
> + finish_request(rq, error);
> + spin_unlock_irqrestore(q->queue_lock, flags);
> + } else
> + finish_request(rq, error);
>
> + return false;
> }
> +EXPORT_SYMBOL_GPL(__blk_end_io);
That's really ugly code due to the locked argument. Just make a helper
for all code down to add_disk_randomness, and have two different
functions call that and finish_request +/- the locking.
On Wed, Apr 22, 2009 at 01:37:59AM +0900, Tejun Heo wrote:
>
> Signed-off-by: Tejun Heo <[email protected]>
> Cc: Joerg Dorchain <[email protected]>
Acked-by: Joerg Dorchain <[email protected]> (for amiflop.c)
> Signed-off-by: Tejun Heo <[email protected]>
...
> Cc: Laurent Vivier <[email protected]>
...
> drivers/block/swim.c | 14 +++++-----
Acked-by: Laurent Vivier <[email protected]>
Regards,
Laurent
On Wed, 22 Apr 2009, Tejun Heo wrote:
> end_request() has been kept around for backward compatibility;
> however, it's about time for it to go away.
>
> * There aren't too many users left.
>
> * Its use of @updtodate is pretty confusing.
>
> * In some cases, newer code ends up using mixture of end_request() and
> [__]blk_end_request[_all](), which is way too confusing.
>
> So, add [__]blk_end_request_cur() and replace end_request() with it.
> drivers/block/ps3disk.c | 6 ++--
> drivers/block/z2ram.c | 4 +-
Acked-by: Geert Uytterhoeven <[email protected]>
With kind regards,
Geert Uytterhoeven
Software Architect
Techsoft Centre
Technology and Software Centre Europe
The Corporate Village ? Da Vincilaan 7-D1 ? B-1935 Zaventem ? Belgium
Phone: +32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: [email protected]
Internet: http://www.sony-europe.com/
A division of Sony Europe (Belgium) N.V.
VAT BE 0413.825.160 ? RPR Brussels
Fortis ? BIC GEBABEBB ? IBAN BE41293037680010
On Tue, Apr 21 2009, Christoph Hellwig wrote:
> On Wed, Apr 22, 2009 at 01:37:54AM +0900, Tejun Heo wrote:
> > Impact: code reorganization
> >
> > elv_next_request() and elv_dequeue_request() are public block layer
> > interface than actual elevator implementation. They mostly deal with
> > how requests interact with block layer and low level drivers at the
> > beginning of rqeuest processing whereas __elv_next_request() is the
> > actual eleveator request fetching interface.
> >
> > Move the two functions to blk-core.c. This prepares for further
> > interface cleanup.
>
> As a follow-up patch you might want to rename them to have a more
> descriptive prefix than elv_ and add some kerneldoc comments for them.
Hmm dunno, elv_ is fine with me. kerneldoc is always nice, though.
--
Jens Axboe
On Wed, Apr 22 2009, Tejun Heo wrote:
> Hello,
>
> Upon ack, please pull from the following git tree, which pulls in
> linux-2.6-ide#for-next into linux-2.6-block#for-next and applies this
> patchset on top of it.
>
> git://git.kernel.org/pub/scm/linux/kernel/git/tj/misc.git block-cleanup
>
> This is the third take of block-cleanup-patches. Changes from the
> last take[L] are
>
> * All ide changes including L0001-0003 are separated out and merged
> through linux-2.6-ide#for-next and pulled into
> linux-2.6-block#for-next.
>
> * L0015-block-kill-end_request.patch replaced with
> 0012-block-replace-end_request-with-__-blk_end_reques.patch.
>
> * L0016-ubd-simplify-block-request-completion.patch
> L0017-block-clean-up-unnecessary-stuff-from-block-drivers.patch are
> postponed to later patchset.
>
> * 0013-0014 to kill rq->data added.
>
> * Updated to the current #for-next.
>
> 0001-block-merge-blk_invoke_request_fn-into-__blk_run_.patch
> 0002-block-kill-blk_start_queueing.patch
> 0003-block-don-t-set-REQ_NOMERGE-unnecessarily.patch
> 0004-block-cleanup-REQ_SOFTBARRIER-usages.patch
> 0005-block-clean-up-misc-stuff-after-block-layer-timeout.patch
> 0006-block-reorder-request-completion-functions.patch
> 0007-block-reorganize-request-fetching-functions.patch
> 0008-block-kill-blk_end_request_callback.patch
> 0009-block-clean-up-request-completion-API.patch
> 0010-block-move-rq-start_time-initialization-to-blk_rq_.patch
> 0011-block-implement-and-use-__-blk_end_request_all.patch
> 0012-block-replace-end_request-with-__-blk_end_reques.patch
> 0013-block-don-t-abuse-rq-data.patch
> 0014-block-kill-data.patch
>
> 0001-0011 are mostly identical to L0004-0014. L0015 incorrectly
> replaced end_request() with __blk_end_request_all(). 0012 adds
> [__]blk_end_request_cur() and replaces end_request() with it.
>
> With ide changes pulled in, omap is the only user of rq->data, 0013
> makes it use only rq->special and 0014 kills rq->data.
>
> This patchset is on top of
>
> linux-2.6-block#for-next(a95320f3ede3f028837a621715cd352e83ffe7c2)
> + linux-2.6-ide#for-next(2ecf0a57c60dcb588f310d94412118e15c510532)
Looks nice, the only real quibble I have is the one that hch also brings
up. Using a 'locked' argument is always ugly. Otherwise, overall the
patchset is really nice and does good cleanups. Getting rid of ->data is
great as well.
Once we get that settled, pulling into for-2.6.31 will be done.
--
Jens Axboe
On Tue, Apr 21, 2009 at 10:37 AM, Tejun Heo <[email protected]> wrote:
> end_request() has been kept around for backward compatibility;
> however, it's about time for it to go away.
[...]
> ?drivers/block/xsysace.c ? ? ? ? | ? ?4 +-
Acked-by: Grant Likely <[email protected]>
Jens Axboe wrote:
> On Tue, Apr 21 2009, Christoph Hellwig wrote:
>> On Wed, Apr 22, 2009 at 01:37:54AM +0900, Tejun Heo wrote:
>>> Impact: code reorganization
>>>
>>> elv_next_request() and elv_dequeue_request() are public block layer
>>> interface than actual elevator implementation. They mostly deal with
>>> how requests interact with block layer and low level drivers at the
>>> beginning of rqeuest processing whereas __elv_next_request() is the
>>> actual eleveator request fetching interface.
>>>
>>> Move the two functions to blk-core.c. This prepares for further
>>> interface cleanup.
>> As a follow-up patch you might want to rename them to have a more
>> descriptive prefix than elv_ and add some kerneldoc comments for them.
>
> Hmm dunno, elv_ is fine with me. kerneldoc is always nice, though.
>
Both changes are scheduled down the pipeline.
Thanks.
--
tejun
Christoph Hellwig wrote:
>> +EXPORT_SYMBOL_GPL(__blk_end_io);
>
> That's really ugly code due to the locked argument.
Yeap, it's ugly. Thought having one ugly path would be better than
duplicate implementations.
> Just make a helper for all code down to add_disk_randomness, and
> have two different functions call that and finish_request +/- the
> locking.
Alright, will change toward that direction.
Thanks.
--
tejun
Request completion has gone through several changes and became a bit
messy over the time. Clean it up.
1. end_that_request_data() is a thin wrapper around
end_that_request_data_first() which checks whether bio is NULL
before doing anything and handles bidi completion.
blk_update_request() is a thin wrapper around
end_that_request_data() which clears nr_sectors on the last
iteration but doesn't use the bidi completion.
Clean it up by moving the initial bio NULL check and nr_sectors
clearing on the last iteration into end_that_request_data() and
renaming it to blk_update_request(), which makes blk_end_io() the
only user of end_that_request_data(). Collapse
end_that_request_data() into blk_end_io().
2. There are four visible completion variants - blk_end_request(),
__blk_end_request(), blk_end_bidi_request() and end_request().
blk_end_request() and blk_end_bidi_request() uses blk_end_request()
as the backend but __blk_end_request() and end_request() use
separate implementation in __blk_end_request() due to different
locking rules.
blk_end_bidi_request() is identical to blk_end_io(). Collapse
blk_end_io() into blk_end_bidi_request(), separate out request
update into internal helper blk_update_bidi_request() and add
__blk_end_bidi_request(). Redefine [__]blk_end_request() as thin
inline wrappers around [__]blk_end_bidi_request().
3. As the whole request issue/completion usages are about to be
modified and audited, it's a good chance to convert completion
functions return bool which better indicates the intended meaning
of return values.
4. The function name end_that_request_last() is from the days when it
was a public interface and slighly confusing. Give it a proper
internal name - blk_finish_request().
5. Add description explaning that blk_end_bidi_request() can be safely
used for uni requests as suggested by Boaz Harrosh.
The only visible behavior change is from #1. nr_sectors counts are
cleared after the final iteration no matter which function is used to
complete the request. I couldn't find any place where the code
assumes those nr_sectors counters contain the values for the last
segment and this change is good as it makes the API much more
consistent as the end result is now same whether a request is
completed using [__]blk_end_request() alone or in combination with
blk_update_request().
API further cleaned up per Christoph's suggestion.
[ Impact: cleanup, rq->*nr_sectors always updated after req completion ]
Signed-off-by: Tejun Heo <[email protected]>
Reviewed-by: Boaz Harrosh <[email protected]>
Cc: Christoph Hellwig <[email protected]>
---
I killed blk_end_io(), added __blk_end_bidi_request() and made others
wrappers around [__]blk_end_bidi_request().
Thanks.
block/blk-core.c | 230 ++++++++++++++++---------------------------------
include/linux/blkdev.h | 94 +++++++++++++++++---
2 files changed, 159 insertions(+), 165 deletions(-)
Index: block/block/blk-core.c
===================================================================
--- block.orig/block/blk-core.c
+++ block/block/blk-core.c
@@ -1806,25 +1806,35 @@ void elv_dequeue_request(struct request_
}
/**
- * __end_that_request_first - end I/O on a request
- * @req: the request being processed
+ * blk_update_request - Special helper function for request stacking drivers
+ * @rq: the request being processed
* @error: %0 for success, < %0 for error
- * @nr_bytes: number of bytes to complete
+ * @nr_bytes: number of bytes to complete @rq
*
* Description:
- * Ends I/O on a number of bytes attached to @req, and sets it up
- * for the next range of segments (if any) in the cluster.
+ * Ends I/O on a number of bytes attached to @rq, but doesn't complete
+ * the request structure even if @rq doesn't have leftover.
+ * If @rq has leftover, sets it up for the next range of segments.
+ *
+ * This special helper function is only for request stacking drivers
+ * (e.g. request-based dm) so that they can handle partial completion.
+ * Actual device drivers should use blk_end_request instead.
+ *
+ * Passing the result of blk_rq_bytes() as @nr_bytes guarantees
+ * %false return from this function.
*
* Return:
- * %0 - we are done with this request, call end_that_request_last()
- * %1 - still buffers pending for this request
+ * %false - this request doesn't have any more data
+ * %true - this request has more data
**/
-static int __end_that_request_first(struct request *req, int error,
- int nr_bytes)
+bool blk_update_request(struct request *req, int error, unsigned int nr_bytes)
{
int total_bytes, bio_nbytes, next_idx = 0;
struct bio *bio;
+ if (!req->bio)
+ return false;
+
trace_block_rq_complete(req->q, req);
/*
@@ -1901,8 +1911,16 @@ static int __end_that_request_first(stru
/*
* completely done
*/
- if (!req->bio)
- return 0;
+ if (!req->bio) {
+ /*
+ * Reset counters so that the request stacking driver
+ * can find how many bytes remain in the request
+ * later.
+ */
+ req->nr_sectors = req->hard_nr_sectors = 0;
+ req->current_nr_sectors = req->hard_cur_sectors = 0;
+ return false;
+ }
/*
* if the request wasn't completed, update state
@@ -1916,29 +1934,31 @@ static int __end_that_request_first(stru
blk_recalc_rq_sectors(req, total_bytes >> 9);
blk_recalc_rq_segments(req);
- return 1;
+ return true;
}
+EXPORT_SYMBOL_GPL(blk_update_request);
-static int end_that_request_data(struct request *rq, int error,
- unsigned int nr_bytes, unsigned int bidi_bytes)
-{
- if (rq->bio) {
- if (__end_that_request_first(rq, error, nr_bytes))
- return 1;
-
- /* Bidi request must be completed as a whole */
- if (blk_bidi_rq(rq) &&
- __end_that_request_first(rq->next_rq, error, bidi_bytes))
- return 1;
- }
+static bool blk_update_bidi_request(struct request *rq, int error,
+ unsigned int nr_bytes,
+ unsigned int bidi_bytes)
+{
+ if (blk_update_request(rq, error, nr_bytes))
+ return true;
+
+ /* Bidi request must be completed as a whole */
+ if (unlikely(blk_bidi_rq(rq)) &&
+ blk_update_request(rq->next_rq, error, bidi_bytes))
+ return true;
- return 0;
+ add_disk_randomness(rq->rq_disk);
+
+ return false;
}
/*
* queue lock must be held
*/
-static void end_that_request_last(struct request *req, int error)
+static void blk_finish_request(struct request *req, int error)
{
if (blk_rq_tagged(req))
blk_queue_end_tag(req->q, req);
@@ -1964,161 +1984,65 @@ static void end_that_request_last(struct
}
/**
- * blk_end_io - Generic end_io function to complete a request.
- * @rq: the request being processed
- * @error: %0 for success, < %0 for error
- * @nr_bytes: number of bytes to complete @rq
- * @bidi_bytes: number of bytes to complete @rq->next_rq
+ * blk_end_bidi_request - Complete a bidi request
+ * @rq: the request to complete
+ * @error: %0 for success, < %0 for error
+ * @nr_bytes: number of bytes to complete @rq
+ * @bidi_bytes: number of bytes to complete @rq->next_rq
*
* Description:
* Ends I/O on a number of bytes attached to @rq and @rq->next_rq.
- * If @rq has leftover, sets it up for the next range of segments.
+ * Drivers that supports bidi can safely call this member for any
+ * type of request, bidi or uni. In the later case @bidi_bytes is
+ * just ignored.
*
* Return:
- * %0 - we are done with this request
- * %1 - this request is not freed yet, it still has pending buffers.
+ * %false - we are done with this request
+ * %true - still buffers pending for this request
**/
-static int blk_end_io(struct request *rq, int error, unsigned int nr_bytes,
- unsigned int bidi_bytes)
+bool blk_end_bidi_request(struct request *rq, int error,
+ unsigned int nr_bytes, unsigned int bidi_bytes)
{
struct request_queue *q = rq->q;
- unsigned long flags = 0UL;
+ unsigned long flags;
- if (end_that_request_data(rq, error, nr_bytes, bidi_bytes))
- return 1;
-
- add_disk_randomness(rq->rq_disk);
+ if (blk_update_bidi_request(rq, error, nr_bytes, bidi_bytes))
+ return true;
spin_lock_irqsave(q->queue_lock, flags);
- end_that_request_last(rq, error);
+ blk_finish_request(rq, error);
spin_unlock_irqrestore(q->queue_lock, flags);
- return 0;
-}
-
-/**
- * blk_end_request - Helper function for drivers to complete the request.
- * @rq: the request being processed
- * @error: %0 for success, < %0 for error
- * @nr_bytes: number of bytes to complete
- *
- * Description:
- * Ends I/O on a number of bytes attached to @rq.
- * If @rq has leftover, sets it up for the next range of segments.
- *
- * Return:
- * %0 - we are done with this request
- * %1 - still buffers pending for this request
- **/
-int blk_end_request(struct request *rq, int error, unsigned int nr_bytes)
-{
- return blk_end_io(rq, error, nr_bytes, 0);
+ return false;
}
-EXPORT_SYMBOL_GPL(blk_end_request);
-
-/**
- * __blk_end_request - Helper function for drivers to complete the request.
- * @rq: the request being processed
- * @error: %0 for success, < %0 for error
- * @nr_bytes: number of bytes to complete
- *
- * Description:
- * Must be called with queue lock held unlike blk_end_request().
- *
- * Return:
- * %0 - we are done with this request
- * %1 - still buffers pending for this request
- **/
-int __blk_end_request(struct request *rq, int error, unsigned int nr_bytes)
-{
- if (rq->bio && __end_that_request_first(rq, error, nr_bytes))
- return 1;
-
- add_disk_randomness(rq->rq_disk);
-
- end_that_request_last(rq, error);
-
- return 0;
-}
-EXPORT_SYMBOL_GPL(__blk_end_request);
+EXPORT_SYMBOL_GPL(blk_end_bidi_request);
/**
- * blk_end_bidi_request - Helper function for drivers to complete bidi request.
- * @rq: the bidi request being processed
+ * __blk_end_bidi_request - Complete a bidi request with queue lock held
+ * @rq: the request to complete
* @error: %0 for success, < %0 for error
* @nr_bytes: number of bytes to complete @rq
* @bidi_bytes: number of bytes to complete @rq->next_rq
*
* Description:
- * Ends I/O on a number of bytes attached to @rq and @rq->next_rq.
+ * Identical to blk_end_bidi_request() except that queue lock is
+ * assumed to be locked on entry and remains so on return.
*
* Return:
- * %0 - we are done with this request
- * %1 - still buffers pending for this request
- **/
-int blk_end_bidi_request(struct request *rq, int error, unsigned int nr_bytes,
- unsigned int bidi_bytes)
-{
- return blk_end_io(rq, error, nr_bytes, bidi_bytes);
-}
-EXPORT_SYMBOL_GPL(blk_end_bidi_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
- *
- * Description:
- * Ends I/O on the current segment of a request. If that is the only
- * remaining segment, the request is also completed and freed.
- *
- * This is a remnant of how older block drivers handled I/O completions.
- * Modern drivers typically end I/O on the full request in one go, unless
- * 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 a request.
+ * %false - we are done with this request
+ * %true - still buffers pending for this request
**/
-void end_request(struct request *req, int uptodate)
+bool __blk_end_bidi_request(struct request *rq, int error,
+ unsigned int nr_bytes, unsigned int bidi_bytes)
{
- int error = 0;
-
- if (uptodate <= 0)
- error = uptodate ? uptodate : -EIO;
+ if (blk_update_bidi_request(rq, error, nr_bytes, bidi_bytes))
+ return true;
- __blk_end_request(req, error, req->hard_cur_sectors << 9);
-}
-EXPORT_SYMBOL(end_request);
+ blk_finish_request(rq, error);
-/**
- * blk_update_request - Special helper function for request stacking drivers
- * @rq: the request being processed
- * @error: %0 for success, < %0 for error
- * @nr_bytes: number of bytes to complete @rq
- *
- * Description:
- * Ends I/O on a number of bytes attached to @rq, but doesn't complete
- * the request structure even if @rq doesn't have leftover.
- * If @rq has leftover, sets it up for the next range of segments.
- *
- * This special helper function is only for request stacking drivers
- * (e.g. request-based dm) so that they can handle partial completion.
- * Actual device drivers should use blk_end_request instead.
- */
-void blk_update_request(struct request *rq, int error, unsigned int nr_bytes)
-{
- if (!end_that_request_data(rq, error, nr_bytes, 0)) {
- /*
- * These members are not updated in end_that_request_data()
- * when all bios are completed.
- * Update them so that the request stacking driver can find
- * how many bytes remain in the request later.
- */
- rq->nr_sectors = rq->hard_nr_sectors = 0;
- rq->current_nr_sectors = rq->hard_cur_sectors = 0;
- }
+ return false;
}
-EXPORT_SYMBOL_GPL(blk_update_request);
+EXPORT_SYMBOL_GPL(__blk_end_bidi_request);
void blk_rq_bio_prep(struct request_queue *q, struct request *rq,
struct bio *bio)
Index: block/include/linux/blkdev.h
===================================================================
--- block.orig/include/linux/blkdev.h
+++ block/include/linux/blkdev.h
@@ -837,27 +837,97 @@ extern unsigned int blk_rq_bytes(struct
extern unsigned int blk_rq_cur_bytes(struct request *rq);
/*
- * blk_end_request() and friends.
- * __blk_end_request() and end_request() must be called with
- * the request queue spinlock acquired.
+ * Request completion related functions.
+ *
+ * blk_update_request() completes given number of bytes and updates
+ * the request without completing it.
+ *
+ * blk_end_request() and friends. __blk_end_request() and
+ * end_request() must be called with the request queue spinlock
+ * acquired.
*
* Several drivers define their own end_request and call
* blk_end_request() for parts of the original function.
* This prevents code duplication in drivers.
*/
-extern int blk_end_request(struct request *rq, int error,
- unsigned int nr_bytes);
-extern int __blk_end_request(struct request *rq, int error,
- unsigned int nr_bytes);
-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 bool blk_update_request(struct request *rq, int error,
+ unsigned int nr_bytes);
+extern bool blk_end_bidi_request(struct request *rq, int error,
+ unsigned int nr_bytes,
+ unsigned int bidi_bytes);
+extern bool __blk_end_bidi_request(struct request *rq, int error,
+ unsigned int nr_bytes,
+ unsigned int bidi_bytes);
+
+/**
+ * blk_end_request - Helper function for drivers to complete the request.
+ * @rq: the request being processed
+ * @error: %0 for success, < %0 for error
+ * @nr_bytes: number of bytes to complete
+ *
+ * Description:
+ * Ends I/O on a number of bytes attached to @rq.
+ * If @rq has leftover, sets it up for the next range of segments.
+ *
+ * Return:
+ * %false - we are done with this request
+ * %true - still buffers pending for this request
+ **/
+static inline bool blk_end_request(struct request *rq, int error,
+ unsigned int nr_bytes)
+{
+ return blk_end_bidi_request(rq, error, nr_bytes, 0);
+}
+
+/**
+ * __blk_end_request - Helper function for drivers to complete the request.
+ * @rq: the request being processed
+ * @error: %0 for success, < %0 for error
+ * @nr_bytes: number of bytes to complete
+ *
+ * Description:
+ * Must be called with queue lock held unlike blk_end_request().
+ *
+ * Return:
+ * %false - we are done with this request
+ * %true - still buffers pending for this request
+ **/
+static inline bool __blk_end_request(struct request *rq, int error,
+ unsigned int nr_bytes)
+{
+ return __blk_end_bidi_request(rq, error, nr_bytes, 0);
+}
+
+/**
+ * end_request - end I/O on the current segment of the request
+ * @rq: the request being processed
+ * @uptodate: error value or %0/%1 uptodate flag
+ *
+ * Description:
+ * Ends I/O on the current segment of a request. If that is the only
+ * remaining segment, the request is also completed and freed.
+ *
+ * This is a remnant of how older block drivers handled I/O completions.
+ * Modern drivers typically end I/O on the full request in one go, unless
+ * 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 a request.
+ **/
+static inline void end_request(struct request *rq, int uptodate)
+{
+ int error = 0;
+
+ if (uptodate <= 0)
+ error = uptodate ? uptodate : -EIO;
+
+ __blk_end_bidi_request(rq, error, rq->hard_cur_sectors << 9, 0);
+}
+
extern void blk_complete_request(struct request *);
extern void __blk_complete_request(struct request *);
extern void blk_abort_request(struct request *);
extern void blk_abort_queue(struct request_queue *);
-extern void blk_update_request(struct request *rq, int error,
- unsigned int nr_bytes);
/*
* Access functions for manipulating queue properties
Hello,
Jens Axboe wrote:
>> linux-2.6-block#for-next(a95320f3ede3f028837a621715cd352e83ffe7c2)
>> + linux-2.6-ide#for-next(2ecf0a57c60dcb588f310d94412118e15c510532)
>
> Looks nice, the only real quibble I have is the one that hch also brings
> up. Using a 'locked' argument is always ugly. Otherwise, overall the
> patchset is really nice and does good cleanups. Getting rid of ->data is
> great as well.
>
> Once we get that settled, pulling into for-2.6.31 will be done.
Tree updated. In addition to the @locked removal, Acked-by:'s are
added to 0012 and 0013 got a proper arm-omap prefix.
git://git.kernel.org/pub/scm/linux/kernel/git/tj/misc.git block-cleanup
Thanks.
--
tejun
On Thu, Apr 23 2009, Tejun Heo wrote:
> Hello,
>
> Jens Axboe wrote:
> >> linux-2.6-block#for-next(a95320f3ede3f028837a621715cd352e83ffe7c2)
> >> + linux-2.6-ide#for-next(2ecf0a57c60dcb588f310d94412118e15c510532)
> >
> > Looks nice, the only real quibble I have is the one that hch also brings
> > up. Using a 'locked' argument is always ugly. Otherwise, overall the
> > patchset is really nice and does good cleanups. Getting rid of ->data is
> > great as well.
> >
> > Once we get that settled, pulling into for-2.6.31 will be done.
>
> Tree updated. In addition to the @locked removal, Acked-by:'s are
> added to 0012 and 0013 got a proper arm-omap prefix.
>
> git://git.kernel.org/pub/scm/linux/kernel/git/tj/misc.git block-cleanup
Thanks, pulled!
--
Jens Axboe
On 04/23/2009 05:08 AM, Tejun Heo wrote:
> Request completion has gone through several changes and became a bit
> messy over the time. Clean it up.
>
> 1. end_that_request_data() is a thin wrapper around
> end_that_request_data_first() which checks whether bio is NULL
> before doing anything and handles bidi completion.
> blk_update_request() is a thin wrapper around
> end_that_request_data() which clears nr_sectors on the last
> iteration but doesn't use the bidi completion.
>
> Clean it up by moving the initial bio NULL check and nr_sectors
> clearing on the last iteration into end_that_request_data() and
> renaming it to blk_update_request(), which makes blk_end_io() the
> only user of end_that_request_data(). Collapse
> end_that_request_data() into blk_end_io().
>
> 2. There are four visible completion variants - blk_end_request(),
> __blk_end_request(), blk_end_bidi_request() and end_request().
> blk_end_request() and blk_end_bidi_request() uses blk_end_request()
> as the backend but __blk_end_request() and end_request() use
> separate implementation in __blk_end_request() due to different
> locking rules.
>
> blk_end_bidi_request() is identical to blk_end_io(). Collapse
> blk_end_io() into blk_end_bidi_request(), separate out request
> update into internal helper blk_update_bidi_request() and add
> __blk_end_bidi_request(). Redefine [__]blk_end_request() as thin
> inline wrappers around [__]blk_end_bidi_request().
>
> 3. As the whole request issue/completion usages are about to be
> modified and audited, it's a good chance to convert completion
> functions return bool which better indicates the intended meaning
> of return values.
>
> 4. The function name end_that_request_last() is from the days when it
> was a public interface and slighly confusing. Give it a proper
> internal name - blk_finish_request().
>
> 5. Add description explaning that blk_end_bidi_request() can be safely
> used for uni requests as suggested by Boaz Harrosh.
>
> The only visible behavior change is from #1. nr_sectors counts are
> cleared after the final iteration no matter which function is used to
> complete the request. I couldn't find any place where the code
> assumes those nr_sectors counters contain the values for the last
> segment and this change is good as it makes the API much more
> consistent as the end result is now same whether a request is
> completed using [__]blk_end_request() alone or in combination with
> blk_update_request().
>
> API further cleaned up per Christoph's suggestion.
>
Rrrr.
This patch could be nice, but not after I've seen the previous one.
The first one was much^3 nicer.
Maybe all you need to do is push the lock flag into blk_finish_request()
<original patch>
+ if (!locked) {
> + spin_lock_irqsave(q->queue_lock, flags);
> + finish_request(rq, error);
> + spin_unlock_irqrestore(q->queue_lock, flags);
> + } else
> + finish_request(rq, error);
>
</original patch>
Then you have only one call site to finish_request() inside blk_end_io().
finish_request() will become the ugly site, but if looking at the alternative
I think it is worth it. Code is smaller, cleaner, and clearer. (Sometimes principles
must be sacrificed)
At the end, I hate that lock around finish_request(), I wish the req->end_io()
was not called with the lock held and the plain blk_put_request() (locking version)
could be called. Having req->end_io() under lock is a pain in the ass that makes
you go through loops when you need proper error handling. One day I will get rid of
it.
Tejun? now that you done both, which one do you like better? or is it just me?
Thanks
Boaz
Hello, Boaz.
Boaz Harrosh wrote:
> Rrrr.
>
> This patch could be nice, but not after I've seen the previous one.
> The first one was much^3 nicer.
Heh... can't make everyone happy. :-)
> Maybe all you need to do is push the lock flag into blk_finish_request()
>
> <original patch>
> + if (!locked) {
>> + spin_lock_irqsave(q->queue_lock, flags);
>> + finish_request(rq, error);
>> + spin_unlock_irqrestore(q->queue_lock, flags);
>> + } else
>> + finish_request(rq, error);
>>
> </original patch>
>
> Then you have only one call site to finish_request() inside blk_end_io().
>
> finish_request() will become the ugly site, but if looking at the
> alternative I think it is worth it. Code is smaller, cleaner, and
> clearer. (Sometimes principles must be sacrificed)
>
> At the end, I hate that lock around finish_request(), I wish the
> req->end_io() was not called with the lock held and the plain
> blk_put_request() (locking version) could be called. Having
> req->end_io() under lock is a pain in the ass that makes you go
> through loops when you need proper error handling. One day I will
> get rid of it.
>
> Tejun? now that you done both, which one do you like better? or is
> it just me?
Frankly, I don't care one way or the other as long as there's no
duplication in code paths. Pros and cons of both approaches are
minimal and/or cosmetic. One might remove a branch at the cost of
minutely larger code. It just doesn't matter all that much.
I think it's best to let the maintainer have his/her way on things
like this. I mean, maintainership is a hard work. There gotta be
some perks. :-)
Joke aside, I think it's a good idea to follow maintainer's decisions
on things like this as it helps making the subsystem code more
consistent which is usually way more important than minute technical
or cosmetic advantages.
Thanks.
--
tejun