From: Tejun Heo <tj@kernel.org>
To: axboe@kernel.dk, linux-kernel@vger.kernel.org, vgoyal@redhat.com
Cc: ctalbott@google.com, ni@google.com, Tejun Heo <tj@kernel.org>
Subject: [PATCH 09/10] block: drop @tsk from attempt_plug_merge() and explain sync rules
Date: Tue, 18 Oct 2011 21:26:23 -0700 [thread overview]
Message-ID: <1318998384-22525-10-git-send-email-tj@kernel.org> (raw)
In-Reply-To: <1318998384-22525-1-git-send-email-tj@kernel.org>
attempt_plug_merge() accesses elevator without holding queue_lock and
may call into ->elevator_bio_merge_fn(). The elvator is guaranteed to
be valid because it's accessed iff the plugged list has requests and
elevator is never exited with live requests, so as long as the
elevator method can deal with unlocked access, this is safe.
Explain the sync rules around attempt_plug_merge() and drop the
unnecessary @tsk parameter.
This patch doesn't introduce any functional change.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Jens Axboe <axboe@kernel.dk>
---
block/blk-core.c | 28 +++++++++++++++++++++-------
include/linux/elevator.h | 6 ++++++
2 files changed, 27 insertions(+), 7 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index fc10f53..2425989 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1204,18 +1204,32 @@ static bool bio_attempt_front_merge(struct request_queue *q,
return true;
}
-/*
- * Attempts to merge with the plugged list in the current process. Returns
- * true if merge was successful, otherwise false.
+/**
+ * attempt_plug_merge - try to merge with %current's plugged list
+ * @q: request_queue new bio is being queued at
+ * @bio: new bio being queued
+ * @request_count: out parameter for number of traversed plugged requests
+ *
+ * Determine whether @bio being queued on @q can be merged with a request
+ * on %current's plugged list. Returns %true if merge was successful,
+ * otherwise %false.
+ *
+ * This function is called without @q->queue_lock; however, elevator is
+ * accessed iff there already are requests on the plugged list which in
+ * turn guarantees validity of the elevator.
+ *
+ * Note that, on successful merge, elevator operation
+ * elevator_bio_merged_fn() will be called without queue lock. Elevator
+ * must be ready for this.
*/
-static bool attempt_plug_merge(struct task_struct *tsk, struct request_queue *q,
- struct bio *bio, unsigned int *request_count)
+static bool attempt_plug_merge(struct request_queue *q, struct bio *bio,
+ unsigned int *request_count)
{
struct blk_plug *plug;
struct request *rq;
bool ret = false;
- plug = tsk->plug;
+ plug = current->plug;
if (!plug)
goto out;
*request_count = 0;
@@ -1283,7 +1297,7 @@ void blk_queue_bio(struct request_queue *q, struct bio *bio)
* Check if we can merge with the plugged list before grabbing
* any locks.
*/
- if (attempt_plug_merge(current, q, bio, &request_count))
+ if (attempt_plug_merge(q, bio, &request_count))
return;
spin_lock_irq(q->queue_lock);
diff --git a/include/linux/elevator.h b/include/linux/elevator.h
index d800d51..1d0f7a2 100644
--- a/include/linux/elevator.h
+++ b/include/linux/elevator.h
@@ -38,6 +38,12 @@ struct elevator_ops
elevator_merged_fn *elevator_merged_fn;
elevator_merge_req_fn *elevator_merge_req_fn;
elevator_allow_merge_fn *elevator_allow_merge_fn;
+
+ /*
+ * Used for both plugged list and elevator merging and in the
+ * former case called without queue_lock. Read comment on top of
+ * attempt_plug_merge() for details.
+ */
elevator_bio_merged_fn *elevator_bio_merged_fn;
elevator_dispatch_fn *elevator_dispatch_fn;
--
1.7.3.1
next prev parent reply other threads:[~2011-10-19 4:26 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-10-19 4:26 [PATCHSET block/for-next] fix request_queue life-cycle management Tejun Heo
2011-10-19 4:26 ` [PATCH 01/10] block: make gendisk hold a reference to its queue Tejun Heo
2011-10-19 4:26 ` [PATCH 02/10] block: fix genhd refcounting in blkio_policy_parse_and_set() Tejun Heo
2011-10-19 13:26 ` Vivek Goyal
2011-10-19 16:29 ` Tejun Heo
2011-10-19 16:59 ` Vivek Goyal
2011-10-19 22:05 ` Tejun Heo
2011-10-19 22:07 ` Tejun Heo
2011-10-19 23:51 ` Tejun Heo
2011-10-20 13:41 ` Vivek Goyal
2011-10-20 16:11 ` Tejun Heo
2011-10-20 16:16 ` Kay Sievers
2011-10-20 17:50 ` Vivek Goyal
2011-10-20 17:47 ` Vivek Goyal
2011-10-19 4:26 ` [PATCH 03/10] block: move blk_throtl prototypes to block/blk.h Tejun Heo
2011-10-19 13:33 ` Vivek Goyal
2011-10-19 4:26 ` [PATCH 04/10] block: pass around REQ_* flags instead of broken down booleans during request alloc/free Tejun Heo
2011-10-19 13:44 ` Vivek Goyal
2011-10-19 16:31 ` Tejun Heo
2011-10-19 4:26 ` [PATCH 05/10] block: drop unnecessary blk_get/put_queue() in scsi_cmd_ioctl() and blk_get_tg() Tejun Heo
2011-10-19 13:52 ` Vivek Goyal
2011-10-19 16:35 ` Tejun Heo
2011-10-19 4:26 ` [PATCH 06/10] block: reorganize queue draining Tejun Heo
2011-10-19 4:26 ` [PATCH 07/10] block: reorganize throtl_get_tg() and blk_throtl_bio() Tejun Heo
2011-10-19 14:56 ` Vivek Goyal
2011-10-19 17:06 ` Tejun Heo
2011-10-19 17:19 ` Vivek Goyal
2011-10-19 17:30 ` Tejun Heo
2011-10-19 17:45 ` Vivek Goyal
2011-10-19 17:49 ` Tejun Heo
2011-10-19 4:26 ` [PATCH 08/10] block: make get_request[_wait]() fail if queue is dead Tejun Heo
2011-10-19 15:22 ` Vivek Goyal
2011-10-19 4:26 ` Tejun Heo [this message]
2011-10-19 4:26 ` [PATCH 10/10] block: fix request_queue lifetime handling by making blk_queue_cleanup() proper shutdown Tejun Heo
2011-10-19 12:43 ` Jens Axboe
2011-10-19 17:13 ` Tejun Heo
2011-10-19 18:04 ` Jens Axboe
2011-10-19 16:18 ` Vivek Goyal
2011-10-19 17:12 ` Tejun Heo
2011-10-19 17:29 ` Vivek Goyal
2011-10-19 17:33 ` Tejun Heo
2011-10-19 4:29 ` [PATCHSET block/for-next] fix request_queue life-cycle management Tejun Heo
2011-10-19 12:44 ` Jens Axboe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1318998384-22525-10-git-send-email-tj@kernel.org \
--to=tj@kernel.org \
--cc=axboe@kernel.dk \
--cc=ctalbott@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=ni@google.com \
--cc=vgoyal@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).