Skip to content

Commit 6b2b045

Browse files
htejunaxboe
authored andcommitted
block: don't merge across cgroup boundaries if blkcg is enabled
blk-iocost and iolatency are cgroup aware rq-qos policies but they didn't disable merges across different cgroups. This obviously can lead to accounting and control errors but more importantly to priority inversions - e.g. an IO which belongs to a higher priority cgroup or IO class may end up getting throttled incorrectly because it gets merged to an IO issued from a low priority cgroup. Fix it by adding blk_cgroup_mergeable() which is called from merge paths and rejects cross-cgroup and cross-issue_as_root merges. Signed-off-by: Tejun Heo <tj@kernel.org> Fixes: d706751 ("block: introduce blk-iolatency io controller") Cc: stable@vger.kernel.org # v4.19+ Cc: Josef Bacik <jbacik@fb.com> Link: https://lore.kernel.org/r/Yi/eE/6zFNyWJ+qd@slm.duckdns.org Signed-off-by: Jens Axboe <axboe@kernel.dk>
1 parent aa1b46d commit 6b2b045

2 files changed

Lines changed: 28 additions & 0 deletions

File tree

block/blk-cgroup.h

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
*/
1616

1717
#include <linux/blk-cgroup.h>
18+
#include <linux/blk-mq.h>
1819

1920
/* percpu_counter batch for blkg_[rw]stats, per-cpu drift doesn't matter */
2021
#define BLKG_STAT_CPU_BATCH (INT_MAX / 2)
@@ -428,6 +429,21 @@ static inline void blkcg_clear_delay(struct blkcg_gq *blkg)
428429
atomic_dec(&blkg->blkcg->css.cgroup->congestion_count);
429430
}
430431

432+
/**
433+
* blk_cgroup_mergeable - Determine whether to allow or disallow merges
434+
* @rq: request to merge into
435+
* @bio: bio to merge
436+
*
437+
* @bio and @rq should belong to the same cgroup and their issue_as_root should
438+
* match. The latter is necessary as we don't want to throttle e.g. a metadata
439+
* update because it happens to be next to a regular IO.
440+
*/
441+
static inline bool blk_cgroup_mergeable(struct request *rq, struct bio *bio)
442+
{
443+
return rq->bio->bi_blkg == bio->bi_blkg &&
444+
bio_issue_as_root_blkg(rq->bio) == bio_issue_as_root_blkg(bio);
445+
}
446+
431447
void blk_cgroup_bio_start(struct bio *bio);
432448
void blkcg_add_delay(struct blkcg_gq *blkg, u64 now, u64 delta);
433449
#else /* CONFIG_BLK_CGROUP */
@@ -467,6 +483,7 @@ static inline void blkg_put(struct blkcg_gq *blkg) { }
467483
static inline bool blkcg_punt_bio_submit(struct bio *bio) { return false; }
468484
static inline void blkcg_bio_issue_init(struct bio *bio) { }
469485
static inline void blk_cgroup_bio_start(struct bio *bio) { }
486+
static inline bool blk_cgroup_mergeable(struct request *rq, struct bio *bio) { return true; }
470487

471488
#define blk_queue_for_each_rl(rl, q) \
472489
for ((rl) = &(q)->root_rl; (rl); (rl) = NULL)

block/blk-merge.c

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include <linux/blk-integrity.h>
1010
#include <linux/scatterlist.h>
1111
#include <linux/part_stat.h>
12+
#include <linux/blk-cgroup.h>
1213

1314
#include <trace/events/block.h>
1415

@@ -598,6 +599,9 @@ static inline unsigned int blk_rq_get_max_sectors(struct request *rq,
598599
static inline int ll_new_hw_segment(struct request *req, struct bio *bio,
599600
unsigned int nr_phys_segs)
600601
{
602+
if (!blk_cgroup_mergeable(req, bio))
603+
goto no_merge;
604+
601605
if (blk_integrity_merge_bio(req->q, req, bio) == false)
602606
goto no_merge;
603607

@@ -694,6 +698,9 @@ static int ll_merge_requests_fn(struct request_queue *q, struct request *req,
694698
if (total_phys_segments > blk_rq_get_max_segments(req))
695699
return 0;
696700

701+
if (!blk_cgroup_mergeable(req, next->bio))
702+
return 0;
703+
697704
if (blk_integrity_merge_rq(q, req, next) == false)
698705
return 0;
699706

@@ -902,6 +909,10 @@ bool blk_rq_merge_ok(struct request *rq, struct bio *bio)
902909
if (bio_data_dir(bio) != rq_data_dir(rq))
903910
return false;
904911

912+
/* don't merge across cgroup boundaries */
913+
if (!blk_cgroup_mergeable(rq, bio))
914+
return false;
915+
905916
/* only merge integrity protected bio into ditto rq */
906917
if (blk_integrity_merge_bio(rq->q, rq, bio) == false)
907918
return false;

0 commit comments

Comments
 (0)