From: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
To: axboe-tSWWG44O7X1aa/9Udqfwiw@public.gmane.org
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
vgoyal-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
avanzini.arianna-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Subject: [PATCH 7/8] cfq-iosched: fold cfq_find_alloc_queue() into cfq_get_queue()
Date: Mon, 8 Jun 2015 17:59:32 +0900 [thread overview]
Message-ID: <1433753973-23684-8-git-send-email-tj@kernel.org> (raw)
In-Reply-To: <1433753973-23684-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
cfq_find_alloc_queue() checks whether a queue actually needs to be
allocated, which is unnecessary as its sole caller, cfq_get_queue(),
only calls it if so. Also, the oom queue fallback logic is scattered
between cfq_get_queue() and cfq_find_alloc_queue(). There really
isn't much going on in the latter and things can be made simpler by
folding it into cfq_get_queue().
This patch collapses cfq_find_alloc_queue() into cfq_get_queue(). The
change is fairly straight-forward with one exception - async_cfqq is
now initialized to NULL and the "!is_sync" test in the last if
conditional is replaced with "async_cfqq" test. This is because gcc
(5.1.1) gets confused for some reason and warns that async_cfqq may be
used uninitialized otherwise. Oh well, the code isn't necessarily
worse this way.
This patch doesn't cause any functional difference.
Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Vivek Goyal <vgoyal-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: Arianna Avanzini <avanzini.arianna-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
block/cfq-iosched.c | 49 +++++++++++++++----------------------------------
1 file changed, 15 insertions(+), 34 deletions(-)
diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
index a775128..393befb 100644
--- a/block/cfq-iosched.c
+++ b/block/cfq-iosched.c
@@ -3572,33 +3572,6 @@ static void check_blkcg_changed(struct cfq_io_cq *cic, struct bio *bio)
static inline void check_blkcg_changed(struct cfq_io_cq *cic, struct bio *bio) { }
#endif /* CONFIG_CFQ_GROUP_IOSCHED */
-static struct cfq_queue *
-cfq_find_alloc_queue(struct cfq_data *cfqd, struct cfq_group *cfqg, bool is_sync,
- struct cfq_io_cq *cic, struct bio *bio)
-{
- struct cfq_queue *cfqq;
-
- cfqq = cic_to_cfqq(cic, is_sync);
-
- /*
- * Always try a new alloc if we fell back to the OOM cfqq
- * originally, since it should just be a temporary situation.
- */
- if (!cfqq || cfqq == &cfqd->oom_cfqq) {
- cfqq = kmem_cache_alloc_node(cfq_pool,
- GFP_ATOMIC | __GFP_ZERO,
- cfqd->queue->node);
- if (cfqq) {
- cfq_init_cfqq(cfqd, cfqq, current->pid, is_sync);
- cfq_init_prio_data(cfqq, cic);
- cfq_link_cfqq_cfqg(cfqq, cfqg);
- cfq_log_cfqq(cfqd, cfqq, "alloced");
- } else
- cfqq = &cfqd->oom_cfqq;
- }
- return cfqq;
-}
-
static struct cfq_queue **
cfq_async_queue_prio(struct cfq_data *cfqd, int ioprio_class, int ioprio)
{
@@ -3623,7 +3596,7 @@ cfq_get_queue(struct cfq_data *cfqd, bool is_sync, struct cfq_io_cq *cic,
{
int ioprio_class = IOPRIO_PRIO_CLASS(cic->ioprio);
int ioprio = IOPRIO_PRIO_DATA(cic->ioprio);
- struct cfq_queue **async_cfqq;
+ struct cfq_queue **async_cfqq = NULL;
struct cfq_queue *cfqq;
struct cfq_group *cfqg;
@@ -3646,12 +3619,20 @@ cfq_get_queue(struct cfq_data *cfqd, bool is_sync, struct cfq_io_cq *cic,
goto out;
}
- cfqq = cfq_find_alloc_queue(cfqd, cfqg, is_sync, cic, bio);
+ cfqq = kmem_cache_alloc_node(cfq_pool, GFP_ATOMIC | __GFP_ZERO,
+ cfqd->queue->node);
+ if (!cfqq) {
+ cfqq = &cfqd->oom_cfqq;
+ goto out;
+ }
+
+ cfq_init_cfqq(cfqd, cfqq, current->pid, is_sync);
+ cfq_init_prio_data(cfqq, cic);
+ cfq_link_cfqq_cfqg(cfqq, cfqg);
+ cfq_log_cfqq(cfqd, cfqq, "alloced");
- /*
- * pin the queue now that it's allocated, scheduler exit will prune it
- */
- if (!is_sync && cfqq != &cfqd->oom_cfqq) {
+ if (async_cfqq) {
+ /* a new async queue is created, pin and remember */
cfqq->ref++;
*async_cfqq = cfqq;
}
@@ -4401,7 +4382,7 @@ static int cfq_init_queue(struct request_queue *q, struct elevator_type *e)
cfqd->prio_trees[i] = RB_ROOT;
/*
- * Our fallback cfqq if cfq_find_alloc_queue() runs into OOM issues.
+ * Our fallback cfqq if cfq_get_queue() runs into OOM issues.
* Grab a permanent reference to it, so that the normal code flow
* will not attempt to free it. oom_cfqq is linked to root_group
* but shouldn't hold a reference as it'll never be unlinked. Lose
--
2.4.2
WARNING: multiple messages have this Message-ID (diff)
From: Tejun Heo <tj@kernel.org>
To: axboe@kernel.dk
Cc: linux-kernel@vger.kernel.org, cgroups@vger.kernel.org,
vgoyal@redhat.com, avanzini.arianna@gmail.com,
Tejun Heo <tj@kernel.org>
Subject: [PATCH 7/8] cfq-iosched: fold cfq_find_alloc_queue() into cfq_get_queue()
Date: Mon, 8 Jun 2015 17:59:32 +0900 [thread overview]
Message-ID: <1433753973-23684-8-git-send-email-tj@kernel.org> (raw)
In-Reply-To: <1433753973-23684-1-git-send-email-tj@kernel.org>
cfq_find_alloc_queue() checks whether a queue actually needs to be
allocated, which is unnecessary as its sole caller, cfq_get_queue(),
only calls it if so. Also, the oom queue fallback logic is scattered
between cfq_get_queue() and cfq_find_alloc_queue(). There really
isn't much going on in the latter and things can be made simpler by
folding it into cfq_get_queue().
This patch collapses cfq_find_alloc_queue() into cfq_get_queue(). The
change is fairly straight-forward with one exception - async_cfqq is
now initialized to NULL and the "!is_sync" test in the last if
conditional is replaced with "async_cfqq" test. This is because gcc
(5.1.1) gets confused for some reason and warns that async_cfqq may be
used uninitialized otherwise. Oh well, the code isn't necessarily
worse this way.
This patch doesn't cause any functional difference.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Vivek Goyal <vgoyal@redhat.com>
Cc: Arianna Avanzini <avanzini.arianna@gmail.com>
---
block/cfq-iosched.c | 49 +++++++++++++++----------------------------------
1 file changed, 15 insertions(+), 34 deletions(-)
diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
index a775128..393befb 100644
--- a/block/cfq-iosched.c
+++ b/block/cfq-iosched.c
@@ -3572,33 +3572,6 @@ static void check_blkcg_changed(struct cfq_io_cq *cic, struct bio *bio)
static inline void check_blkcg_changed(struct cfq_io_cq *cic, struct bio *bio) { }
#endif /* CONFIG_CFQ_GROUP_IOSCHED */
-static struct cfq_queue *
-cfq_find_alloc_queue(struct cfq_data *cfqd, struct cfq_group *cfqg, bool is_sync,
- struct cfq_io_cq *cic, struct bio *bio)
-{
- struct cfq_queue *cfqq;
-
- cfqq = cic_to_cfqq(cic, is_sync);
-
- /*
- * Always try a new alloc if we fell back to the OOM cfqq
- * originally, since it should just be a temporary situation.
- */
- if (!cfqq || cfqq == &cfqd->oom_cfqq) {
- cfqq = kmem_cache_alloc_node(cfq_pool,
- GFP_ATOMIC | __GFP_ZERO,
- cfqd->queue->node);
- if (cfqq) {
- cfq_init_cfqq(cfqd, cfqq, current->pid, is_sync);
- cfq_init_prio_data(cfqq, cic);
- cfq_link_cfqq_cfqg(cfqq, cfqg);
- cfq_log_cfqq(cfqd, cfqq, "alloced");
- } else
- cfqq = &cfqd->oom_cfqq;
- }
- return cfqq;
-}
-
static struct cfq_queue **
cfq_async_queue_prio(struct cfq_data *cfqd, int ioprio_class, int ioprio)
{
@@ -3623,7 +3596,7 @@ cfq_get_queue(struct cfq_data *cfqd, bool is_sync, struct cfq_io_cq *cic,
{
int ioprio_class = IOPRIO_PRIO_CLASS(cic->ioprio);
int ioprio = IOPRIO_PRIO_DATA(cic->ioprio);
- struct cfq_queue **async_cfqq;
+ struct cfq_queue **async_cfqq = NULL;
struct cfq_queue *cfqq;
struct cfq_group *cfqg;
@@ -3646,12 +3619,20 @@ cfq_get_queue(struct cfq_data *cfqd, bool is_sync, struct cfq_io_cq *cic,
goto out;
}
- cfqq = cfq_find_alloc_queue(cfqd, cfqg, is_sync, cic, bio);
+ cfqq = kmem_cache_alloc_node(cfq_pool, GFP_ATOMIC | __GFP_ZERO,
+ cfqd->queue->node);
+ if (!cfqq) {
+ cfqq = &cfqd->oom_cfqq;
+ goto out;
+ }
+
+ cfq_init_cfqq(cfqd, cfqq, current->pid, is_sync);
+ cfq_init_prio_data(cfqq, cic);
+ cfq_link_cfqq_cfqg(cfqq, cfqg);
+ cfq_log_cfqq(cfqd, cfqq, "alloced");
- /*
- * pin the queue now that it's allocated, scheduler exit will prune it
- */
- if (!is_sync && cfqq != &cfqd->oom_cfqq) {
+ if (async_cfqq) {
+ /* a new async queue is created, pin and remember */
cfqq->ref++;
*async_cfqq = cfqq;
}
@@ -4401,7 +4382,7 @@ static int cfq_init_queue(struct request_queue *q, struct elevator_type *e)
cfqd->prio_trees[i] = RB_ROOT;
/*
- * Our fallback cfqq if cfq_find_alloc_queue() runs into OOM issues.
+ * Our fallback cfqq if cfq_get_queue() runs into OOM issues.
* Grab a permanent reference to it, so that the normal code flow
* will not attempt to free it. oom_cfqq is linked to root_group
* but shouldn't hold a reference as it'll never be unlinked. Lose
--
2.4.2
next prev parent reply other threads:[~2015-06-08 8:59 UTC|newest]
Thread overview: 58+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-08 8:59 [PATCHSET block/for-4.2/writeback] block, cgroup: make cfq charge async IOs to the appropriate blkcgs Tejun Heo
2015-06-08 8:59 ` Tejun Heo
[not found] ` <1433753973-23684-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2015-06-08 8:59 ` [PATCH 1/8] cfq-iosched: simplify control flow in cfq_get_queue() Tejun Heo
2015-06-08 8:59 ` Tejun Heo
[not found] ` <1433753973-23684-2-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2015-06-08 18:36 ` Jeff Moyer
2015-06-08 18:36 ` Jeff Moyer
2015-06-08 8:59 ` [PATCH 2/8] cfq-iosched: fix async oom queue handling Tejun Heo
2015-06-08 8:59 ` Tejun Heo
[not found] ` <1433753973-23684-3-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2015-06-08 18:42 ` Jeff Moyer
2015-06-08 18:42 ` Jeff Moyer
2015-06-08 8:59 ` [PATCH 3/8] cfq-iosched: fix oom cfq_queue ref leak in cfq_set_request() Tejun Heo
2015-06-08 8:59 ` Tejun Heo
[not found] ` <1433753973-23684-4-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2015-06-08 18:51 ` Jeff Moyer
2015-06-08 18:51 ` Jeff Moyer
2015-06-08 8:59 ` [PATCH 4/8] cfq-iosched: minor cleanups Tejun Heo
2015-06-08 8:59 ` Tejun Heo
[not found] ` <1433753973-23684-5-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2015-06-08 18:59 ` Jeff Moyer
2015-06-08 18:59 ` Jeff Moyer
2015-06-08 8:59 ` [PATCH 5/8] cfq-iosched: remove @gfp_mask from cfq_find_alloc_queue() Tejun Heo
2015-06-08 8:59 ` Tejun Heo
[not found] ` <1433753973-23684-6-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2015-06-08 19:24 ` Jeff Moyer
2015-06-08 19:24 ` Jeff Moyer
2015-06-08 20:27 ` Jeff Moyer
[not found] ` <x49sia2gd41.fsf-RRHT56Q3PSP4kTEheFKJxxDDeQx5vsVwAInAS/Ez/D0@public.gmane.org>
2015-06-08 21:19 ` Vivek Goyal
2015-06-08 21:19 ` Vivek Goyal
[not found] ` <20150608211930.GA20918-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2015-06-09 3:01 ` Tejun Heo
2015-06-09 3:01 ` Tejun Heo
2015-06-09 3:00 ` Tejun Heo
2015-06-09 3:00 ` Tejun Heo
[not found] ` <20150609030054.GJ21465-qYNAdHglDFBN0TnZuCh8vA@public.gmane.org>
2015-06-09 14:29 ` Jeff Moyer
2015-06-09 14:29 ` Jeff Moyer
2015-06-08 8:59 ` Tejun Heo [this message]
2015-06-08 8:59 ` [PATCH 7/8] cfq-iosched: fold cfq_find_alloc_queue() into cfq_get_queue() Tejun Heo
[not found] ` <1433753973-23684-8-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2015-06-09 14:40 ` Jeff Moyer
2015-06-09 14:40 ` Jeff Moyer
[not found] ` <x49twuhrlml.fsf-RRHT56Q3PSP4kTEheFKJxxDDeQx5vsVwAInAS/Ez/D0@public.gmane.org>
2015-06-10 2:47 ` Tejun Heo
2015-06-10 2:47 ` Tejun Heo
2015-06-09 4:21 ` [PATCH 4.5/8] blkcg, cfq-iosched: use GFP_NOWAIT instead of GFP_ATOMIC for non-critical allocations Tejun Heo
2015-06-09 4:21 ` Tejun Heo
[not found] ` <20150609042131.GN21465-qYNAdHglDFBN0TnZuCh8vA@public.gmane.org>
2015-06-09 14:27 ` Jeff Moyer
2015-06-09 14:27 ` Jeff Moyer
2015-06-08 8:59 ` [PATCH 6/8] cfq-iosched: move cfq_group determination from cfq_find_alloc_queue() to cfq_get_queue() Tejun Heo
[not found] ` <1433753973-23684-7-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2015-06-09 14:32 ` Jeff Moyer
2015-06-09 14:32 ` Jeff Moyer
2015-06-08 8:59 ` [PATCH 8/8] cfq-iosched: charge async IOs to the appropriate blkcg's instead of the root Tejun Heo
[not found] ` <1433753973-23684-9-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2015-06-08 22:29 ` Vivek Goyal
2015-06-08 22:29 ` Vivek Goyal
[not found] ` <20150608222904.GB20918-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2015-06-09 3:11 ` Tejun Heo
2015-06-09 3:11 ` Tejun Heo
2015-06-09 15:03 ` Jeff Moyer
2015-06-09 15:03 ` Jeff Moyer
2015-06-08 19:49 ` [PATCHSET block/for-4.2/writeback] block, cgroup: make cfq charge async IOs to the appropriate blkcgs Jeff Moyer
[not found] ` <x491thmhtej.fsf-RRHT56Q3PSP4kTEheFKJxxDDeQx5vsVwAInAS/Ez/D0@public.gmane.org>
2015-06-09 3:03 ` Tejun Heo
2015-06-09 3:03 ` Tejun Heo
[not found] ` <20150609030327.GL21465-qYNAdHglDFBN0TnZuCh8vA@public.gmane.org>
2015-06-09 15:05 ` Jeff Moyer
2015-06-09 15:05 ` Jeff Moyer
[not found] ` <x49lhfssz05.fsf-RRHT56Q3PSP4kTEheFKJxxDDeQx5vsVwAInAS/Ez/D0@public.gmane.org>
2015-06-10 2:49 ` Tejun Heo
2015-06-10 2:49 ` Tejun Heo
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=1433753973-23684-8-git-send-email-tj@kernel.org \
--to=tj-dgejt+ai2ygdnm+yrofe0a@public.gmane.org \
--cc=avanzini.arianna-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
--cc=axboe-tSWWG44O7X1aa/9Udqfwiw@public.gmane.org \
--cc=cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=vgoyal-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.