From 6ab2187992f4b0112852e5a097a2b6c7d167e2e5 Mon Sep 17 00:00:00 2001 From: Dennis Zhou Date: Wed, 19 Dec 2018 16:43:21 -0600 Subject: [PATCH] blkcg: clean up blkg_tryget_closest() The implementation of blkg_tryget_closest() wasn't super obvious and became a point of suspicion when debugging [1]. So let's clean it up so it's obviously not the problem. Also add missing RCU read locking to bio_clone_blkg_association(), which got exposed by adding the RCU read lock held check in blkg_tryget_closest(). [1] https://lore.kernel.org/linux-block/a7e97e4b-0dd8-3a54-23b7-a0f27b17fde8@kernel.dk/ Signed-off-by: Dennis Zhou Signed-off-by: Jens Axboe --- block/bio.c | 4 ++++ include/linux/blk-cgroup.h | 23 +++++++++++++++++------ 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/block/bio.c b/block/bio.c index c288b9057042..9194d8ad3d5e 100644 --- a/block/bio.c +++ b/block/bio.c @@ -2096,8 +2096,12 @@ EXPORT_SYMBOL_GPL(bio_associate_blkg); */ void bio_clone_blkg_association(struct bio *dst, struct bio *src) { + rcu_read_lock(); + if (src->bi_blkg) __bio_associate_blkg(dst, src->bi_blkg); + + rcu_read_unlock(); } EXPORT_SYMBOL_GPL(bio_clone_blkg_association); #endif /* CONFIG_BLK_CGROUP */ diff --git a/include/linux/blk-cgroup.h b/include/linux/blk-cgroup.h index f025fd1e22e6..76c61318fda5 100644 --- a/include/linux/blk-cgroup.h +++ b/include/linux/blk-cgroup.h @@ -499,22 +499,33 @@ static inline void blkg_get(struct blkcg_gq *blkg) */ static inline bool blkg_tryget(struct blkcg_gq *blkg) { - return percpu_ref_tryget(&blkg->refcnt); + return blkg && percpu_ref_tryget(&blkg->refcnt); } /** * blkg_tryget_closest - try and get a blkg ref on the closet blkg * @blkg: blkg to get * - * This walks up the blkg tree to find the closest non-dying blkg and returns - * the blkg that it did association with as it may not be the passed in blkg. + * This needs to be called rcu protected. As the failure mode here is to walk + * up the blkg tree, this ensure that the blkg->parent pointers are always + * valid. This returns the blkg that it ended up taking a reference on or %NULL + * if no reference was taken. */ static inline struct blkcg_gq *blkg_tryget_closest(struct blkcg_gq *blkg) { - while (blkg && !percpu_ref_tryget(&blkg->refcnt)) - blkg = blkg->parent; + struct blkcg_gq *ret_blkg = NULL; - return blkg; + WARN_ON_ONCE(!rcu_read_lock_held()); + + while (blkg) { + if (blkg_tryget(blkg)) { + ret_blkg = blkg; + break; + } + blkg = blkg->parent; + } + + return ret_blkg; } /**