[Devel] [PATCH hci-8.1] block/blk-cbt: account a write request also when dm starts it

[Pavel Tikhomirov <ptikhomirov@virtuozzo.com>]

CBT accounts a write at bio submission. dm suspend of request based dm
waits only for started requests, so a request submitted before the
suspend but dispatched after resume is accounted before a backup point
taken under the suspend (BLKCBTSTART, CBT_SNAP_CREATE) while its data
lands after it. push_backup then saves the old data, and no incremental
backup copies the new one.

Account a write again in dm_start_request(). It runs under the dispatch
rcu_read_lock(), so a missing bitmap page is allocated with GFP_ATOMIC
(CBT_ERROR on failure). This only happens for requests submitted before
BLKCBTSTART: any later write has its page allocated already by its
submission time accounting.

https://virtuozzo.atlassian.net/browse/VSTOR-149268

Feature: cbt: changed block tracking (for backup)
Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
---
Fix CBT missing writes that request-based dm holds across a suspend,
which made incremental backups taken with ploop-backup silently lose
data.

https://virtuozzo.atlassian.net/browse/VSTOR-149268
---
 block/blk-cbt.c        | 82 +++++++++++++++++++++++++++++++++++++++-----------
 drivers/md/dm-rq.c     |  1 +
 include/linux/blkdev.h |  4 +++
 3 files changed, 70 insertions(+), 17 deletions(-)

diff --git a/block/blk-cbt.c b/block/blk-cbt.c
index 90219f58f1ae5..7513acd713b00 100644
--- a/block/blk-cbt.c
+++ b/block/blk-cbt.c
@@ -11,6 +11,7 @@
 #include <linux/string.h>
 #include <linux/bio.h>
 #include <linux/blkdev.h>
+#include <linux/blk-mq.h>
 #include <linux/cpu.h>
 #include <linux/cpuset.h>
 #include <linux/list.h>
@@ -63,6 +64,13 @@ enum CBT_FLAGS
 	CBT_DEAD  = 1,
 	CBT_NOCACHE  = 2,
 };
+
+/* How a missing bitmap page may be allocated */
+enum cbt_alloc {
+	CBT_ALLOC_SLEEP = 0,	/* may sleep */
+	CBT_ALLOC_RCU = 1,	/* may sleep, dropping our own rcu_read_lock() */
+	CBT_ALLOC_ATOMIC = 2,	/* must not sleep */
+};
 static void cbt_release_callback(struct rcu_head *head);
 static void cbt_flush_cache(struct cbt_info *cbt);
 
@@ -103,7 +111,7 @@ static void set_bits(void *bm, int cur, int len, bool is_set)
  * -ENOMEM if alloc_page failed.
  */
 static int cbt_page_alloc(struct cbt_info  **cbt_pp, unsigned long idx,
-			  int in_rcu)
+			  enum cbt_alloc alloc)
 {
 	struct cbt_info	 *cbt = *cbt_pp;
 	struct page *page;
@@ -129,11 +137,15 @@ 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();
+	if (alloc == CBT_ALLOC_ATOMIC) {
+		page = alloc_page(GFP_ATOMIC | __GFP_ZERO | __GFP_NOWARN);
+	} else {
+		if (alloc == CBT_ALLOC_RCU)
+			rcu_read_unlock();
+		page = alloc_page(GFP_NOIO | __GFP_ZERO);
+		if (alloc == CBT_ALLOC_RCU)
+			rcu_read_lock();
+	}
 	spin_lock_irq(&cbt->lock);
 	if (unlikely(!--(cbt->count) && test_bit(CBT_DEAD, &cbt->flags))) {
 		spin_unlock_irq(&cbt->lock);
@@ -160,7 +172,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, enum cbt_alloc alloc, bool set,
 			 unsigned long *pages_missed,
 			 unsigned long *idx_first)
 {
@@ -210,7 +222,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, alloc);
 		if (ret == -EAGAIN) /* new cbt */
 			continue;
 		else if (ret == -EBADF) /* dead cbt */
@@ -221,7 +233,8 @@ static int __blk_cbt_set(struct cbt_info  *cbt, blkcnt_t block,
 	return (pages_missed && *pages_missed) ? -EAGAIN : 0;
 }
 
-static void blk_cbt_add(struct cbt_info *cbt, blkcnt_t start, blkcnt_t len)
+static void blk_cbt_add(struct cbt_info *cbt, blkcnt_t start, blkcnt_t len,
+			enum cbt_alloc alloc)
 {
 	struct cbt_extent *ex;
 	struct cbt_extent old;
@@ -233,7 +246,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, alloc, 1, NULL, NULL);
 		return;
 	}
 	ex = get_cpu_ptr(cbt->cache);
@@ -248,7 +261,7 @@ 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, alloc, 1, NULL, NULL);
 }
 
 void blk_cbt_bio_queue(struct request_queue *q, struct bio *bio)
@@ -260,12 +273,47 @@ void blk_cbt_bio_queue(struct request_queue *q, struct bio *bio)
 		goto out_rcu;
 
 	list_for_each_entry_rcu(cbt, &q->cbt_list, list)
-		blk_cbt_add(cbt, bio->bi_iter.bi_sector << 9, bio->bi_iter.bi_size);
+		blk_cbt_add(cbt, bio->bi_iter.bi_sector << 9, bio->bi_iter.bi_size,
+			    CBT_ALLOC_RCU);
 
 out_rcu:
 	rcu_read_unlock();
 }
 
+/*
+ * Account a write again when request based dm starts it: dm suspend waits
+ * only for started requests, so one accounted at submission before a
+ * backup point taken under suspend may still reach the disk after it.
+ *
+ * Account bio by bio, as a discard request may hold non-contiguous bios.
+ *
+ * Runs under the dispatch rcu_read_lock() (blk_mq_run_dispatch_ops()),
+ * hence GFP_ATOMIC.
+ */
+void blk_cbt_rq_start(struct request *rq)
+{
+	struct request_queue *q = rq->q;
+	struct cbt_info *cbt;
+	struct bio *bio;
+
+	if (likely(list_empty(&q->cbt_list)))
+		return;
+	if (rq_data_dir(rq) == READ || blk_rq_is_passthrough(rq))
+		return;
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(cbt, &q->cbt_list, list) {
+		__rq_for_each_bio(bio, rq) {
+			if (!bio->bi_iter.bi_size)
+				continue;
+			blk_cbt_add(cbt, bio->bi_iter.bi_sector << 9,
+				    bio->bi_iter.bi_size, CBT_ALLOC_ATOMIC);
+		}
+	}
+	rcu_read_unlock();
+}
+EXPORT_SYMBOL_GPL(blk_cbt_rq_start);
+
 static struct cbt_info *do_cbt_alloc(struct request_queue *q, __u8 *name,
 				     loff_t size, loff_t blocksize)
 {
@@ -545,7 +593,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, CBT_ALLOC_SLEEP))
 				goto out;
 
 			page_main = cbt->map[i];
@@ -734,7 +782,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, CBT_ALLOC_SLEEP, 1,
 					&ctx->pages_missed,
 					&ctx->idx_first);
 		if (!ret) {
@@ -765,7 +813,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, CBT_ALLOC_SLEEP);
 			if (ret == -EAGAIN) /* new cbt */
 				goto try_again;
 			else if (ret) /* dead cbt or alloc_page failed */
@@ -887,7 +935,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, CBT_ALLOC_SLEEP, 0, NULL, NULL);
 		cur_ex++;
 		ci.ci_mapped_extents++;
 		block = ex.start + ex.len;
@@ -971,7 +1019,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, CBT_ALLOC_SLEEP, set, NULL, NULL);
 		if (ret)
 			break;
 		cur_ex++;
diff --git a/drivers/md/dm-rq.c b/drivers/md/dm-rq.c
index 20f495bf5399d..9354e1e0c5a2d 100644
--- a/drivers/md/dm-rq.c
+++ b/drivers/md/dm-rq.c
@@ -444,6 +444,7 @@ ssize_t dm_attr_rq_based_seq_io_merge_deadline_store(struct mapped_device *md,
 
 static void dm_start_request(struct mapped_device *md, struct request *orig)
 {
+	blk_cbt_rq_start(orig);
 	blk_mq_start_request(orig);
 
 	if (unlikely(dm_stats_used(&md->stats))) {
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 1284bb0e4ebc0..848fac73e451f 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1627,6 +1627,7 @@ static inline bool blk_crypto_register(struct blk_crypto_profile *profile,
 extern void blk_cbt_update_size(struct block_device *bdev);
 extern void blk_cbt_release(struct request_queue *q);
 extern void blk_cbt_bio_queue(struct request_queue *q, struct bio *bio);
+extern void blk_cbt_rq_start(struct request *rq);
 extern int blk_cbt_ioctl(struct block_device *bdev, unsigned cmd, char __user *arg);
 #else /* CONFIG_BLK_DEV_CBT */
 static inline void blk_cbt_update_size(struct block_device *bdev)
@@ -1638,6 +1639,9 @@ static inline void blk_cbt_release(struct request_queue *q)
 static inline void blk_cbt_bio_queue(struct request_queue *q, struct bio *bio)
 {
 }
+static inline void blk_cbt_rq_start(struct request *rq)
+{
+}
 static inline int blk_cbt_ioctl(struct block_device *bdev, unsigned cmd,
 				char __user *arg)
 {

---
base-commit: f5d1fdcfffe101fe2bafa7df09d95d2e811c5c5e
change-id: 20261016-ptikhomirov-vstor-149268-cbt-account-at-rq-start-a592cf2010aa

Best regards,
--  
Pavel Tikhomirov <ptikhomirov@virtuozzo.com>

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