Skip to contentSkip to conversation

FAILED: patch "[PATCH] blkcg: make blkcg_print_stat() print stats only for online" failed to apply to 4.19-stable tree

PATCH [email protected] 2 participants 2 messages Nov 11, 2019

Subsystems (3)
lore
Share & tools
lore

[email protected]

author maintainer
#0 lore raw

The patch below does not apply to the 4.19-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From b0814361a25cba73a224548843ed92d8ea78715a Mon Sep 17 00:00:00 2001
From: Tejun Heo <[email protected]>
Date: Tue, 5 Nov 2019 08:09:51 -0800
Subject: [PATCH] blkcg: make blkcg_print_stat() print stats only for online
blkgs

blkcg_print_stat() iterates blkgs under RCU and doesn't test whether the blkg is online. This can call into pd_stat_fn() on a pd which is still being initialized leading to an oops.

The heaviest operation - recursively summing up rwstat counters - is already done while holding the queue_lock. Expand queue_lock to cover the other operations and skip the blkg if it isn't online yet. The online state is protected by both blkcg and queue locks, so this guarantees that only online blkgs are processed.

Cc Josef Bacik <[email protected]>
Fixes 903d23f0a354 ("blk-cgroup: allow controllers to output their own stats")
Cc [email protected] # v4.19+
block/blk-cgroup.c +8 -5
index 5d21027b1faf..1eb8895be4c6 100644
@@ -934,9 +934,14 @@ static int blkcg_print_stat(struct seq_file *sf, void *v)
 		int i;
 		bool has_stats = false;
 
+		spin_lock_irq(&blkg->q->queue_lock);
+
+		if (!blkg->online)
+			goto skip;
+
 		dname = blkg_dev_name(blkg);
 		if (!dname)
-			continue;
+			goto skip;
 
 		/*
 		 * Hooray string manipulation, count is the size written NOT
@@ -946,8 +951,6 @@ static int blkcg_print_stat(struct seq_file *sf, void *v)
 		 */
 		off += scnprintf(buf+off, size-off, "%s ", dname);
 
-		spin_lock_irq(&blkg->q->queue_lock);
-
 		blkg_rwstat_recursive_sum(blkg, NULL,
 				offsetof(struct blkcg_gq, stat_bytes), &rwstat);
 		rbytes = rwstat.cnt[BLKG_RWSTAT_READ];
@@ -960,8 +963,6 @@ static int blkcg_print_stat(struct seq_file *sf, void *v)
 		wios = rwstat.cnt[BLKG_RWSTAT_WRITE];
 		dios = rwstat.cnt[BLKG_RWSTAT_DISCARD];
 
-		spin_unlock_irq(&blkg->q->queue_lock);
-
 		if (rbytes || wbytes || rios || wios) {
 			has_stats = true;
 			off += scnprintf(buf+off, size-off,
@@ -999,6 +1000,8 @@ static int blkcg_print_stat(struct seq_file *sf, void *v)
 				seq_commit(sf, -1);
 			}
 		}
+	skip:
+		spin_unlock_irq(&blkg->q->queue_lock);
 	}
 
 	rcu_read_unlock();
maintainer
#1 lore raw
Quoted 28 lines · On Mon, Nov 11, 2019 at 07:12:24AM +0100, [email protected] wrote:

The patch below does not apply to the 4.19-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From b0814361a25cba73a224548843ed92d8ea78715a Mon Sep 17 00:00:00 2001
From: Tejun Heo <[email protected]>
Date: Tue, 5 Nov 2019 08:09:51 -0800
Subject: [PATCH] blkcg: make blkcg_print_stat() print stats only for online blkgs

blkcg_print_stat() iterates blkgs under RCU and doesn't test whether the blkg is online. This can call into pd_stat_fn() on a pd which is still being initialized leading to an oops.

The heaviest operation - recursively summing up rwstat counters - is already done while holding the queue_lock. Expand queue_lock to cover the other operations and skip the blkg if it isn't online yet. The online state is protected by both blkcg and queue locks, so this guarantees that only online blkgs are processed.

Quoted 6 lines
Cc Josef Bacik <[email protected]>
Fixes 903d23f0a354 ("blk-cgroup: allow controllers to output their own stats")
Cc [email protected] # v4.19+

I've adjusted the patch for not having 0d945c1f966b ("block: remove the queue_lock indirection") and queued it for 4.19.

Signature · 2 lines
Thanks,
Sasha