[Devel] [PATCH hci-8.1 v7 6/8] block/blk-cbt: walk cbt_list under SRCU

[Pavel Tikhomirov <ptikhomirov@virtuozzo.com>]

blk_cbt_bio_queue() walks q->cbt_list under rcu_read_lock(), but a cbt
may need a bitmap page there: cbt_page_alloc() then drops the RCU lock
around the sleeping allocation, pinning only the current cbt. A CBT stop
or resize meanwhile can unlink it and free the next one, which the walk
then reaches through the stale ->next.

Walk cbt_list and release cbts under SRCU instead, whose readers may
sleep, so the allocation no longer drops the lock. Take it after the
early exits of blk_cbt_bio_queue(), which need no lock.

https://virtuozzo.atlassian.net/browse/VSTOR-149268
Feature: cbt: changed block tracking (for backup)
Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
---
 block/blk-cbt.c | 50 +++++++++++++++++++++++++-------------------------
 1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/block/blk-cbt.c b/block/blk-cbt.c
index 10e03a29114a4..3ad4b9a25e1f4 100644
--- a/block/blk-cbt.c
+++ b/block/blk-cbt.c
@@ -15,6 +15,7 @@
 #include <linux/cpuset.h>
 #include <linux/list.h>
 #include <linux/spinlock.h>
+#include <linux/srcu.h>
 #include <linux/pagemap.h>
 #include <linux/vmalloc.h>
 #include <linux/log2.h>
@@ -34,6 +35,8 @@
 })
 
 static __cacheline_aligned_in_smp DEFINE_MUTEX(cbt_mutex);
+/* Protects the q->cbt_list walks, which may sleep to allocate */
+DEFINE_STATIC_SRCU(cbt_srcu);
 
 struct cbt_extent{
 	blkcnt_t start;
@@ -105,8 +108,7 @@ static void set_bits(void *bm, int cur, int len, bool is_set)
  * -EBADF if cbt is dead,
  * -ENOMEM if alloc_page failed.
  */
-static int cbt_page_alloc(struct cbt_info  **cbt_pp, unsigned long idx,
-			  int in_rcu)
+static int cbt_page_alloc(struct cbt_info  **cbt_pp, unsigned long idx)
 {
 	struct cbt_info	 *cbt = *cbt_pp;
 	struct page *page;
@@ -118,7 +120,9 @@ static int cbt_page_alloc(struct cbt_info  **cbt_pp, unsigned long idx,
 	} else {
 		struct cbt_info *i, *new = NULL;
 
-		list_for_each_entry_rcu(i, &cbt->queue->cbt_list, list)
+		list_for_each_entry_srcu(i, &cbt->queue->cbt_list, list,
+					 srcu_read_lock_held(&cbt_srcu) ||
+					 lockdep_is_held(&cbt_mutex))
 			if (!memcmp(i->name, cbt->name, sizeof(cbt->name)))
 				new = i;
 
@@ -132,15 +136,11 @@ static int cbt_page_alloc(struct cbt_info  **cbt_pp, unsigned long idx,
 		}
 	}
 	spin_unlock_irq(&cbt->lock);
-	if (in_rcu)
-		rcu_read_unlock();
 	page = alloc_page(GFP_NOIO|__GFP_ZERO);
-	if (in_rcu)
-		rcu_read_lock();
 	spin_lock_irq(&cbt->lock);
 	if (unlikely(!--(cbt->count) && test_bit(CBT_DEAD, &cbt->flags))) {
 		spin_unlock_irq(&cbt->lock);
-		call_rcu(&cbt->rcu, &cbt_release_callback);
+		call_srcu(&cbt_srcu, &cbt->rcu, &cbt_release_callback);
 		if (page)
 			__free_page(page);
 		return -EBADF;
@@ -165,7 +165,7 @@ static int cbt_page_alloc(struct cbt_info  **cbt_pp, unsigned long idx,
 }
 
 static int __blk_cbt_set(struct cbt_info  *cbt, blkcnt_t block,
-			 blkcnt_t count, bool in_rcu, bool set,
+			 blkcnt_t count, bool set,
 			 unsigned long *pages_missed,
 			 unsigned long *idx_first)
 {
@@ -220,7 +220,7 @@ static int __blk_cbt_set(struct cbt_info  *cbt, blkcnt_t block,
 			}
 		}
 
-		ret = cbt_page_alloc(&cbt, idx, in_rcu);
+		ret = cbt_page_alloc(&cbt, idx);
 		if (ret == -EAGAIN) /* new cbt */
 			continue;
 		else if (ret == -EBADF) /* dead cbt */
@@ -243,7 +243,7 @@ static void blk_cbt_add(struct cbt_info *cbt, blkcnt_t start, blkcnt_t len)
 	start >>= cbt->block_bits;
 	len = end - start;
 	if (unlikely(test_bit(CBT_NOCACHE, &cbt->flags))) {
-		__blk_cbt_set(cbt, start, len, 1, 1, NULL, NULL);
+		__blk_cbt_set(cbt, start, len, 1, NULL, NULL);
 		return;
 	}
 	ex = get_cpu_ptr(cbt->cache);
@@ -258,22 +258,22 @@ static void blk_cbt_add(struct cbt_info *cbt, blkcnt_t start, blkcnt_t len)
 	put_cpu_ptr(cbt->cache);
 
 	if (likely(old.len))
-		__blk_cbt_set(cbt, old.start, old.len, 1, 1, NULL, NULL);
+		__blk_cbt_set(cbt, old.start, old.len, 1, NULL, NULL);
 }
 
 void blk_cbt_bio_queue(struct request_queue *q, struct bio *bio)
 {
 	struct cbt_info *cbt;
+	int idx;
 
-	rcu_read_lock();
 	if (list_empty(&q->cbt_list) || bio_data_dir(bio) == READ || !bio->bi_iter.bi_size)
-		goto out_rcu;
+		return;
 
-	list_for_each_entry_rcu(cbt, &q->cbt_list, list)
+	idx = srcu_read_lock(&cbt_srcu);
+	list_for_each_entry_srcu(cbt, &q->cbt_list, list,
+				 srcu_read_lock_held(&cbt_srcu))
 		blk_cbt_add(cbt, bio->bi_iter.bi_sector << 9, bio->bi_iter.bi_size);
-
-out_rcu:
-	rcu_read_unlock();
+	srcu_read_unlock(&cbt_srcu, idx);
 }
 
 static struct cbt_info *do_cbt_alloc(struct request_queue *q, __u8 *name,
@@ -555,7 +555,7 @@ static int blk_cbt_snap_merge_back(struct request_queue *q, __u8 *name)
 
 		if (!page_main) {
 			ret = -ENOMEM;
-			if (cbt_page_alloc(&cbt, i, 0))
+			if (cbt_page_alloc(&cbt, i))
 				goto out;
 
 			page_main = cbt->map[i];
@@ -608,7 +608,7 @@ static void blk_cbt_update_cbt_size(struct cbt_info *cbt, loff_t new_sz)
 	in_use = cbt->count;
 	spin_unlock_irq(&cbt->lock);
 	if (!in_use)
-		call_rcu(&cbt->rcu, &cbt_release_callback);
+		call_srcu(&cbt_srcu, &cbt->rcu, &cbt_release_callback);
 }
 
 void blk_cbt_update_size(struct block_device *bdev)
@@ -696,7 +696,7 @@ static void blk_cbt_del(struct cbt_info *cbt)
 	in_use = cbt->count;
 	spin_unlock_irqrestore(&cbt->lock, flags);
 	if (!in_use)
-		call_rcu(&cbt->rcu, &cbt_release_callback);
+		call_srcu(&cbt_srcu, &cbt->rcu, &cbt_release_callback);
 }
 
 void blk_cbt_release(struct request_queue *q)
@@ -744,7 +744,7 @@ static inline void __cbt_flush_cpu_cache(void *ptr)
 	struct cbt_extent *ex = this_cpu_ptr(cbt->cache);
 
 	if (ex->len) {
-		int ret = __blk_cbt_set(cbt, ex->start, ex->len, 0, 1,
+		int ret = __blk_cbt_set(cbt, ex->start, ex->len, 1,
 					&ctx->pages_missed,
 					&ctx->idx_first);
 		if (!ret) {
@@ -775,7 +775,7 @@ static void cbt_flush_cache(struct cbt_info *cbt)
 			if (cbt->map[i] != CBT_PAGE_MISSED)
 				continue;
 
-			ret = cbt_page_alloc(&cbt, i, 0);
+			ret = cbt_page_alloc(&cbt, i);
 			if (ret == -EAGAIN) /* new cbt */
 				goto try_again;
 			else if (ret) /* dead cbt or alloc_page failed */
@@ -897,7 +897,7 @@ static int cbt_ioc_get(struct block_device *bdev, struct blk_user_cbt_info __use
 		cur_ex->ce_length = ex.len << cbt->block_bits;
 
 		if (ci.ci_flags & CI_FLAG_ONCE)
-			__blk_cbt_set(cbt, ex.start, ex.len, 0, 0, NULL, NULL);
+			__blk_cbt_set(cbt, ex.start, ex.len, 0, NULL, NULL);
 		cur_ex++;
 		ci.ci_mapped_extents++;
 		block = ex.start + ex.len;
@@ -981,7 +981,7 @@ static int cbt_ioc_set(struct block_device *bdev, struct blk_user_cbt_info __use
 			ret = -EINVAL;
 			break;
 		}
-		ret = __blk_cbt_set(cbt, ex.start, ex.len, 0, set, NULL, NULL);
+		ret = __blk_cbt_set(cbt, ex.start, ex.len, set, NULL, NULL);
 		if (ret)
 			break;
 		cur_ex++;

-- 
2.55.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel