All Virtuozzo development lists (kernel + QEMU)
 help / color / mirror / Atom feed
* [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

* [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

* [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

* [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

* [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

* [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

* [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 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

* 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

* 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

* 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

* 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

* 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

* 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.