* [Devel] [PATCH hci-8.1 v6 0/7] dm/push_backup: event fd interface and fixes
@ 2026-11-04 8:38 Andrey Zhadchenko
2026-11-04 8:38 ` [Devel] [PATCH hci-8.1 v6 1/7] dm/push_backup: correct initialization Andrey Zhadchenko
` (7 more replies)
0 siblings, 8 replies; 16+ messages in thread
From: Andrey Zhadchenko @ 2026-11-04 8:38 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.
v6:
- all: subject prefix changed to dm/push_backup, added Feature: tag.
- 7/7: dropped O_NONBLOCK support, read() always blocks.
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):
dm/push_backup: correct initialization
dm/push_backup: return error if workqueue allocation fails
dm/push_backup: clear pb->alive under pb->lock
dm/push_backup: fix mod_timer() usage
dm/push_backup: arm deadline timer under pb->lock
dm/push_backup: do not re-arm deadline timer in push_backup_write()
dm/push_backup: add event file descriptor interface
drivers/md/dm-push-backup.c | 273 +++++++++++++++++++++-------
include/uapi/linux/dm-push-backup.h | 47 +++++
2 files changed, 259 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] 16+ messages in thread
* [Devel] [PATCH hci-8.1 v6 1/7] dm/push_backup: correct initialization
2026-11-04 8:38 [Devel] [PATCH hci-8.1 v6 0/7] dm/push_backup: event fd interface and fixes Andrey Zhadchenko
@ 2026-11-04 8:38 ` Andrey Zhadchenko
2026-11-04 8:45 ` no-reply
2026-11-04 8:38 ` [Devel] [PATCH hci-8.1 v6 2/7] dm/push_backup: return error if workqueue allocation fails Andrey Zhadchenko
` (6 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Andrey Zhadchenko @ 2026-11-04 8:38 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
Feature: dm-backup: push backup target driver
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
v6: subject prefix changed to dm/push_backup, added Feature: tag
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] 16+ messages in thread
* [Devel] [PATCH hci-8.1 v6 2/7] dm/push_backup: return error if workqueue allocation fails
2026-11-04 8:38 [Devel] [PATCH hci-8.1 v6 0/7] dm/push_backup: event fd interface and fixes Andrey Zhadchenko
2026-11-04 8:38 ` [Devel] [PATCH hci-8.1 v6 1/7] dm/push_backup: correct initialization Andrey Zhadchenko
@ 2026-11-04 8:38 ` Andrey Zhadchenko
2026-11-04 8:46 ` no-reply
2026-11-04 8:38 ` [Devel] [PATCH hci-8.1 v6 3/7] dm/push_backup: clear pb->alive under pb->lock Andrey Zhadchenko
` (5 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Andrey Zhadchenko @ 2026-11-04 8:38 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
Feature: dm-backup: push backup target driver
Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
---
v5: new patch
v6: subject prefix changed to dm/push_backup, added Feature: tag
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] 16+ messages in thread
* [Devel] [PATCH hci-8.1 v6 3/7] dm/push_backup: clear pb->alive under pb->lock
2026-11-04 8:38 [Devel] [PATCH hci-8.1 v6 0/7] dm/push_backup: event fd interface and fixes Andrey Zhadchenko
2026-11-04 8:38 ` [Devel] [PATCH hci-8.1 v6 1/7] dm/push_backup: correct initialization Andrey Zhadchenko
2026-11-04 8:38 ` [Devel] [PATCH hci-8.1 v6 2/7] dm/push_backup: return error if workqueue allocation fails Andrey Zhadchenko
@ 2026-11-04 8:38 ` Andrey Zhadchenko
2026-11-04 8:46 ` no-reply
2026-11-04 8:38 ` [Devel] [PATCH hci-8.1 v6 4/7] dm/push_backup: fix mod_timer() usage Andrey Zhadchenko
` (4 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Andrey Zhadchenko @ 2026-11-04 8:38 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
Feature: dm-backup: push backup target driver
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
v6: subject prefix changed to dm/push_backup, added Feature: tag
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] 16+ messages in thread
* [Devel] [PATCH hci-8.1 v6 4/7] dm/push_backup: fix mod_timer() usage
2026-11-04 8:38 [Devel] [PATCH hci-8.1 v6 0/7] dm/push_backup: event fd interface and fixes Andrey Zhadchenko
` (2 preceding siblings ...)
2026-11-04 8:38 ` [Devel] [PATCH hci-8.1 v6 3/7] dm/push_backup: clear pb->alive under pb->lock Andrey Zhadchenko
@ 2026-11-04 8:38 ` Andrey Zhadchenko
2026-11-04 8:47 ` no-reply
2026-11-04 8:38 ` [Devel] [PATCH hci-8.1 v6 5/7] dm/push_backup: arm deadline timer under pb->lock Andrey Zhadchenko
` (3 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Andrey Zhadchenko @ 2026-11-04 8:38 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
Feature: dm-backup: push backup target driver
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
v6: subject prefix changed to dm/push_backup, added Feature: tag
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] 16+ messages in thread
* [Devel] [PATCH hci-8.1 v6 5/7] dm/push_backup: arm deadline timer under pb->lock
2026-11-04 8:38 [Devel] [PATCH hci-8.1 v6 0/7] dm/push_backup: event fd interface and fixes Andrey Zhadchenko
` (3 preceding siblings ...)
2026-11-04 8:38 ` [Devel] [PATCH hci-8.1 v6 4/7] dm/push_backup: fix mod_timer() usage Andrey Zhadchenko
@ 2026-11-04 8:38 ` Andrey Zhadchenko
2026-11-04 8:48 ` no-reply
2026-11-04 8:38 ` [Devel] [PATCH hci-8.1 v6 6/7] dm/push_backup: do not re-arm deadline timer in push_backup_write() Andrey Zhadchenko
` (2 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Andrey Zhadchenko @ 2026-11-04 8: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
Feature: dm-backup: push backup target driver
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
v6: subject prefix changed to dm/push_backup, added Feature: 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] 16+ messages in thread
* [Devel] [PATCH hci-8.1 v6 6/7] dm/push_backup: do not re-arm deadline timer in push_backup_write()
2026-11-04 8:38 [Devel] [PATCH hci-8.1 v6 0/7] dm/push_backup: event fd interface and fixes Andrey Zhadchenko
` (4 preceding siblings ...)
2026-11-04 8:38 ` [Devel] [PATCH hci-8.1 v6 5/7] dm/push_backup: arm deadline timer under pb->lock Andrey Zhadchenko
@ 2026-11-04 8:38 ` Andrey Zhadchenko
2026-11-04 8:50 ` no-reply
2026-11-04 8:38 ` [Devel] [PATCH hci-8.1 v6 7/7] dm/push_backup: add event file descriptor interface Andrey Zhadchenko
2026-11-04 18:46 ` [Devel] [PATCH hci-8.1 v6 0/7] dm/push_backup: event fd interface and fixes Konstantin Khorenko
7 siblings, 1 reply; 16+ messages in thread
From: Andrey Zhadchenko @ 2026-11-04 8: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
Feature: dm-backup: push backup target driver
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
v6: subject prefix changed to dm/push_backup, added Feature: tag
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] 16+ messages in thread
* [Devel] [PATCH hci-8.1 v6 7/7] dm/push_backup: add event file descriptor interface
2026-11-04 8:38 [Devel] [PATCH hci-8.1 v6 0/7] dm/push_backup: event fd interface and fixes Andrey Zhadchenko
` (5 preceding siblings ...)
2026-11-04 8:38 ` [Devel] [PATCH hci-8.1 v6 6/7] dm/push_backup: do not re-arm deadline timer in push_backup_write() Andrey Zhadchenko
@ 2026-11-04 8:38 ` Andrey Zhadchenko
2026-11-04 8:52 ` no-reply
2026-11-04 18:46 ` [Devel] [PATCH hci-8.1 v6 0/7] dm/push_backup: event fd interface and fixes Konstantin Khorenko
7 siblings, 1 reply; 16+ messages in thread
From: Andrey Zhadchenko @ 2026-11-04 8: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
Feature: dm-backup: push backup target driver
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
v6: subject prefix changed to dm/push_backup, added Feature: tag;
dropped O_NONBLOCK support, read() always blocks
drivers/md/dm-push-backup.c | 216 ++++++++++++++++++++++++----
include/uapi/linux/dm-push-backup.h | 47 ++++++
2 files changed, 234 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..4f6dc1f3c130f 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,46 @@ 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, u64 *clu, u64 *nr)
+{
+ int ret;
+
+ while (1) {
+ ret = pb_next_range(pb, clu, nr);
+ if (ret != -EAGAIN)
+ 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, &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 +528,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 +589,83 @@ 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, &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 +744,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 +762,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 +809,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 +919,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..b43e2e55dc693
--- /dev/null
+++ b/include/uapi/linux/dm-push-backup.h
@@ -0,0 +1,47 @@
+/* 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.
+ * 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] 16+ messages in thread
* Re: [Devel] [PATCH hci-8.1 v6 1/7] dm/push_backup: correct initialization
2026-11-04 8:38 ` [Devel] [PATCH hci-8.1 v6 1/7] dm/push_backup: correct initialization Andrey Zhadchenko
@ 2026-11-04 8:45 ` no-reply
0 siblings, 0 replies; 16+ messages in thread
From: no-reply @ 2026-11-04 8:45 UTC (permalink / raw)
To: khorenko, Andrey Zhadchenko, 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
>
> Feature: dm-backup: push backup target driver
> 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/20261104083818.856720-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] 16+ messages in thread
* Re: [Devel] [PATCH hci-8.1 v6 3/7] dm/push_backup: clear pb->alive under pb->lock
2026-11-04 8:38 ` [Devel] [PATCH hci-8.1 v6 3/7] dm/push_backup: clear pb->alive under pb->lock Andrey Zhadchenko
@ 2026-11-04 8:46 ` no-reply
0 siblings, 0 replies; 16+ messages in thread
From: no-reply @ 2026-11-04 8:46 UTC (permalink / raw)
To: alexander.ivanov, khorenko, 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
>
> Feature: dm-backup: push backup target driver
> 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/20261104083818.856720-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] 16+ messages in thread
* Re: [Devel] [PATCH hci-8.1 v6 2/7] dm/push_backup: return error if workqueue allocation fails
2026-11-04 8:38 ` [Devel] [PATCH hci-8.1 v6 2/7] dm/push_backup: return error if workqueue allocation fails Andrey Zhadchenko
@ 2026-11-04 8:46 ` no-reply
0 siblings, 0 replies; 16+ messages in thread
From: no-reply @ 2026-11-04 8:46 UTC (permalink / raw)
To: khorenko, alexander.ivanov, Andrey Zhadchenko; +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
>
> Feature: dm-backup: push backup target driver
> 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/20261104083818.856720-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] 16+ messages in thread
* Re: [Devel] [PATCH hci-8.1 v6 4/7] dm/push_backup: fix mod_timer() usage
2026-11-04 8:38 ` [Devel] [PATCH hci-8.1 v6 4/7] dm/push_backup: fix mod_timer() usage Andrey Zhadchenko
@ 2026-11-04 8:47 ` no-reply
0 siblings, 0 replies; 16+ messages in thread
From: no-reply @ 2026-11-04 8:47 UTC (permalink / raw)
To: khorenko, Andrey Zhadchenko, 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
>
> Feature: dm-backup: push backup target driver
> 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/20261104083818.856720-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] 16+ messages in thread
* Re: [Devel] [PATCH hci-8.1 v6 5/7] dm/push_backup: arm deadline timer under pb->lock
2026-11-04 8:38 ` [Devel] [PATCH hci-8.1 v6 5/7] dm/push_backup: arm deadline timer under pb->lock Andrey Zhadchenko
@ 2026-11-04 8:48 ` no-reply
0 siblings, 0 replies; 16+ messages in thread
From: no-reply @ 2026-11-04 8:48 UTC (permalink / raw)
To: Andrey Zhadchenko, alexander.ivanov, khorenko; +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.
> [ ... ]
> Fixes: 62e07487348e ("dm: Add push backup driver")
> https://virtuozzo.atlassian.net/browse/VSTOR-152912
>
> Feature: dm-backup: push backup target driver
> 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/20261104083818.856720-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] 16+ messages in thread
* Re: [Devel] [PATCH hci-8.1 v6 6/7] dm/push_backup: do not re-arm deadline timer in push_backup_write()
2026-11-04 8:38 ` [Devel] [PATCH hci-8.1 v6 6/7] dm/push_backup: do not re-arm deadline timer in push_backup_write() Andrey Zhadchenko
@ 2026-11-04 8:50 ` no-reply
0 siblings, 0 replies; 16+ messages in thread
From: no-reply @ 2026-11-04 8:50 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: commit message says timer is 'always pending' while delayed
--
commit 5c8b11d5d9c6ee6b26e3830c348cda4ca8f1bbca
Author: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
dm/push_backup: do not re-arm deadline timer in push_backup_write()
This patch removes the mod_timer() call from push_backup_write(). The
commit message says pb_timer_func() and
postpone_if_required_for_backup() keep the deadline timer pending while
writes are delayed, so the extra re-arm is not needed. The remaining code
only updates pb->deadline_jiffies under pb->lock.
Link: https://virtuozzo.atlassian.net/browse/VSTOR-152912
> dm/push_backup: do not re-arm deadline timer in push_backup_write()
>
> 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.
[Severity: Low]
This isn't a functional problem, but is the statement "the timer is
always pending while writes are delayed" accurate?
In pb_timer_func(), the expired branch does not re-arm the timer:
drivers/md/dm-push-backup.c:pb_timer_func() {
...
if (unlikely(time_before64(now, deadline)))
mod_timer(timer, jiffies + (deadline - now) + 1);
else
queue_work(pb->wq, &pb->worker);
}
If nr_delayed is non-zero and now >= deadline_jiffies, the timer calls
queue_work() and is not pending. From then until do_pb_work() runs
cleanup_backup(), nr_delayed can be non-zero while the timer is not
pending.
Removing the mod_timer() from push_backup_write() still looks safe in this
window. The queued worker drains every delayed pbio unconditionally, so a
deadline extension made by push_backup_write() cannot prevent expiry,
with or without the removed mod_timer().
Could the message be reworded to say the timer is pending, or the expiry
work is already queued?
> 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
[ ... ]
> @@ -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;
[ ... ]
> - 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;
> }
--
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261104083818.856720-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] 16+ messages in thread
* Re: [Devel] [PATCH hci-8.1 v6 7/7] dm/push_backup: add event file descriptor interface
2026-11-04 8:38 ` [Devel] [PATCH hci-8.1 v6 7/7] dm/push_backup: add event file descriptor interface Andrey Zhadchenko
@ 2026-11-04 8:52 ` no-reply
0 siblings, 0 replies; 16+ messages in thread
From: no-reply @ 2026-11-04 8:52 UTC (permalink / raw)
To: alexander.ivanov, Andrey Zhadchenko, khorenko; +Cc: devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] dm/push_backup: pb_fd_read() returns -ERESTARTSYS but UAPI header promises EINTR
--
commit 8c17414f2b8fe21a954bd9ba0c6114af23dc4c79
Author: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
dm/push_backup: add event file descriptor interface
This patch adds a push_backup_get_fd target message that returns an anon
inode fd. The fd supports read() for the next range to back up and write()
to acknowledge a range. It also adds a kref on struct push_backup so the
object lives until the last fd is released.
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 pb_wait_range(struct push_backup *pb, u64 *clu, u64 *nr)
> +{
> + int ret;
> +
> + while (1) {
> + ret = pb_next_range(pb, clu, nr);
> + if (ret != -EAGAIN)
> + return ret;
> + if (wait_event_interruptible(pb->waitq, pb_has_event(pb)))
> + return -ERESTARTSYS;
> + }
> +}
[ ... ]
> +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, &range.clu, &range.nr);
> + if (ret <= 0)
> + return ret;
[Severity: Low]
Is the EINTR documented for read() actually what userspace will see here?
pb_wait_range() returns -ERESTARTSYS when wait_event_interruptible() is
interrupted, and pb_fd_read() passes it through unchanged via the
"ret <= 0" return. The legacy push_backup_read() maps -ERESTARTSYS to
-EINTR, but the fd path does not.
If the signal handler has SA_RESTART, or no handler runs, the kernel
restarts read() transparently and userspace never sees EINTR. Without
SA_RESTART it is converted to EINTR.
> diff --git a/include/uapi/linux/dm-push-backup.h b/include/uapi/linux/dm-push-backup.h
> new file mode 100644
> --- /dev/null
> +++ b/include/uapi/linux/dm-push-backup.h
[ ... ]
> + * EINTR - interrupted by a signal while waiting.
The header documents EINTR unconditionally, so the documented behaviour
depends on signal disposition. Should the header describe the restart
behaviour, or should pb_fd_read() map -ERESTARTSYS to -EINTR as
push_backup_read() does?
--
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261104083818.856720-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] 16+ messages in thread
* Re: [Devel] [PATCH hci-8.1 v6 0/7] dm/push_backup: event fd interface and fixes
2026-11-04 8:38 [Devel] [PATCH hci-8.1 v6 0/7] dm/push_backup: event fd interface and fixes Andrey Zhadchenko
` (6 preceding siblings ...)
2026-11-04 8:38 ` [Devel] [PATCH hci-8.1 v6 7/7] dm/push_backup: add event file descriptor interface Andrey Zhadchenko
@ 2026-11-04 18:46 ` Konstantin Khorenko
7 siblings, 0 replies; 16+ messages in thread
From: Konstantin Khorenko @ 2026-11-04 18:46 UTC (permalink / raw)
To: alexander.ivanov, Andrey Zhadchenko; +Cc: devel
On Wed, 04 Nov 2026 11:38:11 +0300, Andrey Zhadchenko wrote:
> dm/push_backup: event fd interface and fixes
>
> 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.
>
> [...]
Applied to branch-rh10-6.12.0-211.52.1.22.x.vz10-ovz, will be released in rh10-6.12.0-211.52.1.22.18.vz10, thanks!
[1/7] dm/push_backup: correct initialization
https://bitbucket.org/virtuozzocore/vzkernel.vzs/commits/2c3179e99249
[2/7] dm/push_backup: return error if workqueue allocation fails
https://bitbucket.org/virtuozzocore/vzkernel.vzs/commits/d3737f6dad9b
[3/7] dm/push_backup: clear pb->alive under pb->lock
https://bitbucket.org/virtuozzocore/vzkernel.vzs/commits/5b615448f233
[4/7] dm/push_backup: fix mod_timer() usage
https://bitbucket.org/virtuozzocore/vzkernel.vzs/commits/33cc901a3a60
[5/7] dm/push_backup: arm deadline timer under pb->lock
https://bitbucket.org/virtuozzocore/vzkernel.vzs/commits/f14fc880548e
[6/7] dm/push_backup: do not re-arm deadline timer in push_backup_write()
https://bitbucket.org/virtuozzocore/vzkernel.vzs/commits/65f085404139
[7/7] dm/push_backup: add event file descriptor interface
https://bitbucket.org/virtuozzocore/vzkernel.vzs/commits/a44ab4b5751c
--
Konstantin Khorenko,
Virtuozzo Linux Kernel Team
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2026-11-04 18:48 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-04 8:38 [Devel] [PATCH hci-8.1 v6 0/7] dm/push_backup: event fd interface and fixes Andrey Zhadchenko
2026-11-04 8:38 ` [Devel] [PATCH hci-8.1 v6 1/7] dm/push_backup: correct initialization Andrey Zhadchenko
2026-11-04 8:45 ` no-reply
2026-11-04 8:38 ` [Devel] [PATCH hci-8.1 v6 2/7] dm/push_backup: return error if workqueue allocation fails Andrey Zhadchenko
2026-11-04 8:46 ` no-reply
2026-11-04 8:38 ` [Devel] [PATCH hci-8.1 v6 3/7] dm/push_backup: clear pb->alive under pb->lock Andrey Zhadchenko
2026-11-04 8:46 ` no-reply
2026-11-04 8:38 ` [Devel] [PATCH hci-8.1 v6 4/7] dm/push_backup: fix mod_timer() usage Andrey Zhadchenko
2026-11-04 8:47 ` no-reply
2026-11-04 8:38 ` [Devel] [PATCH hci-8.1 v6 5/7] dm/push_backup: arm deadline timer under pb->lock Andrey Zhadchenko
2026-11-04 8:48 ` no-reply
2026-11-04 8:38 ` [Devel] [PATCH hci-8.1 v6 6/7] dm/push_backup: do not re-arm deadline timer in push_backup_write() Andrey Zhadchenko
2026-11-04 8:50 ` no-reply
2026-11-04 8:38 ` [Devel] [PATCH hci-8.1 v6 7/7] dm/push_backup: add event file descriptor interface Andrey Zhadchenko
2026-11-04 8:52 ` no-reply
2026-11-04 18:46 ` [Devel] [PATCH hci-8.1 v6 0/7] dm/push_backup: event fd interface and fixes Konstantin Khorenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox