[PATCH] block: add assert_spin_locked() to ensure lock is held

[PATCH] block: add assert_spin_locked() to ensure lock is held

[Namhyung Kim]

Use assert_spin_locked() macro in order to ensure that queue_lock
must be held prior to calling some EXPORTed functions.

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
---
 block/blk-core.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 32a1c12..b849064 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -975,6 +975,8 @@ EXPORT_SYMBOL(blk_make_request);
  */
 void blk_requeue_request(struct request_queue *q, struct request *rq)
 {
+	assert_spin_locked(q->queue_lock);
+
 	blk_delete_timer(rq);
 	blk_clear_rq_complete(rq);
 	trace_block_rq_requeue(q, rq);
@@ -1098,6 +1100,8 @@ EXPORT_SYMBOL_GPL(part_round_stats);
  */
 void __blk_put_request(struct request_queue *q, struct request *req)
 {
+	assert_spin_locked(q->queue_lock);
+
 	if (unlikely(!q))
 		return;
 	if (unlikely(--req->ref_count))
@@ -1729,6 +1733,8 @@ unsigned int blk_rq_err_bytes(const struct request *rq)
 	unsigned int bytes = 0;
 	struct bio *bio;
 
+	assert_spin_locked(rq->q->queue_lock);
+
 	if (!(rq->cmd_flags & REQ_MIXED_MERGE))
 		return blk_rq_bytes(rq);
 
@@ -1811,6 +1817,8 @@ struct request *blk_peek_request(struct request_queue *q)
 	struct request *rq;
 	int ret;
 
+	assert_spin_locked(q->queue_lock);
+
 	while ((rq = __elv_next_request(q)) != NULL) {
 		if (!(rq->cmd_flags & REQ_STARTED)) {
 			/*
@@ -1926,6 +1934,8 @@ void blk_dequeue_request(struct request *rq)
  */
 void blk_start_request(struct request *req)
 {
+	assert_spin_locked(req->q->queue_lock);
+
 	blk_dequeue_request(req);
 
 	/*
@@ -1959,6 +1969,8 @@ struct request *blk_fetch_request(struct request_queue *q)
 {
 	struct request *rq;
 
+	assert_spin_locked(q->queue_lock);
+
 	rq = blk_peek_request(q);
 	if (rq)
 		blk_start_request(rq);
-- 
1.7.2.2

Re: [PATCH] block: add assert_spin_locked() to ensure lock is held

[Jens Axboe]

On 2010-09-15 14:33, Namhyung Kim wrote:
> Use assert_spin_locked() macro in order to ensure that queue_lock
> must be held prior to calling some EXPORTed functions.

As far as I know, no such bug has ever occurred that I know
of. So while I don't mind adding such instrumentation, there's
little point to doing it when you are not seeing any usability
problems in there. And all these paths (requeue less) are
heavily used, so problems would appear quickly.

Locking in the block layer is still fairly trivial, we just
have the one lock per queue.

-- 
Jens Axboe

Re: [PATCH] block: add assert_spin_locked() to ensure lock is held

[Namhyung Kim]

On Wed, Sep 15, 2010 at 21:48, Jens Axboe <axboe@kernel.dk> wrote:
> As far as I know, no such bug has ever occurred that I know
> of. So while I don't mind adding such instrumentation, there's
> little point to doing it when you are not seeing any usability
> problems in there. And all these paths (requeue less) are
> heavily used, so problems would appear quickly.
>

I didn't see any problems in there. I just thought
if a code has such a restriction, generally it is better
to have an explicit assertion in the code also
not only in comments. Since those functions are
EXPORTed ones I thought there will be a possibility
of misusing them, although they could be found
quickly without this facility. :-)


-- 
Regards,
Namhyung Kim