public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: axboe@kernel.dk
Cc: linux-kernel@vger.kernel.org, kernel-team@fb.com,
	vgoyal@redhat.com, avanzini.arianna@gmail.com,
	Tejun Heo <tj@kernel.org>
Subject: [PATCH 04/11] blkcg: restructure blkg_policy_data allocation in blkcg_activate_policy()
Date: Sat, 11 Jul 2015 14:00:47 -0400	[thread overview]
Message-ID: <1436637654-28110-5-git-send-email-tj@kernel.org> (raw)
In-Reply-To: <1436637654-28110-1-git-send-email-tj@kernel.org>

When a policy gets activated, it needs to allocate and install its
policy data on all existing blkg's (blkcg_gq's).  Because blkg
iteration is protected by a spinlock, it currently counts the total
number of blkg's in the system, allocates the matching number of
policy data on a list and installs them during a single iteration.

This can be simplified by using speculative GFP_NOWAIT allocations
while iterating and falling back to a preallocated policy data on
failure.  If the preallocated one has already been consumed, it
releases the lock, preallocate with GFP_KERNEL and then restarts the
iteration.  This can be a bit more expensive than before but policy
activation is a very cold path and shouldn't matter.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
 block/blk-cgroup.c         | 55 ++++++++++++++++++----------------------------
 include/linux/blk-cgroup.h |  3 ---
 2 files changed, 21 insertions(+), 37 deletions(-)

diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index 2a493ce..5dbbacd 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -1043,65 +1043,52 @@ EXPORT_SYMBOL_GPL(blkio_cgrp_subsys);
 int blkcg_activate_policy(struct request_queue *q,
 			  const struct blkcg_policy *pol)
 {
-	LIST_HEAD(pds);
+	struct blkg_policy_data *pd_prealloc = NULL;
 	struct blkcg_gq *blkg;
-	struct blkg_policy_data *pd, *nd;
-	int cnt = 0, ret;
+	int ret;
 
 	if (blkcg_policy_enabled(q, pol))
 		return 0;
 
-	/* count and allocate policy_data for all existing blkgs */
 	blk_queue_bypass_start(q);
-	spin_lock_irq(q->queue_lock);
-	list_for_each_entry(blkg, &q->blkg_list, q_node)
-		cnt++;
-	spin_unlock_irq(q->queue_lock);
-
-	/* allocate per-blkg policy data for all existing blkgs */
-	while (cnt--) {
-		pd = kzalloc_node(pol->pd_size, GFP_KERNEL, q->node);
-		if (!pd) {
+pd_prealloc:
+	if (!pd_prealloc) {
+		pd_prealloc = kzalloc_node(pol->pd_size, GFP_KERNEL, q->node);
+		if (!pd_prealloc) {
 			ret = -ENOMEM;
-			goto out_free;
+			goto out_bypass_end;
 		}
-		list_add_tail(&pd->alloc_node, &pds);
 	}
 
-	/*
-	 * Install the allocated pds and cpds. With @q bypassing, no new blkg
-	 * should have been created while the queue lock was dropped.
-	 */
 	spin_lock_irq(q->queue_lock);
 
 	list_for_each_entry(blkg, &q->blkg_list, q_node) {
-		if (WARN_ON(list_empty(&pds))) {
-			/* umm... this shouldn't happen, just abort */
-			ret = -ENOMEM;
-			goto out_unlock;
-		}
-		pd = list_first_entry(&pds, struct blkg_policy_data, alloc_node);
-		list_del_init(&pd->alloc_node);
+		struct blkg_policy_data *pd;
 
-		/* grab blkcg lock too while installing @pd on @blkg */
-		spin_lock(&blkg->blkcg->lock);
+		if (blkg->pd[pol->plid])
+			continue;
+
+		pd = kzalloc_node(pol->pd_size, GFP_NOWAIT, q->node);
+		if (!pd)
+			swap(pd, pd_prealloc);
+		if (!pd) {
+			spin_unlock_irq(q->queue_lock);
+			goto pd_prealloc;
+		}
 
 		blkg->pd[pol->plid] = pd;
 		pd->blkg = blkg;
 		pd->plid = pol->plid;
 		pol->pd_init_fn(blkg);
-
-		spin_unlock(&blkg->blkcg->lock);
 	}
 
 	__set_bit(pol->plid, q->blkcg_pols);
 	ret = 0;
-out_unlock:
+
 	spin_unlock_irq(q->queue_lock);
-out_free:
+out_bypass_end:
 	blk_queue_bypass_end(q);
-	list_for_each_entry_safe(pd, nd, &pds, alloc_node)
-		kfree(pd);
+	kfree(pd_prealloc);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(blkcg_activate_policy);
diff --git a/include/linux/blk-cgroup.h b/include/linux/blk-cgroup.h
index 9711fc2..db82288 100644
--- a/include/linux/blk-cgroup.h
+++ b/include/linux/blk-cgroup.h
@@ -80,9 +80,6 @@ struct blkg_policy_data {
 	/* the blkg and policy id this per-policy data belongs to */
 	struct blkcg_gq			*blkg;
 	int				plid;
-
-	/* used during policy activation */
-	struct list_head		alloc_node;
 };
 
 /*
-- 
2.4.3


  parent reply	other threads:[~2015-07-11 18:01 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-11 18:00 [PATCHSET v3 block/for-4.3] blkcg: blkcg policy methods and data handling cleanup Tejun Heo
2015-07-11 18:00 ` [PATCH 01/11] blkcg: remove unnecessary request_list->blkg NULL test in blk_put_rl() Tejun Heo
2015-07-11 18:00 ` [PATCH 02/11] blkcg: use blkg_free() in blkcg_init_queue() failure path Tejun Heo
2015-07-11 18:00 ` [PATCH 03/11] blkcg: remove unnecessary blkcg_root handling from css_alloc/free paths Tejun Heo
2015-07-11 18:00 ` Tejun Heo [this message]
2015-07-11 18:00 ` [PATCH 05/11] blkcg: make blkcg_activate_policy() allow NULL ->pd_init_fn Tejun Heo
2015-07-11 18:00 ` [PATCH 06/11] blkcg: replace blkcg_policy->pd_size with ->pd_alloc/free_fn() methods Tejun Heo
2015-07-11 18:00 ` [PATCH 07/11] blk-throttle: remove asynchrnous percpu stats allocation mechanism Tejun Heo
2015-07-11 18:00 ` [PATCH 08/11] blk-throttle: clean up blkg_policy_data alloc/init/exit/free methods Tejun Heo
2015-07-11 18:00 ` [PATCH 09/11] blkcg: make blkcg_policy methods take a pointer to blkcg_policy_data Tejun Heo
2015-07-11 18:00 ` [PATCH 10/11] blkcg: cosmetic updates about blkcg_policy_data Tejun Heo
2015-07-30 22:57   ` [PATCH 10/11] blkcg: minor updates around blkcg_policy_data Tejun Heo
2015-07-11 18:00 ` [PATCH 11/11] blkcg: replace blkcg_policy->cpd_size with ->cpd_alloc/free_fn() methods 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=1436637654-28110-5-git-send-email-tj@kernel.org \
    --to=tj@kernel.org \
    --cc=avanzini.arianna@gmail.com \
    --cc=axboe@kernel.dk \
    --cc=kernel-team@fb.com \
    --cc=linux-kernel@vger.kernel.org \
    --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