Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kernel-6.1: add patch for blkio with cgroupsv1 #225

Merged
merged 1 commit into from
Oct 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
From 4c5b35e202a3dd2c0d0bf0715c695ef3cc6d902e Mon Sep 17 00:00:00 2001
From: Jinke Han <hanjinke.666@bytedance.com>
Date: Mon, 8 May 2023 01:06:31 +0800
Subject: [PATCH] blk-throttle: Fix io statistics for cgroup v1

After commit f382fb0bcef4 ("block: remove legacy IO schedulers"),
blkio.throttle.io_serviced and blkio.throttle.io_service_bytes become
the only stable io stats interface of cgroup v1, and these statistics
are done in the blk-throttle code. But the current code only counts the
bios that are actually throttled. When the user does not add the throttle
limit, the io stats for cgroup v1 has nothing. I fix it according to the
statistical method of v2, and made it count all ios accurately.

Fixes: a7b36ee6ba29 ("block: move blk-throtl fast path inline")
Tested-by: Andrea Righi <andrea.righi@canonical.com>
Signed-off-by: Jinke Han <hanjinke.666@bytedance.com>
Acked-by: Muchun Song <songmuchun@bytedance.com>
Acked-by: Tejun Heo <tj@kernel.org>
Link: https://lore.kernel.org/r/20230507170631.89607-1-hanjinke.666@bytedance.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
[bcressey:
- backport to 6.1
- adjust context in blk_cgroup_bio_start
- avoid changes from 3b8cc629 ("blk-cgroup: Optimize blkcg_rstat_flush()")]
Signed-off-by: Ben Cressey <bcressey@amazon.com>
---
block/blk-cgroup.c | 6 ++++--
block/blk-throttle.c | 6 ------
block/blk-throttle.h | 9 +++++++++
3 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index 1b7fd1fc2f33..435432c4a62e 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -1969,6 +1969,9 @@ void blk_cgroup_bio_start(struct bio *bio)
struct blkg_iostat_set *bis;
unsigned long flags;

+ if (!cgroup_subsys_on_dfl(io_cgrp_subsys))
+ return;
+
cpu = get_cpu();
bis = per_cpu_ptr(bio->bi_blkg->iostat_cpu, cpu);
flags = u64_stats_update_begin_irqsave(&bis->sync);
@@ -1984,8 +1987,7 @@ void blk_cgroup_bio_start(struct bio *bio)
bis->cur.ios[rwd]++;
Comment on lines +43 to +47
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bcressey were these lines added to make the cherry-pick/port-back happy? I saw they were in what was patch 1008.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bcressey were these lines added to make the cherry-pick/port-back happy? I saw they were in what was patch 1008.

The call to cgroup_rstat_updated was what changed, to match the prior code in 6.1, which didn't have the lockless change.

There was also a bit of context that needed to be removed to relocate the guard to the top of the function.


u64_stats_update_end_irqrestore(&bis->sync, flags);
- if (cgroup_subsys_on_dfl(io_cgrp_subsys))
- cgroup_rstat_updated(bio->bi_blkg->blkcg->css.cgroup, cpu);
+ cgroup_rstat_updated(bio->bi_blkg->blkcg->css.cgroup, cpu);
put_cpu();
}

diff --git a/block/blk-throttle.c b/block/blk-throttle.c
index 62a3f62316df..ab847abe30b0 100644
--- a/block/blk-throttle.c
+++ b/block/blk-throttle.c
@@ -2176,12 +2176,6 @@ bool __blk_throtl_bio(struct bio *bio)

rcu_read_lock();

- if (!cgroup_subsys_on_dfl(io_cgrp_subsys)) {
- blkg_rwstat_add(&tg->stat_bytes, bio->bi_opf,
- bio->bi_iter.bi_size);
- blkg_rwstat_add(&tg->stat_ios, bio->bi_opf, 1);
- }
-
spin_lock_irq(&q->queue_lock);

throtl_update_latency_buckets(td);
diff --git a/block/blk-throttle.h b/block/blk-throttle.h
index ef4b7a4de987..d1ccbfe9f797 100644
--- a/block/blk-throttle.h
+++ b/block/blk-throttle.h
@@ -185,6 +185,15 @@ static inline bool blk_should_throtl(struct bio *bio)
struct throtl_grp *tg = blkg_to_tg(bio->bi_blkg);
int rw = bio_data_dir(bio);

+ if (!cgroup_subsys_on_dfl(io_cgrp_subsys)) {
+ if (!bio_flagged(bio, BIO_CGROUP_ACCT)) {
+ bio_set_flag(bio, BIO_CGROUP_ACCT);
+ blkg_rwstat_add(&tg->stat_bytes, bio->bi_opf,
+ bio->bi_iter.bi_size);
+ }
+ blkg_rwstat_add(&tg->stat_ios, bio->bi_opf, 1);
+ }
+
/* iops limit is always counted */
if (tg->has_rules_iops[rw])
return true;
--
2.45.1

3 changes: 3 additions & 0 deletions packages/kernel-6.1/kernel-6.1.spec
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ Patch1004: 1004-af_unix-increase-default-max_dgram_qlen-to-512.patch
# options for nvidia are instead included through DRM_SIMPLE
Patch1005: 1005-Revert-Revert-drm-fb_helper-improve-CONFIG_FB-depend.patch

# Fix cgroup v1 I/O statistics in blk-throttle to count all I/Os, not just throttled ones, aligning with cgroup v2 methods.
Patch1100: 1100-blk-throttle-Fix-io-statistics-for-cgroup-v1.patch

BuildRequires: bc
BuildRequires: elfutils-devel
BuildRequires: hostname
Expand Down