* [Devel] [PATCH hci-8.1 v5 0/7] dm/push_backup: event fd interface and fixes
@ 2026-11-03 12:37 Andrey Zhadchenko
2026-11-03 12:37 ` [Devel] [PATCH hci-8.1 v5 1/7] drivers/md/push_backup: correct initialization Andrey Zhadchenko
` (7 more replies)
0 siblings, 8 replies; 17+ messages in thread
From: Andrey Zhadchenko @ 2026-11-03 12:37 UTC (permalink / raw)
To: khorenko, alexander.ivanov; +Cc: devel
The push_backup target currently makes the backup tool wait for the next
cluster range inside the "push_backup_read" target message. dm runs
target_message() under dm_get_live_table(), i.e. with md->io_barrier SRCU
read-locked, so while the tool sleeps there anything that needs
synchronize_srcu(&md->io_barrier) on that device is stuck: suspend, table
reload/remove and dm_internal_suspend_fast() from "@stats_create". An
unrelated dm-stats user hangs, and everything that later queues behind
md->suspend_lock hangs with it.
Patch 7 adds a file descriptor based interface instead. The tool gets an
anon inode fd via the "push_backup_get_fd" message and then uses plain
read()/write() of struct dm_push_backup_range to fetch the next range and
to acknowledge it, without going through dm messages on the data path. The
fd pins struct push_backup with a kref so it survives target removal and
simply fails with ENODEV afterwards. The fd is bound to the target, not to
a backup session. The legacy messages are kept for compatibility.
Patches 1-6 are fixes found while working on this:
1. init the lock, waitqueue and timer before anything that can fail so
pb_destroy() is safe on every error path of pb_ctr();
2. return -ENOMEM when the workqueue allocation fails. pb_ctr()
returned 0 after freeing pb, and pb_dtr() freed it again;
3. clear pb->alive under pb->lock like the other users do;
4. pass absolute expiry to mod_timer(). All callers passed a relative
value, which is always in the past, so the timer fired and re-armed
on every tick while a write was delayed;
5. arm the timer under pb->lock and wait for its callback properly.
pb_timer_func() and postpone_if_required_for_backup() both decided
under the lock and armed after dropping it, so the callback's stale
S64_MAX could overwrite a fresh deadline while writes were delayed.
timer_delete_sync_try() does not wait for a running callback, so a
firing after the synchronize_rcu() grace period could outlive
push_backup_stop() and expire a later session, or run under
pb_destroy() and use pb after it is freed. With arming done under
the lock, the callback checking pb->nr_delayed and stop/destroy
waiting via timer_delete_sync()/timer_shutdown_sync(), the RCU dance
between postpone and stop is no longer needed and is removed;
6. drop the redundant mod_timer() in push_backup_write(). The timer
re-arms itself from pb->deadline_jiffies while writes are delayed,
so updating the deadline under the lock is enough. The call was not
under pb->lock, which matters once writes may arrive via the fd
without pb->ctl_rwsem.
v5:
- 1/7: commit message: safe on every error path of pb_ctr(), the
zeroed timer triggers an ODEBUG warning on debug kernels.
- 2/7: new patch.
- 4/7, 5/7, 6/7: reordered so that the timer is armed under pb->lock
before push_backup_write() stops re-arming it; no bisect point has
the race window. 4/7 now also fixes the mod_timer() call in
push_backup_write().
- 4/7: commit message: the relative expiry is always in the past.
- 5/7: pb_timer_func() checks pb->nr_delayed instead of pb->alive, so
a "push_backup_stop" interrupted before taking pb->ctl_rwsem still
expires the deadline. Commit message describes the race, the
synchronize_rcu() window and the pb_destroy() use-after-free. Added
Fixes: tag.
- 6/7: dropped the "only place" claim.
- 7/7: header documents that the fd survives stop/start, errno
semantics, the EINTR/EFAULT/EBADMSG and write() errors, O_CLOEXEC
and that "dmsetup message" is of no use. pb_destroy() NULLs the
pointers it frees. checkpatch blank line. Commit message mentions
the legacy u64 parsing changes and the version bump, and corrects
the resubmit_pbios() rationale.
Patch numbers below refer to the 6-patch v4 numbering.
v4:
- 3/6, 6/6: commit message typos.
- 4/6: added Fixes: tag.
v3:
- 5/6: pb_timer_func() bails out when !pb->alive; push_backup_stop()
uses timer_delete_sync() and pb_destroy() timer_shutdown_sync()
instead of WARNing on timer_delete_sync_try() < 0, which never waited
for a running callback. Commit message corrected accordingly.
- 6/6: push_backup_get_fd() installs the fd only after its number fits
into the result buffer (reserve -> emit -> check -> install), so a
too small buffer no longer leaks the fd with its pb kref and module
reference. pb->dead renamed to pb->destroyed. UAPI type namespaced
as struct dm_push_backup_range. read() now requires exactly
sizeof(range) like write(). Subject prefix aligned with the rest of
the series.
v2:
- 2/6: reworded the commit message, dropped the KCSAN claim.
- 5/6: new patch: arm the deadline timer under pb->lock and drop the
rcu_read_lock()/synchronize_rcu() between
postpone_if_required_for_backup() and push_backup_stop().
- 6/6: resubmit_pbios() now frees the pbio before
dm_requeue_original_rq(). The requeue drops the md reference held by
the in-flight request, after which __dm_destroy() may run .dtr and
destroy pbio_pool under the fd writer's feet.
https://virtuozzo.atlassian.net/browse/VSTOR-152912
Andrey Zhadchenko (7):
drivers/md/push_backup: correct initialization
drivers/md/push_backup: return error if workqueue allocation fails
drivers/md/push_backup: clear pb->alive under pb->lock
drivers/md/push_backup: fix mod_timer() usage
drivers/md/push_backup: arm deadline timer under pb->lock
drivers/md/push_backup: do not re-arm deadline timer in
push_backup_write()
drivers/md/push_backup: add event file descriptor interface
drivers/md/dm-push-backup.c | 275 ++++++++++++++++++++++------
include/uapi/linux/dm-push-backup.h | 48 +++++
2 files changed, 262 insertions(+), 61 deletions(-)
create mode 100644 include/uapi/linux/dm-push-backup.h
--
2.43.5
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
^ permalink raw reply [flat|nested] 17+ messages in thread* [Devel] [PATCH hci-8.1 v5 1/7] drivers/md/push_backup: correct initialization 2026-11-03 12:37 [Devel] [PATCH hci-8.1 v5 0/7] dm/push_backup: event fd interface and fixes Andrey Zhadchenko @ 2026-11-03 12:37 ` Andrey Zhadchenko 2026-11-03 12:46 ` no-reply 2026-11-03 12:37 ` [Devel] [PATCH hci-8.1 v5 2/7] drivers/md/push_backup: return error if workqueue allocation fails Andrey Zhadchenko ` (6 subsequent siblings) 7 siblings, 1 reply; 17+ messages in thread From: Andrey Zhadchenko @ 2026-11-03 12:37 UTC (permalink / raw) To: khorenko, alexander.ivanov; +Cc: devel Allocate pool and htable after lock, waitqueue and deadline timer init so that pb_destroy() is safe on every error path of pb_ctr(). Otherwise a failed allocation makes pb_destroy() call timer_delete_sync_try() on a zeroed timer struct. This happens to work on release kernels, but with CONFIG_DEBUG_OBJECTS_TIMERS=y debug_assert_init() reports the uninitialized timer with an ODEBUG warning. https://virtuozzo.atlassian.net/browse/VSTOR-152912 Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com> Reviewed-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com> --- v2: no changes v3: no changes v4: no changes v5: commit message: safe on every error path of pb_ctr(); the zeroed timer triggers an ODEBUG warning with CONFIG_DEBUG_OBJECTS_TIMERS=y drivers/md/dm-push-backup.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/drivers/md/dm-push-backup.c b/drivers/md/dm-push-backup.c index 885ea7b31cf67..9a6252f2b21e2 100644 --- a/drivers/md/dm-push-backup.c +++ b/drivers/md/dm-push-backup.c @@ -658,24 +658,22 @@ static int pb_ctr(struct dm_target *ti, unsigned int argc, char **argv) pb = kzalloc(sizeof(*pb), GFP_KERNEL); if (!pb) goto err; - - pb->pbio_pool = mempool_create_slab_pool(PUSH_BACKUP_POOL_SIZE, - pbio_cache); - pb->pending_htable = kcalloc(PB_HASH_TABLE_SIZE, - sizeof(struct hlist_head), - GFP_KERNEL); - if (!pb->pbio_pool || !pb->pending_htable) - goto err; - spin_lock_init(&pb->lock); init_rwsem(&pb->ctl_rwsem); bio_list_init(&pb->deferred_bios); INIT_LIST_HEAD(&pb->pending); timer_setup(&pb->deadline_timer, pb_timer_func, 0); - init_waitqueue_head(&pb->waitq); INIT_WORK(&pb->worker, do_pb_work); + pb->pbio_pool = mempool_create_slab_pool(PUSH_BACKUP_POOL_SIZE, + pbio_cache); + pb->pending_htable = kcalloc(PB_HASH_TABLE_SIZE, + sizeof(struct hlist_head), + GFP_KERNEL); + if (!pb->pbio_pool || !pb->pending_htable) + goto err; + ti->private = pb; pb->ti = ti; -- 2.43.5 _______________________________________________ Devel mailing list Devel@openvz.org https://lists.openvz.org/mailman/listinfo/devel ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Devel] [PATCH hci-8.1 v5 1/7] drivers/md/push_backup: correct initialization 2026-11-03 12:37 ` [Devel] [PATCH hci-8.1 v5 1/7] drivers/md/push_backup: correct initialization Andrey Zhadchenko @ 2026-11-03 12:46 ` no-reply 0 siblings, 0 replies; 17+ messages in thread From: no-reply @ 2026-11-03 12:46 UTC (permalink / raw) To: alexander.ivanov, Andrey Zhadchenko, khorenko; +Cc: devel > Allocate pool and htable after lock, waitqueue and deadline > timer init so that pb_destroy() is safe on every error path > of pb_ctr(). Otherwise a failed allocation makes pb_destroy() > call timer_delete_sync_try() on a zeroed timer struct. This > happens to work on release kernels, but with > CONFIG_DEBUG_OBJECTS_TIMERS=y debug_assert_init() reports the > uninitialized timer with an ODEBUG warning. > > https://virtuozzo.atlassian.net/browse/VSTOR-152912 > Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com> Sashiko has reviewed this patch and found no issues. It looks great! -- Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261103123802.846676-1-andrey.zhadchenko@virtuozzo.com?part=1 _______________________________________________ Devel mailing list Devel@openvz.org https://lists.openvz.org/mailman/listinfo/devel ^ permalink raw reply [flat|nested] 17+ messages in thread
* [Devel] [PATCH hci-8.1 v5 2/7] drivers/md/push_backup: return error if workqueue allocation fails 2026-11-03 12:37 [Devel] [PATCH hci-8.1 v5 0/7] dm/push_backup: event fd interface and fixes Andrey Zhadchenko 2026-11-03 12:37 ` [Devel] [PATCH hci-8.1 v5 1/7] drivers/md/push_backup: correct initialization Andrey Zhadchenko @ 2026-11-03 12:37 ` Andrey Zhadchenko 2026-11-03 12:45 ` no-reply 2026-11-03 12:37 ` [Devel] [PATCH hci-8.1 v5 3/7] drivers/md/push_backup: clear pb->alive under pb->lock Andrey Zhadchenko ` (5 subsequent siblings) 7 siblings, 1 reply; 17+ messages in thread From: Andrey Zhadchenko @ 2026-11-03 12:37 UTC (permalink / raw) To: khorenko, alexander.ivanov; +Cc: devel ret is still 0 from dm_get_device() when alloc_ordered_workqueue() fails, so pb_ctr() frees pb and returns success. The table keeps ti->private pointing to freed memory, and pb_dtr() destroys it a second time. Fixes: 62e07487348e ("dm: Add push backup driver") https://virtuozzo.atlassian.net/browse/VSTOR-152912 Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com> --- v5: new patch drivers/md/dm-push-backup.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/md/dm-push-backup.c b/drivers/md/dm-push-backup.c index 9a6252f2b21e2..48d6d6a59daf4 100644 --- a/drivers/md/dm-push-backup.c +++ b/drivers/md/dm-push-backup.c @@ -698,6 +698,7 @@ static int pb_ctr(struct dm_target *ti, unsigned int argc, char **argv) pb->wq = alloc_ordered_workqueue("dm-" DM_MSG_PREFIX, WQ_MEM_RECLAIM); if (!pb->wq) { + ret = -ENOMEM; ti->error = "could not create workqueue for metadata object"; goto err; } -- 2.43.5 _______________________________________________ Devel mailing list Devel@openvz.org https://lists.openvz.org/mailman/listinfo/devel ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Devel] [PATCH hci-8.1 v5 2/7] drivers/md/push_backup: return error if workqueue allocation fails 2026-11-03 12:37 ` [Devel] [PATCH hci-8.1 v5 2/7] drivers/md/push_backup: return error if workqueue allocation fails Andrey Zhadchenko @ 2026-11-03 12:45 ` no-reply 0 siblings, 0 replies; 17+ messages in thread From: no-reply @ 2026-11-03 12:45 UTC (permalink / raw) To: Andrey Zhadchenko, khorenko, alexander.ivanov; +Cc: devel > ret is still 0 from dm_get_device() when alloc_ordered_workqueue() > fails, so pb_ctr() frees pb and returns success. The table keeps > ti->private pointing to freed memory, and pb_dtr() destroys it a > second time. > > Fixes: 62e07487348e ("dm: Add push backup driver") > https://virtuozzo.atlassian.net/browse/VSTOR-152912 > Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com> Sashiko has reviewed this patch and found no issues. It looks great! -- Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261103123802.846676-1-andrey.zhadchenko@virtuozzo.com?part=2 _______________________________________________ Devel mailing list Devel@openvz.org https://lists.openvz.org/mailman/listinfo/devel ^ permalink raw reply [flat|nested] 17+ messages in thread
* [Devel] [PATCH hci-8.1 v5 3/7] drivers/md/push_backup: clear pb->alive under pb->lock 2026-11-03 12:37 [Devel] [PATCH hci-8.1 v5 0/7] dm/push_backup: event fd interface and fixes Andrey Zhadchenko 2026-11-03 12:37 ` [Devel] [PATCH hci-8.1 v5 1/7] drivers/md/push_backup: correct initialization Andrey Zhadchenko 2026-11-03 12:37 ` [Devel] [PATCH hci-8.1 v5 2/7] drivers/md/push_backup: return error if workqueue allocation fails Andrey Zhadchenko @ 2026-11-03 12:37 ` Andrey Zhadchenko 2026-11-03 12:46 ` no-reply 2026-11-03 12:37 ` [Devel] [PATCH hci-8.1 v5 4/7] drivers/md/push_backup: fix mod_timer() usage Andrey Zhadchenko ` (4 subsequent siblings) 7 siblings, 1 reply; 17+ messages in thread From: Andrey Zhadchenko @ 2026-11-03 12:37 UTC (permalink / raw) To: khorenko, alexander.ivanov; +Cc: devel Most other users handle this under the pb->lock so let's make it a bit more consistent. https://virtuozzo.atlassian.net/browse/VSTOR-152912 Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com> Reviewed-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com> --- v2: reworded the commit message, dropped the KCSAN claim v3: no changes v4: no changes v5: no changes drivers/md/dm-push-backup.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/md/dm-push-backup.c b/drivers/md/dm-push-backup.c index 48d6d6a59daf4..5c4f3d625fb82 100644 --- a/drivers/md/dm-push-backup.c +++ b/drivers/md/dm-push-backup.c @@ -581,7 +581,9 @@ static int pb_message(struct dm_target *ti, unsigned int argc, char **argv, if (!pb->map) return -EBADF; + spin_lock_irq(&pb->lock); pb->alive = false; + spin_unlock_irq(&pb->lock); wake_up_interruptible(&pb->waitq); /* pb->alive = false */ } -- 2.43.5 _______________________________________________ Devel mailing list Devel@openvz.org https://lists.openvz.org/mailman/listinfo/devel ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Devel] [PATCH hci-8.1 v5 3/7] drivers/md/push_backup: clear pb->alive under pb->lock 2026-11-03 12:37 ` [Devel] [PATCH hci-8.1 v5 3/7] drivers/md/push_backup: clear pb->alive under pb->lock Andrey Zhadchenko @ 2026-11-03 12:46 ` no-reply 0 siblings, 0 replies; 17+ messages in thread From: no-reply @ 2026-11-03 12:46 UTC (permalink / raw) To: khorenko, alexander.ivanov, Andrey Zhadchenko; +Cc: devel > Most other users handle this under the pb->lock so let's > make it a bit more consistent. > > https://virtuozzo.atlassian.net/browse/VSTOR-152912 > Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com> Sashiko has reviewed this patch and found no issues. It looks great! -- Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261103123802.846676-1-andrey.zhadchenko@virtuozzo.com?part=3 _______________________________________________ Devel mailing list Devel@openvz.org https://lists.openvz.org/mailman/listinfo/devel ^ permalink raw reply [flat|nested] 17+ messages in thread
* [Devel] [PATCH hci-8.1 v5 4/7] drivers/md/push_backup: fix mod_timer() usage 2026-11-03 12:37 [Devel] [PATCH hci-8.1 v5 0/7] dm/push_backup: event fd interface and fixes Andrey Zhadchenko ` (2 preceding siblings ...) 2026-11-03 12:37 ` [Devel] [PATCH hci-8.1 v5 3/7] drivers/md/push_backup: clear pb->alive under pb->lock Andrey Zhadchenko @ 2026-11-03 12:37 ` Andrey Zhadchenko 2026-11-03 12:47 ` no-reply 2026-11-03 12:38 ` [Devel] [PATCH hci-8.1 v5 5/7] drivers/md/push_backup: arm deadline timer under pb->lock Andrey Zhadchenko ` (3 subsequent siblings) 7 siblings, 1 reply; 17+ messages in thread From: Andrey Zhadchenko @ 2026-11-03 12:37 UTC (permalink / raw) To: khorenko, alexander.ivanov; +Cc: devel mod_timer() takes an absolute jiffies value, but all callers pass a relative one, which is always in the past. The timer fires and re-arms on every tick while a write is delayed. It has worked so far because pb_timer_func() checks pb->deadline_jiffies each time. Fixes: 62e07487348e ("dm: Add push backup driver") https://virtuozzo.atlassian.net/browse/VSTOR-152912 Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com> Reviewed-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com> --- v2: no changes v3: no changes v4: added Fixes: tag v5: moved before "do not re-arm deadline timer", so it also fixes the mod_timer() in push_backup_write(); commit message: the relative expiry is always in the past, not only after 5 min drivers/md/dm-push-backup.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/md/dm-push-backup.c b/drivers/md/dm-push-backup.c index 5c4f3d625fb82..b6d1629245235 100644 --- a/drivers/md/dm-push-backup.c +++ b/drivers/md/dm-push-backup.c @@ -244,7 +244,7 @@ static void pb_timer_func(struct timer_list *timer) spin_unlock_irqrestore(&pb->lock, flags); if (unlikely(time_before64(now, deadline))) - mod_timer(timer, deadline - now + 1); + mod_timer(timer, jiffies + (deadline - now) + 1); else queue_work(pb->wq, &pb->worker); } @@ -278,7 +278,7 @@ static int postpone_if_required_for_backup(struct push_backup *pb, spin_unlock_irqrestore(&pb->lock, flags); if (queue_timer) - mod_timer(&pb->deadline_timer, pb->timeout_in_jiffies + 1); + mod_timer(&pb->deadline_timer, jiffies + pb->timeout_in_jiffies + 1); rcu_read_unlock(); if (queue_timer) @@ -532,7 +532,7 @@ static int push_backup_write(struct push_backup *pb, if (!list_empty(&pbio_list)) { resubmit_pbios(pb, &pbio_list); if (has_more) - mod_timer(&pb->deadline_timer, pb->timeout_in_jiffies + 1); + mod_timer(&pb->deadline_timer, jiffies + pb->timeout_in_jiffies + 1); } return 0; -- 2.43.5 _______________________________________________ Devel mailing list Devel@openvz.org https://lists.openvz.org/mailman/listinfo/devel ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Devel] [PATCH hci-8.1 v5 4/7] drivers/md/push_backup: fix mod_timer() usage 2026-11-03 12:37 ` [Devel] [PATCH hci-8.1 v5 4/7] drivers/md/push_backup: fix mod_timer() usage Andrey Zhadchenko @ 2026-11-03 12:47 ` no-reply 0 siblings, 0 replies; 17+ messages in thread From: no-reply @ 2026-11-03 12:47 UTC (permalink / raw) To: Andrey Zhadchenko, khorenko, alexander.ivanov; +Cc: devel > mod_timer() takes an absolute jiffies value, but all callers pass a > relative one, which is always in the past. The timer fires and re-arms > on every tick while a write is delayed. It has worked so far because > pb_timer_func() checks pb->deadline_jiffies each time. > > Fixes: 62e07487348e ("dm: Add push backup driver") > https://virtuozzo.atlassian.net/browse/VSTOR-152912 > Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com> Sashiko has reviewed this patch and found no issues. It looks great! -- Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261103123802.846676-1-andrey.zhadchenko@virtuozzo.com?part=4 _______________________________________________ Devel mailing list Devel@openvz.org https://lists.openvz.org/mailman/listinfo/devel ^ permalink raw reply [flat|nested] 17+ messages in thread
* [Devel] [PATCH hci-8.1 v5 5/7] drivers/md/push_backup: arm deadline timer under pb->lock 2026-11-03 12:37 [Devel] [PATCH hci-8.1 v5 0/7] dm/push_backup: event fd interface and fixes Andrey Zhadchenko ` (3 preceding siblings ...) 2026-11-03 12:37 ` [Devel] [PATCH hci-8.1 v5 4/7] drivers/md/push_backup: fix mod_timer() usage Andrey Zhadchenko @ 2026-11-03 12:38 ` Andrey Zhadchenko 2026-11-03 12:48 ` no-reply 2026-11-03 12:38 ` [Devel] [PATCH hci-8.1 v5 6/7] drivers/md/push_backup: do not re-arm deadline timer in push_backup_write() Andrey Zhadchenko ` (2 subsequent siblings) 7 siblings, 1 reply; 17+ messages in thread From: Andrey Zhadchenko @ 2026-11-03 12:38 UTC (permalink / raw) To: khorenko, alexander.ivanov; +Cc: devel Both pb_timer_func() and postpone_if_required_for_backup() decide whether to arm the timer under pb->lock but call mod_timer() after dropping it, so the decisions can be applied out of order: CPU1 pb_timer_func() CPU2 postpone_if_required_for_backup() lock; deadline = S64_MAX unlock lock; nr_delayed = 1 deadline_jiffies = now + timeout unlock mod_timer(timeout) mod_timer(from S64_MAX) The late mod_timer() of the callback overwrites the fresh deadline, and the write is delayed with the timer parked in a very distant future. Only a later push_backup_write() that leaves requests delayed re-arms it; if the backup tool stops acknowledging ranges, the backup never expires. Make the decision and the arming atomic: call mod_timer() under pb->lock at both sites, and do not re-arm when nothing is delayed. mod_timer() and queue_work() are fine under a spinlock, and the timer base lock is dropped while the callback runs, so there is no lock inversion. The callback checks pb->nr_delayed rather than pb->alive on purpose. pb_message() clears pb->alive for "push_backup_stop" before taking pb->ctl_rwsem, and if the tool gets a fatal signal while waiting for the rwsem, push_backup_stop() never runs. The deadline must still expire then, so that do_pb_work() releases the delayed writes. cleanup_backup() zeroes nr_delayed under the same lock hold that clears alive, and postpone only delays while alive, so after stop the callback neither re-arms nor queues work. While alive, nr_delayed == 0 is the same as deadline_jiffies == S64_MAX. The callback racing with push_backup_stop() was never handled either. synchronize_rcu() waits for a callback that is already running when it starts, since the timer softirq is an RCU reader, but not for a firing that starts after the grace period. timer_delete_sync_try() is try_to_del_timer_sync(), it returns -1 without waiting when the callback is running, and we only WARN on that. Such a callback could re-arm the timer or queue do_pb_work() after flush_workqueue(), expiring a later session. The same happens in pb_destroy(), which WARNs and goes on to destroy_workqueue() and kfree(pb) under the running callback: a use-after-free. Use timer_delete_sync() in push_backup_stop() and timer_shutdown_sync() in pb_destroy() to wait for it; the work it may have queued is flushed by flush_workqueue()/destroy_workqueue() which follow. As a bonus the rcu_read_lock()/synchronize_rcu() dance between postpone_if_required_for_backup() and push_backup_stop() is no longer needed: the timer is only armed under pb->lock, by postpone while pb->alive is set and by the callback while requests are delayed, both of which cleanup_backup() ends under pb->lock, and the callback itself is waited for. Drop it. push_backup_write() still re-arms the timer outside pb->lock, but it is serialized with push_backup_stop() by pb->ctl_rwsem; the next patch removes that call. Fixes: 62e07487348e ("dm: Add push backup driver") https://virtuozzo.atlassian.net/browse/VSTOR-152912 Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com> Reviewed-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com> --- v2: new patch v3: bail out of pb_timer_func() when !pb->alive; wait for a running callback with timer_delete_sync() in push_backup_stop() and timer_shutdown_sync() in pb_destroy() instead of WARNing on timer_delete_sync_try() < 0; fixed the commit message claim about it v4: no changes v5: moved before "do not re-arm deadline timer"; pb_timer_func() checks pb->nr_delayed instead of pb->alive, so an interrupted "push_backup_stop" still expires the deadline; commit message describes the race instead of the harmless parked timer, the synchronize_rcu() window precisely and the pb_destroy() use-after-free; added Fixes: tag drivers/md/dm-push-backup.c | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/drivers/md/dm-push-backup.c b/drivers/md/dm-push-backup.c index b6d1629245235..f0de26add11c2 100644 --- a/drivers/md/dm-push-backup.c +++ b/drivers/md/dm-push-backup.c @@ -241,23 +241,24 @@ static void pb_timer_func(struct timer_list *timer) spin_lock_irqsave(&pb->lock, flags); deadline = pb->deadline_jiffies; - spin_unlock_irqrestore(&pb->lock, flags); - + if (!pb->nr_delayed) + goto unlock; if (unlikely(time_before64(now, deadline))) mod_timer(timer, jiffies + (deadline - now) + 1); else queue_work(pb->wq, &pb->worker); +unlock: + spin_unlock_irqrestore(&pb->lock, flags); } static int postpone_if_required_for_backup(struct push_backup *pb, struct request *rq) { - bool queue_timer = false; struct pb_bio *pbio; unsigned long flags; + bool wake = false; int ret = 0; - rcu_read_lock(); /* See push_backup_stop() */ spin_lock_irqsave(&pb->lock, flags); if (!pb->alive) goto unlock; @@ -272,16 +273,13 @@ static int postpone_if_required_for_backup(struct push_backup *pb, pb->nr_delayed += 1; if (pb->nr_delayed == 1) { pb->deadline_jiffies = get_jiffies_64() + pb->timeout_in_jiffies; - queue_timer = true; + mod_timer(&pb->deadline_timer, jiffies + pb->timeout_in_jiffies + 1); + wake = true; } unlock: spin_unlock_irqrestore(&pb->lock, flags); - if (queue_timer) - mod_timer(&pb->deadline_timer, jiffies + pb->timeout_in_jiffies + 1); - rcu_read_unlock(); - - if (queue_timer) + if (wake) wake_up_interruptible(&pb->waitq); return ret; @@ -400,9 +398,7 @@ static int push_backup_stop(struct push_backup *pb, return -EBADF; cleanup_backup(pb); - /* Wait postpone_if_required_for_backup() starts timer */ - synchronize_rcu(); - WARN_ON_ONCE(timer_delete_sync_try(&pb->deadline_timer) < 0); + timer_delete_sync(&pb->deadline_timer); flush_workqueue(pb->wq); spin_lock_irq(&pb->lock); @@ -632,7 +628,7 @@ static void pb_destroy(struct push_backup *pb) { WARN_ON_ONCE(pb->nr_delayed); - WARN_ON_ONCE(timer_delete_sync_try(&pb->deadline_timer) < 0); + timer_shutdown_sync(&pb->deadline_timer); if (pb->wq) destroy_workqueue(pb->wq); kvfree(pb->map); /* Is's not zero if stop was not called */ -- 2.43.5 _______________________________________________ Devel mailing list Devel@openvz.org https://lists.openvz.org/mailman/listinfo/devel ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Devel] [PATCH hci-8.1 v5 5/7] drivers/md/push_backup: arm deadline timer under pb->lock 2026-11-03 12:38 ` [Devel] [PATCH hci-8.1 v5 5/7] drivers/md/push_backup: arm deadline timer under pb->lock Andrey Zhadchenko @ 2026-11-03 12:48 ` no-reply 0 siblings, 0 replies; 17+ messages in thread From: no-reply @ 2026-11-03 12:48 UTC (permalink / raw) To: khorenko, alexander.ivanov, Andrey Zhadchenko; +Cc: devel > Both pb_timer_func() and postpone_if_required_for_backup() decide > whether to arm the timer under pb->lock but call mod_timer() after > dropping it, so the decisions can be applied out of order: > > CPU1 pb_timer_func() CPU2 postpone_if_required_for_backup() > lock; deadline = S64_MAX > unlock > lock; nr_delayed = 1 > deadline_jiffies = now + timeout > unlock > mod_timer(timeout) > mod_timer(from S64_MAX) > > The late mod_timer() of the callback overwrites the fresh deadline, and > the write is delayed with the timer parked in a very distant future. > [ ... ] > pb->ctl_rwsem; the next patch removes that call. > > Fixes: 62e07487348e ("dm: Add push backup driver") > https://virtuozzo.atlassian.net/browse/VSTOR-152912 > Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com> Sashiko has reviewed this patch and found no issues. It looks great! -- Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261103123802.846676-1-andrey.zhadchenko@virtuozzo.com?part=5 _______________________________________________ Devel mailing list Devel@openvz.org https://lists.openvz.org/mailman/listinfo/devel ^ permalink raw reply [flat|nested] 17+ messages in thread
* [Devel] [PATCH hci-8.1 v5 6/7] drivers/md/push_backup: do not re-arm deadline timer in push_backup_write() 2026-11-03 12:37 [Devel] [PATCH hci-8.1 v5 0/7] dm/push_backup: event fd interface and fixes Andrey Zhadchenko ` (4 preceding siblings ...) 2026-11-03 12:38 ` [Devel] [PATCH hci-8.1 v5 5/7] drivers/md/push_backup: arm deadline timer under pb->lock Andrey Zhadchenko @ 2026-11-03 12:38 ` Andrey Zhadchenko 2026-11-03 12:48 ` no-reply 2026-11-03 12:38 ` [Devel] [PATCH hci-8.1 v5 7/7] drivers/md/push_backup: add event file descriptor interface Andrey Zhadchenko 2026-11-03 14:44 ` [Devel] [PATCH hci-8.1 v5 0/7] dm/push_backup: event fd interface and fixes Konstantin Khorenko 7 siblings, 1 reply; 17+ messages in thread From: Andrey Zhadchenko @ 2026-11-03 12:38 UTC (permalink / raw) To: khorenko, alexander.ivanov; +Cc: devel pb_timer_func() checks pb->deadline_jiffies every time it fires and re-arms itself under pb->lock while requests are delayed, and postpone_if_required_for_backup() arms it under pb->lock when the first request is delayed. So the timer is always pending while writes are delayed and we don't need to re-arm it after updating the field. This call is also not done under pb->lock. Today it is harmless because write and stop are serialized by pb->ctl_rwsem, but it would re-arm the timer behind timer_delete_sync() in push_backup_stop() once the write path is reachable without the rwsem (next patch). https://virtuozzo.atlassian.net/browse/VSTOR-152912 Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com> Reviewed-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com> --- v2: no changes v3: no changes v4: commit message typo v5: moved after "arm deadline timer under pb->lock", so no bisect point re-opens the race; dropped the "only place" claim drivers/md/dm-push-backup.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/drivers/md/dm-push-backup.c b/drivers/md/dm-push-backup.c index f0de26add11c2..52ba80b703ff1 100644 --- a/drivers/md/dm-push-backup.c +++ b/drivers/md/dm-push-backup.c @@ -463,8 +463,8 @@ static int push_backup_write(struct push_backup *pb, unsigned int clu, unsigned int nr) { u64 i, key, nr_clus = pb->nr_clus; - bool finished, has_more = false; LIST_HEAD(pbio_list); + bool finished; struct pb_bio *pbio; if (!pb) @@ -515,8 +515,7 @@ static int push_backup_write(struct push_backup *pb, } } - has_more = (pb->nr_delayed != 0); - if (has_more) + if (pb->nr_delayed) pb->deadline_jiffies = get_jiffies_64() + pb->timeout_in_jiffies; else pb->deadline_jiffies = S64_MAX; @@ -525,11 +524,8 @@ static int push_backup_write(struct push_backup *pb, if (finished) wake_up_interruptible(&pb->waitq); - if (!list_empty(&pbio_list)) { + if (!list_empty(&pbio_list)) resubmit_pbios(pb, &pbio_list); - if (has_more) - mod_timer(&pb->deadline_timer, jiffies + pb->timeout_in_jiffies + 1); - } return 0; } -- 2.43.5 _______________________________________________ Devel mailing list Devel@openvz.org https://lists.openvz.org/mailman/listinfo/devel ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Devel] [PATCH hci-8.1 v5 6/7] drivers/md/push_backup: do not re-arm deadline timer in push_backup_write() 2026-11-03 12:38 ` [Devel] [PATCH hci-8.1 v5 6/7] drivers/md/push_backup: do not re-arm deadline timer in push_backup_write() Andrey Zhadchenko @ 2026-11-03 12:48 ` no-reply 0 siblings, 0 replies; 17+ messages in thread From: no-reply @ 2026-11-03 12:48 UTC (permalink / raw) To: alexander.ivanov, Andrey Zhadchenko, khorenko; +Cc: devel > pb_timer_func() checks pb->deadline_jiffies every time it fires and > re-arms itself under pb->lock while requests are delayed, and > postpone_if_required_for_backup() arms it under pb->lock when the first > request is delayed. So the timer is always pending while writes are > delayed and we don't need to re-arm it after updating the field. > > This call is also not done under pb->lock. Today it is harmless because > write and stop are serialized by pb->ctl_rwsem, but it would re-arm the > timer behind timer_delete_sync() in push_backup_stop() once the write > path is reachable without the rwsem (next patch). > > https://virtuozzo.atlassian.net/browse/VSTOR-152912 > Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com> Sashiko has reviewed this patch and found no issues. It looks great! -- Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261103123802.846676-1-andrey.zhadchenko@virtuozzo.com?part=6 _______________________________________________ Devel mailing list Devel@openvz.org https://lists.openvz.org/mailman/listinfo/devel ^ permalink raw reply [flat|nested] 17+ messages in thread
* [Devel] [PATCH hci-8.1 v5 7/7] drivers/md/push_backup: add event file descriptor interface 2026-11-03 12:37 [Devel] [PATCH hci-8.1 v5 0/7] dm/push_backup: event fd interface and fixes Andrey Zhadchenko ` (5 preceding siblings ...) 2026-11-03 12:38 ` [Devel] [PATCH hci-8.1 v5 6/7] drivers/md/push_backup: do not re-arm deadline timer in push_backup_write() Andrey Zhadchenko @ 2026-11-03 12:38 ` Andrey Zhadchenko 2026-11-03 12:53 ` no-reply 2026-11-03 14:44 ` Konstantin Khorenko 2026-11-03 14:44 ` [Devel] [PATCH hci-8.1 v5 0/7] dm/push_backup: event fd interface and fixes Konstantin Khorenko 7 siblings, 2 replies; 17+ messages in thread From: Andrey Zhadchenko @ 2026-11-03 12:38 UTC (permalink / raw) To: khorenko, alexander.ivanov; +Cc: devel The backup tool currently waits for the next cluster range to back up by sleeping inside the "push_backup_read" target message. target_message() calls into the target under dm_get_live_table(), i.e. with md->io_barrier SRCU read-locked, so for the whole time the tool waits for a write to be intercepted anything doing synchronize_srcu(&md->io_barrier) on this device is stuck: dm suspend, table reload/remove and in particular dm_internal_suspend_fast() called by "@stats_create". An unrelated dm-stats consumer therefore hangs together with everything that later queues behind md->suspend_lock. Add a file descriptor based interface that does not involve dm messages on the data path: push_backup_get_fd - returns an anon inode fd in the message result read(fd, &range, 16) - next struct dm_push_backup_range to back up; 0 when the backup is finished, ESTALE when not running, ENODEV after the target is destroyed write(fd, &range, 16) - acknowledge the range, same as push_backup_write The fd is bound to the target, not to a backup session, and keeps working across stop and a later start. We can't access/uninstall fd across all processes that have it, so add kref on the push_backup struct and tie it to number of fds. Free only when everything is gone. The legacy messages are kept for compatibility but we will clean them up later. They now share the code with the fd, which changes two things: pb_message() parsed u64 but passed unsigned int to push_backup_write(), so "push_backup_write 4294967296:1" acked cluster 0 and now fails with -E2BIG, and push_backup_read printed "%u" ranges, which was wrong on devices with 2^32 or more clusters. Also free the pbio before requeuing its request in resubmit_pbios(). In-flight requests pin the mapped device (dm_start_request() -> dm_get()), not the target, so once the last request is requeued the target may be destroyed together with pbio_pool. The dm messages run under the live table and are safe, but the fd write path holds no md reference. Bump the target version to 1.1.0. https://virtuozzo.atlassian.net/browse/VSTOR-152912 Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com> Reviewed-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com> --- v2: free the pbio before dm_requeue_original_rq() in resubmit_pbios(), the requeue may drop the last md reference and let .dtr run v3: install the fd only after its number fits into the result buffer (reserve fd -> DMEMIT -> overflow check -> anon_inode_getfile -> fd_install); rename pb->dead to pb->destroyed; prefix the UAPI type as struct dm_push_backup_range; read() requires exactly sizeof(range) like write(); subject prefix aligned with the rest of the series v4: commit message typos v5: header: the fd survives stop/start, -1/errno, EINTR, EFAULT, EBADMSG and write() errors, O_CLOEXEC, "dmsetup message" is of no use; pb_destroy() NULLs the pointers it frees; blank line after the declaration in resubmit_pbios(); commit message: legacy u64 parsing changes, correct resubmit_pbios() rationale, version bump to 1.1.0 drivers/md/dm-push-backup.c | 218 ++++++++++++++++++++++++---- include/uapi/linux/dm-push-backup.h | 48 ++++++ 2 files changed, 237 insertions(+), 29 deletions(-) create mode 100644 include/uapi/linux/dm-push-backup.h diff --git a/drivers/md/dm-push-backup.c b/drivers/md/dm-push-backup.c index 52ba80b703ff1..c85e8446895d7 100644 --- a/drivers/md/dm-push-backup.c +++ b/drivers/md/dm-push-backup.c @@ -15,6 +15,11 @@ #include <linux/ctype.h> #include <linux/dm-io.h> #include <linux/blk-mq.h> +#include <linux/anon_inodes.h> +#include <linux/file.h> +#include <linux/kref.h> +#include <linux/uaccess.h> +#include <linux/dm-push-backup.h> #define DM_MSG_PREFIX "push-backup" @@ -46,6 +51,9 @@ struct push_backup { u64 clu_size; u64 nr_clus; + struct kref ref; + bool destroyed; + bool alive; void *map; u64 map_bits; @@ -69,6 +77,16 @@ struct push_backup { struct rw_semaphore ctl_rwsem; }; +static void pb_release(struct kref *ref) +{ + kfree(container_of(ref, struct push_backup, ref)); +} + +static void pb_put(struct push_backup *pb) +{ + kref_put(&pb->ref, pb_release); +} + static u64 pbio_first_required_for_backup_clu(struct push_backup *pb, struct pb_bio *pbio) { u64 clu; @@ -196,9 +214,11 @@ static void resubmit_pbios(struct push_backup *pb, struct list_head *list) struct pb_bio *pbio; while ((pbio = list_first_entry_or_null(list, struct pb_bio, list)) != NULL) { + struct request *rq = pbio->rq; + list_del_init(&pbio->list); - dm_requeue_original_rq(pbio->rq); mempool_free(pbio, pb->pbio_pool); + dm_requeue_original_rq(rq); } } @@ -411,35 +431,41 @@ static int push_backup_stop(struct push_backup *pb, return 0; } -static int push_backup_read(struct push_backup *pb, - char *result, unsigned int maxlen) +static bool pb_has_event(struct push_backup *pb) +{ + return READ_ONCE(pb->destroyed) || !READ_ONCE(pb->map) || + !READ_ONCE(pb->alive) || !READ_ONCE(pb->map_bits) || + !list_empty_careful(&pb->pending); +} + +/* + * Returns: + * 1 - next range to back up is stored in @clu/@nr + * 0 - backup is finished (nothing left to back up) + * -EAGAIN - nothing pending at the moment + * -ESTALE - backup is not running (not started, stopped or expired) + * -ENODEV - target was destroyed + */ +static int pb_next_range(struct push_backup *pb, u64 *clu, u64 *nr) { - unsigned int left, right, sz = 0; struct pb_bio *pbio; + u64 left, right; int ret; - if (!pb) - return -EBADF; - if (!pb->map) - return -ESTALE; -again: - if (wait_event_interruptible(pb->waitq, - !list_empty_careful(&pb->pending) || - !pb->alive || !pb->map_bits)) - return -EINTR; - spin_lock_irq(&pb->lock); + ret = -ENODEV; + if (pb->destroyed) + goto unlock; ret = -ESTALE; - if (!pb->alive) + if (!pb->map || !pb->alive) goto unlock; ret = 0; if (!pb->map_bits) goto unlock; + ret = -EAGAIN; pbio = list_first_entry_or_null(&pb->pending, typeof(*pbio), list); - if (unlikely(!pbio)) { - spin_unlock_irq(&pb->lock); - goto again; - } + if (!pbio) + goto unlock; ret = -EBADMSG; left = pbio_first_required_for_backup_clu(pb, pbio); @@ -452,15 +478,47 @@ static int push_backup_read(struct push_backup *pb, else right = pb->nr_clus - 1; - DMEMIT("%u:%u", left, right - left + 1); + *clu = left; + *nr = right - left + 1; ret = 1; unlock: spin_unlock_irq(&pb->lock); return ret; } -static int push_backup_write(struct push_backup *pb, - unsigned int clu, unsigned int nr) +static int pb_wait_range(struct push_backup *pb, bool nonblock, + u64 *clu, u64 *nr) +{ + int ret; + + while (1) { + ret = pb_next_range(pb, clu, nr); + if (ret != -EAGAIN || nonblock) + return ret; + if (wait_event_interruptible(pb->waitq, pb_has_event(pb))) + return -ERESTARTSYS; + } +} + +static int push_backup_read(struct push_backup *pb, + char *result, unsigned int maxlen) +{ + unsigned int sz = 0; + u64 clu, nr; + int ret; + + if (!pb) + return -EBADF; + + ret = pb_wait_range(pb, false, &clu, &nr); + if (ret == -ERESTARTSYS) + ret = -EINTR; + if (ret == 1) + DMEMIT("%llu:%llu", clu, nr); + return ret; +} + +static int push_backup_write(struct push_backup *pb, u64 clu, u64 nr) { u64 i, key, nr_clus = pb->nr_clus; LIST_HEAD(pbio_list); @@ -471,11 +529,13 @@ static int push_backup_write(struct push_backup *pb, return -EBADF; if (clu >= nr_clus || nr > nr_clus - clu) return -E2BIG; - if (!pb->map) - return -ESTALE; spin_lock_irq(&pb->lock); - if (!pb->alive) { + if (pb->destroyed) { + spin_unlock_irq(&pb->lock); + return -ENODEV; + } + if (!pb->map || !pb->alive) { spin_unlock_irq(&pb->lock); return -ESTALE; } @@ -530,6 +590,84 @@ static int push_backup_write(struct push_backup *pb, return 0; } +static ssize_t pb_fd_read(struct file *file, char __user *buf, + size_t count, loff_t *ppos) +{ + struct push_backup *pb = file->private_data; + struct dm_push_backup_range range; + int ret; + + if (count != sizeof(range)) + return -EINVAL; + + ret = pb_wait_range(pb, file->f_flags & O_NONBLOCK, + &range.clu, &range.nr); + if (ret <= 0) + return ret; + + if (copy_to_user(buf, &range, sizeof(range))) + return -EFAULT; + return sizeof(range); +} + +static ssize_t pb_fd_write(struct file *file, const char __user *buf, + size_t count, loff_t *ppos) +{ + struct push_backup *pb = file->private_data; + struct dm_push_backup_range range; + int ret; + + if (count != sizeof(range)) + return -EINVAL; + if (copy_from_user(&range, buf, sizeof(range))) + return -EFAULT; + + ret = push_backup_write(pb, range.clu, range.nr); + return ret ? : sizeof(range); +} + +static int pb_fd_release(struct inode *inode, struct file *file) +{ + pb_put(file->private_data); + return 0; +} + +static const struct file_operations pb_fops = { + .owner = THIS_MODULE, + .read = pb_fd_read, + .write = pb_fd_write, + .release = pb_fd_release, +}; + +static int push_backup_get_fd(struct push_backup *pb, + char *result, unsigned int maxlen) +{ + unsigned int sz = 0; + struct file *file; + int fd; + + fd = get_unused_fd_flags(O_CLOEXEC); + if (fd < 0) + return fd; + + DMEMIT("%d", fd); + if (sz + 1 >= maxlen) { + put_unused_fd(fd); + return 1; + } + + kref_get(&pb->ref); + file = anon_inode_getfile("[push_backup]", &pb_fops, pb, O_RDWR); + if (IS_ERR(file)) { + pb_put(pb); + put_unused_fd(fd); + return PTR_ERR(file); + } + + fd_install(fd, file); + return 1; +} + static int push_backup_statistics(struct push_backup *pb, char *result, unsigned int maxlen) { @@ -608,6 +746,10 @@ static int pb_message(struct dm_target *ti, unsigned int argc, char **argv, ret = push_backup_write(pb, val, val2); } else if (!strcmp(argv[0], "push_backup_statistics")) { ret = push_backup_statistics(pb, result, maxlen); + } else if (!strcmp(argv[0], "push_backup_get_fd")) { + if (argc != 1) + goto unlock; + ret = push_backup_get_fd(pb, result, maxlen); } else { ret = -ENOTSUPP; } @@ -622,18 +764,35 @@ static int pb_message(struct dm_target *ti, unsigned int argc, char **argv, } static void pb_destroy(struct push_backup *pb) { + void *map, *pending_map; + WARN_ON_ONCE(pb->nr_delayed); + spin_lock_irq(&pb->lock); + pb->destroyed = true; + map = pb->map; /* It's not zero if stop was not called */ + pb->map = NULL; + pending_map = pb->pending_map; + pb->pending_map = NULL; + spin_unlock_irq(&pb->lock); + + wake_up_interruptible(&pb->waitq); + timer_shutdown_sync(&pb->deadline_timer); if (pb->wq) destroy_workqueue(pb->wq); - kvfree(pb->map); /* Is's not zero if stop was not called */ - kvfree(pb->pending_map); + pb->wq = NULL; + kvfree(map); + kvfree(pending_map); kvfree(pb->pending_htable); + pb->pending_htable = NULL; mempool_destroy(pb->pbio_pool); + pb->pbio_pool = NULL; if (pb->origin_dev) dm_put_device(pb->ti, pb->origin_dev); - kfree(pb); + pb->origin_dev = NULL; + pb->ti = NULL; + pb_put(pb); } /* @@ -652,6 +811,7 @@ static int pb_ctr(struct dm_target *ti, unsigned int argc, char **argv) pb = kzalloc(sizeof(*pb), GFP_KERNEL); if (!pb) goto err; + kref_init(&pb->ref); spin_lock_init(&pb->lock); init_rwsem(&pb->ctl_rwsem); bio_list_init(&pb->deferred_bios); @@ -761,7 +921,7 @@ static loff_t pb_llseek_hole(struct dm_target *ti, loff_t offset, static struct target_type pb_target = { .name = "push_backup", - .version = {1, 0, 0}, + .version = {1, 1, 0}, .features = DM_TARGET_SINGLETON, .module = THIS_MODULE, .ctr = pb_ctr, diff --git a/include/uapi/linux/dm-push-backup.h b/include/uapi/linux/dm-push-backup.h new file mode 100644 index 0000000000000..7e24835f2c83a --- /dev/null +++ b/include/uapi/linux/dm-push-backup.h @@ -0,0 +1,48 @@ +/* SPDX-License-Identifier: GPL-2.0-only WITH Linux-syscall-note */ +/* + * Copyright (c) 2026 Virtuozzo International GmbH. All rights reserved. + */ + +/* + * Userspace interface of the dm-push-backup target's event file descriptor. + * + * The fd is obtained with the "push_backup_get_fd" target message; the fd + * is installed into the calling process and the number is returned in the + * message result buffer. + * + * The fd is bound to the target and push_backup_stop does not invalidate + * it. + * + * Both read() and write() operate on exactly one struct dm_push_backup_range + * and fail with EINVAL for any other count. On failure they return -1 and + * set errno as usual. + * + * read(fd, &range, sizeof(range)): + * sizeof(range) - @range holds the next cluster range that must be backed + * up before the writes postponed on it may proceed. + * Reading does not consume anything: the same (or a wider) + * range is returned until it is acknowledged via write(). + * 0 - backup has finished, nothing is left to back up. + * EAGAIN - O_NONBLOCK is set and no writes are pending. + * ESTALE - backup is not running (not started, stopped or expired). + * ENODEV - the target has been destroyed. + * EINTR - interrupted by a signal while waiting. + * EFAULT - @range is not accessible. + * EBADMSG - internal inconsistency of the pending state. + * + * write(fd, &range, sizeof(range)): + * acknowledge that @range has been backed up; equivalent to the + * "push_backup_write clu:nr" message. Fails with E2BIG if @range is out + * of the device, ESTALE and ENODEV as read(), and EFAULT. + */ +#ifndef _UAPI_LINUX_DM_PUSH_BACKUP_H +#define _UAPI_LINUX_DM_PUSH_BACKUP_H + +#include <linux/types.h> + +struct dm_push_backup_range { + __u64 clu; /* first cluster */ + __u64 nr; /* number of clusters */ +}; + +#endif /* _UAPI_LINUX_DM_PUSH_BACKUP_H */ -- 2.43.5 _______________________________________________ Devel mailing list Devel@openvz.org https://lists.openvz.org/mailman/listinfo/devel ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Devel] [PATCH hci-8.1 v5 7/7] drivers/md/push_backup: add event file descriptor interface 2026-11-03 12:38 ` [Devel] [PATCH hci-8.1 v5 7/7] drivers/md/push_backup: add event file descriptor interface Andrey Zhadchenko @ 2026-11-03 12:53 ` no-reply 2026-11-03 14:44 ` Konstantin Khorenko 1 sibling, 0 replies; 17+ messages in thread From: no-reply @ 2026-11-03 12:53 UTC (permalink / raw) To: khorenko, alexander.ivanov, Andrey Zhadchenko; +Cc: devel Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] dm-push-backup: push_backup_get_fd() installs fd before ioctl copy-out can fail -- commit a842da27ce790f5d0a5ba6d00f2e722207c6d972 Author: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com> drivers/md/push_backup: add event file descriptor interface This patch adds a "push_backup_get_fd" target message that returns an anon inode fd. The backup tool can read() the next cluster range from that fd and write() an acknowledgement to it, instead of sleeping inside a dm message. The patch also adds a kref on struct push_backup so that it outlives the target while fds remain open. Link: https://virtuozzo.atlassian.net/browse/VSTOR-152912 > diff --git a/drivers/md/dm-push-backup.c b/drivers/md/dm-push-backup.c > --- a/drivers/md/dm-push-backup.c > +++ b/drivers/md/dm-push-backup.c [ ... ] > +static int push_backup_get_fd(struct push_backup *pb, > + char *result, unsigned int maxlen) > +{ > + unsigned int sz = 0; > + struct file *file; > + int fd; > + > + fd = get_unused_fd_flags(O_CLOEXEC); > + if (fd < 0) > + return fd; > + > + DMEMIT("%d", fd); > + if (sz + 1 >= maxlen) { > + put_unused_fd(fd); > + return 1; > + } > + > + kref_get(&pb->ref); > + file = anon_inode_getfile("[push_backup]", &pb_fops, pb, O_RDWR); > + if (IS_ERR(file)) { > + pb_put(pb); > + put_unused_fd(fd); > + return PTR_ERR(file); > + } > + > + fd_install(fd, file); > + return 1; > +} [Severity: Low] Can the fd number be lost after fd_install() here? The fd is installed into the caller's table before the dm ioctl core has copied the result buffer back to userspace: ctl_ioctl() target_message() pb_message() push_backup_get_fd() fd_install(fd, file); return 1; ... if (!r && copy_to_user(user, param, param->data_size)) r = -EFAULT; If that copy_to_user() in ctl_ioctl() fails, for example because the user buffer is read-only or gets unmapped by another thread, the ioctl returns -EFAULT. The fd number was only ever placed in param, so the caller never learns it. The fd then stays open, and it keeps its reference on pb->ref, until the process exits or happens to close that number. The overflow check and the anon_inode_getfile() failure path are both unwound before fd_install(), so only the copy-out failure is uncovered. Would it be possible to deliver the fd number through a path that cannot fail after installation? One option is a dedicated ioctl that does put_user() first and then fd_install(). -- Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261103123802.846676-1-andrey.zhadchenko@virtuozzo.com?part=7 _______________________________________________ Devel mailing list Devel@openvz.org https://lists.openvz.org/mailman/listinfo/devel ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Devel] [PATCH hci-8.1 v5 7/7] drivers/md/push_backup: add event file descriptor interface 2026-11-03 12:38 ` [Devel] [PATCH hci-8.1 v5 7/7] drivers/md/push_backup: add event file descriptor interface Andrey Zhadchenko 2026-11-03 12:53 ` no-reply @ 2026-11-03 14:44 ` Konstantin Khorenko 1 sibling, 0 replies; 17+ messages in thread From: Konstantin Khorenko @ 2026-11-03 14:44 UTC (permalink / raw) To: Andrey Zhadchenko; +Cc: devel On 11/3/26 15:38, Andrey Zhadchenko wrote: > + ret = pb_wait_range(pb, file->f_flags & O_NONBLOCK, > + &range.clu, &range.nr); Hi Andrey, - either drop .poll and O_NONBLOCK together: pass false here, drop the nonblock argument of pb_wait_range() and the EAGAIN line from the header; - or keep O_NONBLOCK and add .poll on pb->waitq with pb_has_event(). Today it is in between: read() honours O_NONBLOCK, but there is no way to wait for the fd to become readable. A non-blocking reader can only spin, and poll()/epoll report the fd as always ready. Either variant is fine with me. -- Konstantin Khorenko, Virtuozzo Linux Kernel Team _______________________________________________ Devel mailing list Devel@openvz.org https://lists.openvz.org/mailman/listinfo/devel ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Devel] [PATCH hci-8.1 v5 0/7] dm/push_backup: event fd interface and fixes 2026-11-03 12:37 [Devel] [PATCH hci-8.1 v5 0/7] dm/push_backup: event fd interface and fixes Andrey Zhadchenko ` (6 preceding siblings ...) 2026-11-03 12:38 ` [Devel] [PATCH hci-8.1 v5 7/7] drivers/md/push_backup: add event file descriptor interface Andrey Zhadchenko @ 2026-11-03 14:44 ` Konstantin Khorenko 7 siblings, 0 replies; 17+ messages in thread From: Konstantin Khorenko @ 2026-11-03 14:44 UTC (permalink / raw) To: Andrey Zhadchenko; +Cc: devel On 11/3/26 15:37, Andrey Zhadchenko wrote: > Andrey Zhadchenko (7): > drivers/md/push_backup: correct initialization Hi Andrey, Thanks, v5 covers the v4 remarks. Left for the whole series: - add "Feature: dm-backup: push backup target driver" to every commit; - use the "dm/push_backup:" subject prefix. The earlier commits of the driver carry that Feature line and use that prefix, and so does this cover letter. One more remark is on 7/7 (O_NONBLOCK). -- Konstantin Khorenko, Virtuozzo Linux Kernel Team _______________________________________________ Devel mailing list Devel@openvz.org https://lists.openvz.org/mailman/listinfo/devel ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2026-11-03 14:46 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-11-03 12:37 [Devel] [PATCH hci-8.1 v5 0/7] dm/push_backup: event fd interface and fixes Andrey Zhadchenko 2026-11-03 12:37 ` [Devel] [PATCH hci-8.1 v5 1/7] drivers/md/push_backup: correct initialization Andrey Zhadchenko 2026-11-03 12:46 ` no-reply 2026-11-03 12:37 ` [Devel] [PATCH hci-8.1 v5 2/7] drivers/md/push_backup: return error if workqueue allocation fails Andrey Zhadchenko 2026-11-03 12:45 ` no-reply 2026-11-03 12:37 ` [Devel] [PATCH hci-8.1 v5 3/7] drivers/md/push_backup: clear pb->alive under pb->lock Andrey Zhadchenko 2026-11-03 12:46 ` no-reply 2026-11-03 12:37 ` [Devel] [PATCH hci-8.1 v5 4/7] drivers/md/push_backup: fix mod_timer() usage Andrey Zhadchenko 2026-11-03 12:47 ` no-reply 2026-11-03 12:38 ` [Devel] [PATCH hci-8.1 v5 5/7] drivers/md/push_backup: arm deadline timer under pb->lock Andrey Zhadchenko 2026-11-03 12:48 ` no-reply 2026-11-03 12:38 ` [Devel] [PATCH hci-8.1 v5 6/7] drivers/md/push_backup: do not re-arm deadline timer in push_backup_write() Andrey Zhadchenko 2026-11-03 12:48 ` no-reply 2026-11-03 12:38 ` [Devel] [PATCH hci-8.1 v5 7/7] drivers/md/push_backup: add event file descriptor interface Andrey Zhadchenko 2026-11-03 12:53 ` no-reply 2026-11-03 14:44 ` Konstantin Khorenko 2026-11-03 14:44 ` [Devel] [PATCH hci-8.1 v5 0/7] dm/push_backup: event fd interface and fixes Konstantin Khorenko
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.