* [Devel] [PATCH hci-8.1 v3 0/3] block/blk-cbt: don't lose writes held across a dm suspend
@ 2026-10-27 10:18 Pavel Tikhomirov
2026-10-27 10:18 ` [Devel] [PATCH hci-8.1 v3 1/3] block/blk-cbt: shift the block size as unsigned Pavel Tikhomirov
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Pavel Tikhomirov @ 2026-10-27 10:18 UTC (permalink / raw)
To: Konstantin Khorenko; +Cc: devel
Fix CBT missing writes that request-based dm holds across a suspend,
which made incremental backups taken with ploop-backup silently lose
data.
Claude tested full and incremental ploop-backup under write load (a
random writer, and fio with 4 jobs x iodepth 64): the incremental backup
lost blocks without the fix, and was consistent 10/10 with it. CBT
start/stop and device resize under fio ran without warnings.
https://virtuozzo.atlassian.net/browse/VSTOR-149268
Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
---
Changes in v3:
- New patch: account a write delayed by blk-throttle in CBT, as the
throttle dispatch work skipped CBT (sashiko review).
- New patch: shift the CBT block size as unsigned, and use the unsigned
shift in the new cbt_set_present() too (sashiko review).
- Link to v2: http://lore.virtuozzo.com/20261027-ptikhomirov-vstor-149268-cbt-account-at-rq-start-v2-1-c64ee6178730@virtuozzo.com
Changes in v2:
- No GFP_ATOMIC allocation at dispatch: bitmap pages are now allocated
at bio submission, and the dispatch hook only sets bits in present pages.
- A page missing at dispatch (a write submitted before BLKCBTSTART) is
allocated with GFP_NOIO from the kblockd workqueue, and dm-rq returns
BLK_STS_RESOURCE so that blk-mq dispatches the request again later,
instead of setting CBT_ERROR when an atomic allocation fails.
- Move the hook from dm_start_request() to dm_mq_queue_rq(), before the
request is started, so that it can return BLK_STS_RESOURCE.
- Link to v1: http://lore.virtuozzo.com/20261016-ptikhomirov-vstor-149268-cbt-account-at-rq-start-v1-1-1f20b151431d@virtuozzo.com
---
Pavel Tikhomirov (3):
block/blk-cbt: shift the block size as unsigned
block/blk-throttle: account a throttled write in CBT
block/blk-cbt: account a write request also when dm starts it
block/blk-cbt.c | 183 ++++++++++++++++++++++++++++++++++++++++++++++++-
block/blk-throttle.c | 5 +-
drivers/md/dm-rq.c | 3 +
include/linux/blkdev.h | 5 ++
4 files changed, 192 insertions(+), 4 deletions(-)
---
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Devel] [PATCH hci-8.1 v3 1/3] block/blk-cbt: shift the block size as unsigned
2026-10-27 10:18 [Devel] [PATCH hci-8.1 v3 0/3] block/blk-cbt: don't lose writes held across a dm suspend Pavel Tikhomirov
@ 2026-10-27 10:18 ` Pavel Tikhomirov
2026-10-27 10:22 ` no-reply
2026-10-27 10:18 ` [Devel] [PATCH hci-8.1 v3 2/3] block/blk-throttle: account a throttled write in CBT Pavel Tikhomirov
2026-10-27 10:18 ` [Devel] [PATCH hci-8.1 v3 3/3] block/blk-cbt: account a write request also when dm starts it Pavel Tikhomirov
2 siblings, 1 reply; 7+ messages in thread
From: Pavel Tikhomirov @ 2026-10-27 10:18 UTC (permalink / raw)
To: Konstantin Khorenko; +Cc: devel
cbt_ioc_init() accepts any power of two block size, so block_bits can be
31, and then 1 << block_bits overflows int: the sign extended block size
makes the rounded up block counts wrong. Shift an unsigned one instead.
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 | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/block/blk-cbt.c b/block/blk-cbt.c
index 90219f58f1ae5..1066b6a33b6b5 100644
--- a/block/blk-cbt.c
+++ b/block/blk-cbt.c
@@ -229,7 +229,7 @@ static void blk_cbt_add(struct cbt_info *cbt, blkcnt_t start, blkcnt_t len)
if (unlikely(test_bit(CBT_ERROR, &cbt->flags)))
return;
- end = DIV_ROUND_UP(start + len, 1 << cbt->block_bits);
+ end = DIV_ROUND_UP(start + len, 1UL << cbt->block_bits);
start >>= cbt->block_bits;
len = end - start;
if (unlikely(test_bit(CBT_NOCACHE, &cbt->flags))) {
@@ -575,7 +575,7 @@ static void blk_cbt_update_cbt_size(struct cbt_info *cbt, loff_t new_sz)
unsigned bsz;
int in_use = 0;
- bsz = 1 << cbt->block_bits;
+ bsz = 1U << cbt->block_bits;
if (DIV_ROUND_UP(new_sz, bsz) <= cbt->block_max)
return;
@@ -964,7 +964,7 @@ static int cbt_ioc_set(struct block_device *bdev, struct blk_user_cbt_info __use
struct cbt_extent ex;
ex.start = cur_ex->ce_physical >> cbt->block_bits;
- ex.len = DIV_ROUND_UP(cur_ex->ce_length, 1 << cbt->block_bits);
+ ex.len = DIV_ROUND_UP(cur_ex->ce_length, 1UL << cbt->block_bits);
if (ex.start > cbt->block_max ||
ex.start + ex.len > cbt->block_max ||
ex.len == 0) {
--
2.55.0
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Devel] [PATCH hci-8.1 v3 2/3] block/blk-throttle: account a throttled write in CBT
2026-10-27 10:18 [Devel] [PATCH hci-8.1 v3 0/3] block/blk-cbt: don't lose writes held across a dm suspend Pavel Tikhomirov
2026-10-27 10:18 ` [Devel] [PATCH hci-8.1 v3 1/3] block/blk-cbt: shift the block size as unsigned Pavel Tikhomirov
@ 2026-10-27 10:18 ` Pavel Tikhomirov
2026-10-27 10:22 ` no-reply
2026-10-27 10:18 ` [Devel] [PATCH hci-8.1 v3 3/3] block/blk-cbt: account a write request also when dm starts it Pavel Tikhomirov
2 siblings, 1 reply; 7+ messages in thread
From: Pavel Tikhomirov @ 2026-10-27 10:18 UTC (permalink / raw)
To: Konstantin Khorenko; +Cc: devel
submit_bio_noacct() accounts a write in CBT only when blk-throttle does
not hold it: a throttled bio is later issued from the throttle dispatch
work with submit_bio_noacct_nocheck(), which skips CBT. So writes delayed
by an IO limit never reach CBT, and incremental backups miss them.
Account the bio in CBT when the dispatch work issues it. The work runs
in process context with no locks held, so CBT may allocate there as it
does at submission.
https://virtuozzo.atlassian.net/browse/VSTOR-149268
Feature: cbt: changed block tracking (for backup)
Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
---
block/blk-throttle.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/block/blk-throttle.c b/block/blk-throttle.c
index 397b6a410f9e5..e1e008a30bf2c 100644
--- a/block/blk-throttle.c
+++ b/block/blk-throttle.c
@@ -1223,8 +1223,11 @@ static void blk_throtl_dispatch_work_fn(struct work_struct *work)
if (!bio_list_empty(&bio_list_on_stack)) {
blk_start_plug(&plug);
- while ((bio = bio_list_pop(&bio_list_on_stack)))
+ while ((bio = bio_list_pop(&bio_list_on_stack))) {
+ /* submit_bio_noacct() did not account it in CBT */
+ blk_cbt_bio_queue(q, bio);
submit_bio_noacct_nocheck(bio);
+ }
blk_finish_plug(&plug);
}
}
--
2.55.0
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Devel] [PATCH hci-8.1 v3 3/3] block/blk-cbt: account a write request also when dm starts it
2026-10-27 10:18 [Devel] [PATCH hci-8.1 v3 0/3] block/blk-cbt: don't lose writes held across a dm suspend Pavel Tikhomirov
2026-10-27 10:18 ` [Devel] [PATCH hci-8.1 v3 1/3] block/blk-cbt: shift the block size as unsigned Pavel Tikhomirov
2026-10-27 10:18 ` [Devel] [PATCH hci-8.1 v3 2/3] block/blk-throttle: account a throttled write in CBT Pavel Tikhomirov
@ 2026-10-27 10:18 ` Pavel Tikhomirov
2026-10-27 10:26 ` no-reply
2 siblings, 1 reply; 7+ messages in thread
From: Pavel Tikhomirov @ 2026-10-27 10:18 UTC (permalink / raw)
To: Konstantin Khorenko; +Cc: devel
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 when dm starts it. This runs under the dispatch
rcu_read_lock(), so it can't allocate: it sets the bits directly in the
bitmap pages, which are now allocated already at submission. A page can
still be missing only for a write submitted before BLKCBTSTART, then dm
returns BLK_STS_RESOURCE, so that blk-mq dispatches the request again
later, and the page is allocated from the kblockd workqueue.
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 | 177 +++++++++++++++++++++++++++++++++++++++++++++++++
drivers/md/dm-rq.c | 3 +
include/linux/blkdev.h | 5 ++
3 files changed, 185 insertions(+)
diff --git a/block/blk-cbt.c b/block/blk-cbt.c
index 1066b6a33b6b5..7e3c354941aae 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>
@@ -52,6 +53,10 @@ struct cbt_info {
struct page **snp_map;
blkcnt_t snp_block_max;
+ /* Map pages blk_cbt_rq_start() found missing, see cbt_alloc_work_fn() */
+ unsigned long *missing;
+ struct work_struct alloc_work;
+
spinlock_t lock;
struct list_head list;
};
@@ -221,6 +226,24 @@ static int __blk_cbt_set(struct cbt_info *cbt, blkcnt_t block,
return (pages_missed && *pages_missed) ? -EAGAIN : 0;
}
+/* Allocate the missing map pages for blocks [block, block + count) */
+static void cbt_alloc_pages(struct cbt_info *cbt, blkcnt_t block,
+ blkcnt_t count)
+{
+ unsigned long idx, last;
+
+ if (unlikely(!count || block + count > cbt->block_max))
+ return;
+
+ last = (block + count - 1) / BITS_PER_PAGE;
+ for (idx = block / BITS_PER_PAGE; idx <= last; idx++) {
+ struct cbt_info *c = cbt;
+
+ if (!CBT_PAGE(cbt, idx) && cbt_page_alloc(&c, idx, 1))
+ return;
+ }
+}
+
static void blk_cbt_add(struct cbt_info *cbt, blkcnt_t start, blkcnt_t len)
{
struct cbt_extent *ex;
@@ -236,6 +259,12 @@ static void blk_cbt_add(struct cbt_info *cbt, blkcnt_t start, blkcnt_t len)
__blk_cbt_set(cbt, start, len, 1, 1, NULL, NULL);
return;
}
+ /*
+ * Preallocate the pages for blk_cbt_rq_start(): it runs under the
+ * dispatch rcu_read_lock(), and a missing page there means delaying
+ * the request and allocating the page from the kblockd workqueue.
+ */
+ cbt_alloc_pages(cbt, start, len);
ex = get_cpu_ptr(cbt->cache);
if (ex->start + ex->len == start) {
ex->len += len;
@@ -266,6 +295,145 @@ void blk_cbt_bio_queue(struct request_queue *q, struct bio *bio)
rcu_read_unlock();
}
+/* Drop a cbt->count reference, release the cbt if it is dead */
+static void cbt_put(struct cbt_info *cbt)
+{
+ unsigned long flags;
+ bool release;
+
+ spin_lock_irqsave(&cbt->lock, flags);
+ release = !--cbt->count && test_bit(CBT_DEAD, &cbt->flags);
+ spin_unlock_irqrestore(&cbt->lock, flags);
+ if (release)
+ call_rcu(&cbt->rcu, &cbt_release_callback);
+}
+
+/* Allocate the map pages blk_cbt_rq_start() found missing */
+static void cbt_alloc_work_fn(struct work_struct *work)
+{
+ struct cbt_info *cbt = container_of(work, struct cbt_info, alloc_work);
+ unsigned long idx;
+
+ for_each_set_bit(idx, cbt->missing, NR_PAGES(cbt->block_max)) {
+ struct page *page;
+
+ clear_bit(idx, cbt->missing);
+ if (CBT_PAGE(cbt, idx))
+ continue;
+
+ page = alloc_page(GFP_NOIO | __GFP_ZERO);
+ spin_lock_irq(&cbt->lock);
+ if (!CBT_PAGE(cbt, idx) && !test_bit(CBT_DEAD, &cbt->flags)) {
+ if (page) {
+ cbt->map[idx] = page;
+ page = NULL;
+ } else {
+ set_bit(CBT_ERROR, &cbt->flags);
+ }
+ }
+ spin_unlock_irq(&cbt->lock);
+ if (page)
+ __free_page(page);
+ }
+ cbt_put(cbt);
+}
+
+static void cbt_queue_alloc(struct cbt_info *cbt, unsigned long idx)
+{
+ unsigned long flags;
+
+ set_bit(idx, cbt->missing);
+ spin_lock_irqsave(&cbt->lock, flags);
+ if (test_bit(CBT_DEAD, &cbt->flags)) {
+ spin_unlock_irqrestore(&cbt->lock, flags);
+ return;
+ }
+ cbt->count++;
+ spin_unlock_irqrestore(&cbt->lock, flags);
+
+ /* The queued work drops the reference, or drop it if already queued */
+ if (!kblockd_schedule_work(&cbt->alloc_work))
+ cbt_put(cbt);
+}
+
+/*
+ * Set the bits of [start, start + len) in the present pages, without
+ * allocating. Return true if a page is missing, its allocation is queued.
+ */
+static bool cbt_set_present(struct cbt_info *cbt, blkcnt_t start, blkcnt_t len)
+{
+ blkcnt_t block, end;
+ unsigned long flags;
+ bool missing = false;
+
+ end = DIV_ROUND_UP(start + len, 1UL << cbt->block_bits);
+ block = start >> cbt->block_bits;
+ if (unlikely(end > cbt->block_max)) {
+ set_bit(CBT_ERROR, &cbt->flags);
+ return false;
+ }
+
+ while (block < end) {
+ unsigned long idx = block / BITS_PER_PAGE;
+ unsigned long off = block & (BITS_PER_PAGE - 1);
+ unsigned long n = min_t(unsigned long, BITS_PER_PAGE - off,
+ end - block);
+ struct page *page = CBT_PAGE(cbt, idx);
+
+ if (page) {
+ local_irq_save(flags);
+ spin_lock_page(page);
+ set_bits(page_address(page), off, n, 1);
+ unlock_page(page);
+ local_irq_restore(flags);
+ } else {
+ cbt_queue_alloc(cbt, idx);
+ missing = true;
+ }
+ block += n;
+ }
+
+ return missing;
+}
+
+/*
+ * 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.
+ *
+ * Runs under the dispatch rcu_read_lock() (blk_mq_run_dispatch_ops()), so
+ * it must not sleep: if a bitmap page is missing, its allocation is queued
+ * and true is returned, for dm to retry the request later.
+ */
+bool blk_cbt_rq_start(struct request *rq)
+{
+ struct request_queue *q = rq->q;
+ struct cbt_info *cbt;
+ struct bio *bio;
+ bool retry = false;
+
+ if (likely(list_empty(&q->cbt_list)))
+ return false;
+ if (rq_data_dir(rq) == READ || blk_rq_is_passthrough(rq))
+ return false;
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(cbt, &q->cbt_list, list) {
+ if (unlikely(test_bit(CBT_ERROR, &cbt->flags)))
+ continue;
+ __rq_for_each_bio(bio, rq) {
+ if (bio->bi_iter.bi_size &&
+ cbt_set_present(cbt, bio->bi_iter.bi_sector << 9,
+ bio->bi_iter.bi_size))
+ retry = true;
+ }
+ }
+ rcu_read_unlock();
+
+ return retry;
+}
+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)
{
@@ -288,8 +456,16 @@ static struct cbt_info *do_cbt_alloc(struct request_queue *q, __u8 *name,
goto err_pcpu;
memset(cbt->map, 0, NR_PAGES(cbt->block_max) * sizeof(void*));
+
+ cbt->missing = bitmap_zalloc(NR_PAGES(cbt->block_max), GFP_KERNEL);
+ if (!cbt->missing)
+ goto err_map;
+ INIT_WORK(&cbt->alloc_work, cbt_alloc_work_fn);
+
cbt->queue = q;
return cbt;
+err_map:
+ vfree(cbt->map);
err_pcpu:
free_percpu(cbt->cache);
err_cbt:
@@ -671,6 +847,7 @@ static void cbt_release_callback(struct rcu_head *head)
free_map(cbt->snp_map, nr_pages);
}
+ bitmap_free(cbt->missing);
free_percpu(cbt->cache);
kfree(cbt);
}
diff --git a/drivers/md/dm-rq.c b/drivers/md/dm-rq.c
index 20f495bf5399d..5a8cd59ec257f 100644
--- a/drivers/md/dm-rq.c
+++ b/drivers/md/dm-rq.c
@@ -522,6 +522,9 @@ static blk_status_t dm_mq_queue_rq(struct blk_mq_hw_ctx *hctx,
if (ti->type->busy && ti->type->busy(ti))
return BLK_STS_RESOURCE;
+ if (blk_cbt_rq_start(rq))
+ return BLK_STS_RESOURCE;
+
dm_start_request(md, rq);
/* Init tio using md established in .init_request */
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 1284bb0e4ebc0..d76a0c9be28f0 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 bool 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,10 @@ 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 bool blk_cbt_rq_start(struct request *rq)
+{
+ return false;
+}
static inline int blk_cbt_ioctl(struct block_device *bdev, unsigned cmd,
char __user *arg)
{
--
2.55.0
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Devel] [PATCH hci-8.1 v3 2/3] block/blk-throttle: account a throttled write in CBT
2026-10-27 10:18 ` [Devel] [PATCH hci-8.1 v3 2/3] block/blk-throttle: account a throttled write in CBT Pavel Tikhomirov
@ 2026-10-27 10:22 ` no-reply
0 siblings, 0 replies; 7+ messages in thread
From: no-reply @ 2026-10-27 10:22 UTC (permalink / raw)
To: Pavel Tikhomirov, Konstantin Khorenko; +Cc: devel
> submit_bio_noacct() accounts a write in CBT only when blk-throttle does
> not hold it: a throttled bio is later issued from the throttle dispatch
> work with submit_bio_noacct_nocheck(), which skips CBT. So writes delayed
> by an IO limit never reach CBT, and incremental backups miss them.
>
> Account the bio in CBT when the dispatch work issues it. The work runs
> in process context with no locks held, so CBT may allocate there as it
> does at submission.
>
> https://virtuozzo.atlassian.net/browse/VSTOR-149268
> Feature: cbt: changed block tracking (for backup)
> Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261027-ptikhomirov-vstor-149268-cbt-account-at-rq-start-v3-0-e1bc61a829a0@virtuozzo.com?part=2
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Devel] [PATCH hci-8.1 v3 1/3] block/blk-cbt: shift the block size as unsigned
2026-10-27 10:18 ` [Devel] [PATCH hci-8.1 v3 1/3] block/blk-cbt: shift the block size as unsigned Pavel Tikhomirov
@ 2026-10-27 10:22 ` no-reply
0 siblings, 0 replies; 7+ messages in thread
From: no-reply @ 2026-10-27 10:22 UTC (permalink / raw)
To: Pavel Tikhomirov, Konstantin Khorenko; +Cc: devel
> cbt_ioc_init() accepts any power of two block size, so block_bits can be
> 31, and then 1 << block_bits overflows int: the sign extended block size
> makes the rounded up block counts wrong. Shift an unsigned one instead.
>
> https://virtuozzo.atlassian.net/browse/VSTOR-149268
> Feature: cbt: changed block tracking (for backup)
> Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261027-ptikhomirov-vstor-149268-cbt-account-at-rq-start-v3-0-e1bc61a829a0@virtuozzo.com?part=1
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Devel] [PATCH hci-8.1 v3 3/3] block/blk-cbt: account a write request also when dm starts it
2026-10-27 10:18 ` [Devel] [PATCH hci-8.1 v3 3/3] block/blk-cbt: account a write request also when dm starts it Pavel Tikhomirov
@ 2026-10-27 10:26 ` no-reply
0 siblings, 0 replies; 7+ messages in thread
From: no-reply @ 2026-10-27 10:26 UTC (permalink / raw)
To: Konstantin Khorenko, Pavel Tikhomirov; +Cc: devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] block: blk_cbt_add() sleeps in cbt_page_alloc() mid RCU list walk, successor can be freed
--
commit 373148515c83f51a5e87240494ce9cc20409c1c2
Author: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
block/blk-cbt: account a write request also when dm starts it
This patch accounts a write in CBT a second time when request based dm
starts the request, so that a request submitted before a dm suspend but
dispatched after resume is still tracked. To make this possible without
allocating under the dispatch RCU read lock, bitmap pages are now
preallocated at bio submission time in blk_cbt_add(). If a page is still
missing at dm start time, dm returns BLK_STS_RESOURCE and the page is
allocated from kblockd.
Link: https://virtuozzo.atlassian.net/browse/VSTOR-149268
> diff --git a/block/blk-cbt.c b/block/blk-cbt.c
> --- a/block/blk-cbt.c
> +++ b/block/blk-cbt.c
[ ... ]
> +/* Allocate the missing map pages for blocks [block, block + count) */
> +static void cbt_alloc_pages(struct cbt_info *cbt, blkcnt_t block,
> + blkcnt_t count)
> +{
> + unsigned long idx, last;
> +
> + if (unlikely(!count || block + count > cbt->block_max))
> + return;
> +
> + last = (block + count - 1) / BITS_PER_PAGE;
> + for (idx = block / BITS_PER_PAGE; idx <= last; idx++) {
> + struct cbt_info *c = cbt;
> +
[Severity: Medium]
> + if (!CBT_PAGE(cbt, idx) && cbt_page_alloc(&c, idx, 1))
> + return;
Can this sleep in the middle of the RCU list walk in blk_cbt_bio_queue()?
Passing in_rcu=1 makes cbt_page_alloc() do:
if (in_rcu)
rcu_read_unlock();
page = alloc_page(GFP_NOIO|__GFP_ZERO);
if (in_rcu)
rcu_read_lock();
The call chain is:
blk_cbt_bio_queue()
list_for_each_entry_rcu(cbt, &q->cbt_list, list)
blk_cbt_add()
cbt_alloc_pages()
cbt_page_alloc()
The only thing pinned across the sleep is cbt->count of the current cbt,
which does not protect the next list entry. If the current cbt is
unlinked in that window, for example by blk_cbt_del() with
list_del_rcu(), its list.next is left unchanged. If the successor is also
removed and a grace period elapses (possible because this task holds no
RCU read lock while sleeping), the iterator then follows the stale next
pointer into freed memory.
This needs at least two CBTs on the queue and a concurrent stop or resize
operation, so the window is narrow. A similar shape already existed for
the eviction flush in blk_cbt_add() with in_rcu=1. However, the new call
extends it to the first write touching any bitmap page that is not yet
allocated, on every write submission.
The new allocation also ignores REQ_NOWAIT semantics on the submission
path. Would it be safer to avoid dropping the RCU read lock inside the
list walk, for example by deferring the allocation the way
cbt_queue_alloc() does?
> + }
> +}
> +
> static void blk_cbt_add(struct cbt_info *cbt, blkcnt_t start, blkcnt_t len)
[ ... ]
> + /*
> + * Preallocate the pages for blk_cbt_rq_start(): it runs under the
> + * dispatch rcu_read_lock(), and a missing page there means delaying
> + * the request and allocating the page from the kblockd workqueue.
> + */
> + cbt_alloc_pages(cbt, start, len);
This is the call site in blk_cbt_add() that the comment above refers to.
It is reached from blk_cbt_bio_queue() under rcu_read_lock() while
iterating q->cbt_list.
Thanks,
--
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261027-ptikhomirov-vstor-149268-cbt-account-at-rq-start-v3-0-e1bc61a829a0@virtuozzo.com?part=3
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-10-27 10:28 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-27 10:18 [Devel] [PATCH hci-8.1 v3 0/3] block/blk-cbt: don't lose writes held across a dm suspend Pavel Tikhomirov
2026-10-27 10:18 ` [Devel] [PATCH hci-8.1 v3 1/3] block/blk-cbt: shift the block size as unsigned Pavel Tikhomirov
2026-10-27 10:22 ` no-reply
2026-10-27 10:18 ` [Devel] [PATCH hci-8.1 v3 2/3] block/blk-throttle: account a throttled write in CBT Pavel Tikhomirov
2026-10-27 10:22 ` no-reply
2026-10-27 10:18 ` [Devel] [PATCH hci-8.1 v3 3/3] block/blk-cbt: account a write request also when dm starts it Pavel Tikhomirov
2026-10-27 10:26 ` no-reply
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.