blkcg: clean up blkg_tryget_closest()
authorDennis Zhou <dennis@kernel.org>
Wed, 19 Dec 2018 22:43:21 +0000 (16:43 -0600)
committerJens Axboe <axboe@kernel.dk>
Fri, 21 Dec 2018 15:47:05 +0000 (08:47 -0700)
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 <dennis@kernel.org>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
block/bio.c
include/linux/blk-cgroup.h

index c288b90570423682f32053bff15d8a29621a71cb..9194d8ad3d5e23535e5752c18c8e7882a81c848e 100644 (file)
@@ -2096,8 +2096,12 @@ EXPORT_SYMBOL_GPL(bio_associate_blkg);
  */
 void bio_clone_blkg_association(struct bio *dst, struct bio *src)
 {
  */
 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);
        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 */
 }
 EXPORT_SYMBOL_GPL(bio_clone_blkg_association);
 #endif /* CONFIG_BLK_CGROUP */
index f025fd1e22e654bff0803b0a80599eafee196ae4..76c61318fda5b5c714cb8f2926b79c439fcb45f8 100644 (file)
@@ -499,22 +499,33 @@ static inline void blkg_get(struct blkcg_gq *blkg)
  */
 static inline bool blkg_tryget(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
  *
 }
 
 /**
  * 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)
 {
  */
 static inline struct blkcg_gq *blkg_tryget_closest(struct blkcg_gq *blkg)
 {
-       while (blkg && !percpu_ref_tryget(&blkg->refcnt))
+       struct blkcg_gq *ret_blkg = NULL;
+
+       WARN_ON_ONCE(!rcu_read_lock_held());
+
+       while (blkg) {
+               if (blkg_tryget(blkg)) {
+                       ret_blkg = blkg;
+                       break;
+               }
                blkg = blkg->parent;
                blkg = blkg->parent;
+       }
 
 
-       return blkg;
+       return ret_blkg;
 }
 
 /**
 }
 
 /**