diff mbox series

[2/2] blkcg: don't offline parent blkcg first

Message ID 20190724173755.GB569612@devbig004.ftw2.facebook.com (mailing list archive)
State New, archived
Headers show
Series [RESEND,1/2] blkcg: rename blkcg->cgwb_refcnt to ->online_pin and always use it | expand

Commit Message

Tejun Heo July 24, 2019, 5:37 p.m. UTC
blkcg->cgwb_refcnt is used to delay blkcg offlining so that blkgs
don't get offlined while there are active cgwbs on them.  However, it
ends up making offlining unordered sometimes causing parents to be
offlined before children.

Let's fix this by making child blkcgs pin the parents' online states.

Note that pin/unpin names are chosen over get/put intentionally
because css uses get/put online for something different.

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

Comments

Tejun Heo April 1, 2020, 8:35 p.m. UTC | #1
On Wed, Jul 24, 2019 at 10:37:55AM -0700, Tejun Heo wrote:
> blkcg->cgwb_refcnt is used to delay blkcg offlining so that blkgs
> don't get offlined while there are active cgwbs on them.  However, it
> ends up making offlining unordered sometimes causing parents to be
> offlined before children.
> 
> Let's fix this by making child blkcgs pin the parents' online states.
> 
> Note that pin/unpin names are chosen over get/put intentionally
> because css uses get/put online for something different.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>

Jens, these two patches slipped through the cracks. Can you please take a look
at them?

Thanks.
Jens Axboe April 1, 2020, 8:57 p.m. UTC | #2
On 4/1/20 2:35 PM, Tejun Heo wrote:
> On Wed, Jul 24, 2019 at 10:37:55AM -0700, Tejun Heo wrote:
>> blkcg->cgwb_refcnt is used to delay blkcg offlining so that blkgs
>> don't get offlined while there are active cgwbs on them.  However, it
>> ends up making offlining unordered sometimes causing parents to be
>> offlined before children.
>>
>> Let's fix this by making child blkcgs pin the parents' online states.
>>
>> Note that pin/unpin names are chosen over get/put intentionally
>> because css uses get/put online for something different.
>>
>> Signed-off-by: Tejun Heo <tj@kernel.org>
> 
> Jens, these two patches slipped through the cracks. Can you please take a look
> at them?

Huh indeed, looks fine to me. I'll add for 5.7, thanks.
diff mbox series

Patch

--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -1158,6 +1158,21 @@  unlock:
 	return ret;
 }
 
+static int blkcg_css_online(struct cgroup_subsys_state *css)
+{
+	struct blkcg *blkcg = css_to_blkcg(css);
+	struct blkcg *parent = blkcg_parent(blkcg);
+
+	/*
+	 * blkcg_pin_online() is used to delay blkcg offline so that blkgs
+	 * don't go offline while cgwbs are still active on them.  Pin the
+	 * parent so that offline always happens towards the root.
+	 */
+	if (parent)
+		blkcg_pin_online(parent);
+	return 0;
+}
+
 /**
  * blkcg_init_queue - initialize blkcg part of request queue
  * @q: request_queue to initialize
@@ -1300,6 +1315,7 @@  static void blkcg_exit(struct task_struc
 
 struct cgroup_subsys io_cgrp_subsys = {
 	.css_alloc = blkcg_css_alloc,
+	.css_online = blkcg_css_online,
 	.css_offline = blkcg_css_offline,
 	.css_free = blkcg_css_free,
 	.can_attach = blkcg_can_attach,
--- a/include/linux/blk-cgroup.h
+++ b/include/linux/blk-cgroup.h
@@ -455,8 +455,12 @@  static inline void blkcg_pin_online(stru
  */
 static inline void blkcg_unpin_online(struct blkcg *blkcg)
 {
-	if (refcount_dec_and_test(&blkcg->online_pin))
+	do {
+		if (!refcount_dec_and_test(&blkcg->online_pin))
+			break;
 		blkcg_destroy_blkgs(blkcg);
+		blkcg = blkcg_parent(blkcg);
+	} while (blkcg);
 }
 
 /**