All Virtuozzo development lists (kernel + QEMU)
 help / color / mirror / Atom feed
* [Devel] [PATCH hci-8.1 v3 0/6] dm/push_backup: event fd interface and fixes
@ 2026-11-03  6:17 Andrey Zhadchenko
  2026-11-03  6:17 ` [Devel] [PATCH hci-8.1 v3 1/6] drivers/md/push_backup: correct initialization Andrey Zhadchenko
                   ` (6 more replies)
  0 siblings, 7 replies; 18+ messages in thread
From: Andrey Zhadchenko @ 2026-11-03  6:17 UTC (permalink / raw)
  To: 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 6 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 returns -ENODEV afterwards. The legacy messages are kept for
compatibility.

Patches 1-5 are fixes found while working on this:

  1. init the lock, waitqueue and timer before anything that can fail so
     pb_destroy() is safe from any error path;
  2. clear pb->alive under pb->lock like the other users do;
  3. drop the redundant mod_timer() in push_backup_write(). The timer
     re-arms itself from pb->deadline_jiffies, so updating the deadline
     under the lock is enough. This was also the only place that armed
     the timer outside the window push_backup_stop() waits for, which
     matters once writes may arrive via the fd without pb->ctl_rwsem;
  4. pass absolute expiry to mod_timer(). Both remaining callers passed
     a relative value, so after ~5 min of uptime 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 a stale S64_MAX could
     overwrite a fresh deadline and park the timer ~12 days out with
     delayed writes still pending. Also, timer_delete_sync_try() does not
     wait for a running callback, so one blocked on pb->lock could outlive
     push_backup_stop() and expire a later session. With arming done under
     the lock, the callback checking pb->alive 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.

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 (6):
  drivers/md/push_backup: correct initialization
  drivers/md/push_backup: clear pb->alive under pb->lock
  drivers/md/push_backup: do not re-arm deadline timer in
    push_backup_write()
  drivers/md/push_backup: fix mod_timer() usage
  drivers/md/push_backup: arm deadline timer under pb->lock
  drivers/md/push_backup: add event file descriptor interface

 drivers/md/dm-push-backup.c         | 268 +++++++++++++++++++++-------
 include/uapi/linux/dm-push-backup.h |  39 ++++
 2 files changed, 246 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] 18+ messages in thread

* [Devel] [PATCH hci-8.1 v3 1/6] drivers/md/push_backup: correct initialization
  2026-11-03  6:17 [Devel] [PATCH hci-8.1 v3 0/6] dm/push_backup: event fd interface and fixes Andrey Zhadchenko
@ 2026-11-03  6:17 ` Andrey Zhadchenko
  2026-11-03  6:25   ` no-reply
  2026-11-03  6:17 ` [Devel] [PATCH hci-8.1 v3 2/6] drivers/md/push_backup: clear pb->alive under pb->lock Andrey Zhadchenko
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 18+ messages in thread
From: Andrey Zhadchenko @ 2026-11-03  6:17 UTC (permalink / raw)
  To: alexander.ivanov; +Cc: devel

Allocate pool and htable after lock, waitqueue and deadline
timer init so we are sure that pb_destroy() can be called
from anywhere. Otherwise we risk timer_delete_sync_try on
zeroed timer struct (currently luckily fine).

https://virtuozzo.atlassian.net/browse/VSTOR-152912
Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
---
v2: no changes
v3: no changes

 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] 18+ messages in thread

* [Devel] [PATCH hci-8.1 v3 2/6] drivers/md/push_backup: clear pb->alive under pb->lock
  2026-11-03  6:17 [Devel] [PATCH hci-8.1 v3 0/6] dm/push_backup: event fd interface and fixes Andrey Zhadchenko
  2026-11-03  6:17 ` [Devel] [PATCH hci-8.1 v3 1/6] drivers/md/push_backup: correct initialization Andrey Zhadchenko
@ 2026-11-03  6:17 ` Andrey Zhadchenko
  2026-11-03  6:25   ` no-reply
  2026-11-03  6:17 ` [Devel] [PATCH hci-8.1 v3 3/6] drivers/md/push_backup: do not re-arm deadline timer in push_backup_write() Andrey Zhadchenko
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 18+ messages in thread
From: Andrey Zhadchenko @ 2026-11-03  6:17 UTC (permalink / raw)
  To: 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>
---
v2: reworded the commit message, dropped the KCSAN claim
v3: 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 9a6252f2b21e2..1ae4ccc208ffa 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] 18+ messages in thread

* [Devel] [PATCH hci-8.1 v3 3/6] drivers/md/push_backup: do not re-arm deadline timer in push_backup_write()
  2026-11-03  6:17 [Devel] [PATCH hci-8.1 v3 0/6] dm/push_backup: event fd interface and fixes Andrey Zhadchenko
  2026-11-03  6:17 ` [Devel] [PATCH hci-8.1 v3 1/6] drivers/md/push_backup: correct initialization Andrey Zhadchenko
  2026-11-03  6:17 ` [Devel] [PATCH hci-8.1 v3 2/6] drivers/md/push_backup: clear pb->alive under pb->lock Andrey Zhadchenko
@ 2026-11-03  6:17 ` Andrey Zhadchenko
  2026-11-03  6:26   ` no-reply
  2026-11-03  6:49   ` Alexander Ivanov
  2026-11-03  6:17 ` [Devel] [PATCH hci-8.1 v3 4/6] drivers/md/push_backup: fix mod_timer() usage Andrey Zhadchenko
                   ` (3 subsequent siblings)
  6 siblings, 2 replies; 18+ messages in thread
From: Andrey Zhadchenko @ 2026-11-03  6:17 UTC (permalink / raw)
  To: alexander.ivanov; +Cc: devel

pb_timer_func() checks pb->deadline_jiffies every time it fires,
so we actually don't need to re-arm the timer after setting the
filed.

This is also the only place that arms the timer outside the window
push_backup_stop() waits for with synchronize_rcu(). 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_try() once
the write path is reachable without the rwsem (next patches).

https://virtuozzo.atlassian.net/browse/VSTOR-152912
Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
---
v2: no changes
v3: no changes

 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 1ae4ccc208ffa..8ce84a1a696e5 100644
--- a/drivers/md/dm-push-backup.c
+++ b/drivers/md/dm-push-backup.c
@@ -467,8 +467,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)
@@ -519,8 +519,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;
@@ -529,11 +528,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, 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] 18+ messages in thread

* [Devel] [PATCH hci-8.1 v3 4/6] drivers/md/push_backup: fix mod_timer() usage
  2026-11-03  6:17 [Devel] [PATCH hci-8.1 v3 0/6] dm/push_backup: event fd interface and fixes Andrey Zhadchenko
                   ` (2 preceding siblings ...)
  2026-11-03  6:17 ` [Devel] [PATCH hci-8.1 v3 3/6] drivers/md/push_backup: do not re-arm deadline timer in push_backup_write() Andrey Zhadchenko
@ 2026-11-03  6:17 ` Andrey Zhadchenko
  2026-11-03  6:26   ` no-reply
  2026-11-03  6:53   ` Alexander Ivanov
  2026-11-03  6:17 ` [Devel] [PATCH hci-8.1 v3 5/6] drivers/md/push_backup: arm deadline timer under pb->lock Andrey Zhadchenko
                   ` (2 subsequent siblings)
  6 siblings, 2 replies; 18+ messages in thread
From: Andrey Zhadchenko @ 2026-11-03  6:17 UTC (permalink / raw)
  To: alexander.ivanov; +Cc: devel

mod_timer() takes an absolute jiffies value, but both callers pass a
relative one. After about 5 min uptime this causes the timer to fire
and re-arm on every tick. This has not exploded because we still have
checked pb->deadline_jiffies each time.

https://virtuozzo.atlassian.net/browse/VSTOR-152912
Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
---
v2: no changes
v3: no changes

 drivers/md/dm-push-backup.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/md/dm-push-backup.c b/drivers/md/dm-push-backup.c
index 8ce84a1a696e5..b1861204f6e3e 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)
-- 
2.43.5

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Devel] [PATCH hci-8.1 v3 5/6] drivers/md/push_backup: arm deadline timer under pb->lock
  2026-11-03  6:17 [Devel] [PATCH hci-8.1 v3 0/6] dm/push_backup: event fd interface and fixes Andrey Zhadchenko
                   ` (3 preceding siblings ...)
  2026-11-03  6:17 ` [Devel] [PATCH hci-8.1 v3 4/6] drivers/md/push_backup: fix mod_timer() usage Andrey Zhadchenko
@ 2026-11-03  6:17 ` Andrey Zhadchenko
  2026-11-03  6:28   ` no-reply
  2026-11-03  6:17 ` [Devel] [PATCH hci-8.1 v3 6/6] drivers/md/push_backup: add event file descriptor interface Andrey Zhadchenko
  2026-11-03  7:34 ` [Devel] [PATCH hci-8.1 v3 0/6] dm/push_backup: event fd interface and fixes Alexander Ivanov
  6 siblings, 1 reply; 18+ messages in thread
From: Andrey Zhadchenko @ 2026-11-03  6:17 UTC (permalink / raw)
  To: alexander.ivanov; +Cc: devel

When push_backup_write acks last delayed request it sets
deadline_jiffies = S64_MAX and the next fired timer uses this
for mod_timer(): expiry is set to some very distant future.

Previously a later push_backup_write() would accidentally
repair the expiry; since it no longer touches the timer, nothing
does until stop.

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 or
the backup is not alive. 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 racing with push_backup_stop() was never handled either:
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.
A callback blocked on pb->lock could thus outlive stop and re-arm the
timer or queue do_pb_work() after flush_workqueue(), expiring a later
session. 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: cleanup_backup() clears pb->alive under pb->lock, the timer is
only armed under pb->lock while pb->alive is set, and the callback
itself is waited for. Drop it.

https://virtuozzo.atlassian.net/browse/VSTOR-152912
Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@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

 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 b1861204f6e3e..6951e3fcdbfbf 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->alive || deadline == S64_MAX)
+		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);
@@ -628,7 +624,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] 18+ messages in thread

* [Devel] [PATCH hci-8.1 v3 6/6] drivers/md/push_backup: add event file descriptor interface
  2026-11-03  6:17 [Devel] [PATCH hci-8.1 v3 0/6] dm/push_backup: event fd interface and fixes Andrey Zhadchenko
                   ` (4 preceding siblings ...)
  2026-11-03  6:17 ` [Devel] [PATCH hci-8.1 v3 5/6] drivers/md/push_backup: arm deadline timer under pb->lock Andrey Zhadchenko
@ 2026-11-03  6:17 ` Andrey Zhadchenko
  2026-11-03  6:31   ` no-reply
  2026-11-03  7:06   ` Alexander Ivanov
  2026-11-03  7:34 ` [Devel] [PATCH hci-8.1 v3 0/6] dm/push_backup: event fd interface and fixes Alexander Ivanov
  6 siblings, 2 replies; 18+ messages in thread
From: Andrey Zhadchenko @ 2026-11-03  6:17 UTC (permalink / raw)
  To: 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

This way we will avoid aforementioned problems.

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 keep working unchanged for compatibility but we will
clean it up later.

Also destroy pbio object first before requing in resubmit_pbios():
in-flight requests hold the target which we may have already stopped,
so dm_requeue_original may trigger .dtr and invalidate mempool.

https://virtuozzo.atlassian.net/browse/VSTOR-152912
Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@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

 drivers/md/dm-push-backup.c         | 212 ++++++++++++++++++++++++----
 include/uapi/linux/dm-push-backup.h |  39 +++++
 2 files changed, 222 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 6951e3fcdbfbf..54c33fabba083 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,10 @@ 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 +430,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 +477,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 +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,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 +745,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 +763,30 @@ 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);
+	kvfree(map);
+	kvfree(pending_map);
 	kvfree(pb->pending_htable);
 	mempool_destroy(pb->pbio_pool);
 	if (pb->origin_dev)
 		dm_put_device(pb->ti, pb->origin_dev);
-	kfree(pb);
+	pb_put(pb);
 }
 
 /*
@@ -652,6 +805,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);
@@ -760,7 +914,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..764899f3525e2
--- /dev/null
+++ b/include/uapi/linux/dm-push-backup.h
@@ -0,0 +1,39 @@
+/* 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
+ * number is returned in the message result buffer.
+ *
+ * Both read() and write() operate on exactly one struct dm_push_backup_range
+ * and return -EINVAL for any other count.
+ *
+ * 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.
+ *
+ * write(fd, &range, sizeof(range)):
+ *   acknowledge that @range has been backed up; equivalent to the
+ *   "push_backup_write clu:nr" message.
+ */
+#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] 18+ messages in thread

* Re: [Devel] [PATCH hci-8.1 v3 2/6] drivers/md/push_backup: clear pb->alive under pb->lock
  2026-11-03  6:17 ` [Devel] [PATCH hci-8.1 v3 2/6] drivers/md/push_backup: clear pb->alive under pb->lock Andrey Zhadchenko
@ 2026-11-03  6:25   ` no-reply
  0 siblings, 0 replies; 18+ messages in thread
From: no-reply @ 2026-11-03  6:25 UTC (permalink / raw)
  To: Andrey Zhadchenko, 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>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261103061752.720813-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] 18+ messages in thread

* Re: [Devel] [PATCH hci-8.1 v3 1/6] drivers/md/push_backup: correct initialization
  2026-11-03  6:17 ` [Devel] [PATCH hci-8.1 v3 1/6] drivers/md/push_backup: correct initialization Andrey Zhadchenko
@ 2026-11-03  6:25   ` no-reply
  0 siblings, 0 replies; 18+ messages in thread
From: no-reply @ 2026-11-03  6:25 UTC (permalink / raw)
  To: Andrey Zhadchenko, alexander.ivanov; +Cc: devel

> Allocate pool and htable after lock, waitqueue and deadline
> timer init so we are sure that pb_destroy() can be called
> from anywhere. Otherwise we risk timer_delete_sync_try on
> zeroed timer struct (currently luckily fine).
> 
> 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/20261103061752.720813-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] 18+ messages in thread

* Re: [Devel] [PATCH hci-8.1 v3 3/6] drivers/md/push_backup: do not re-arm deadline timer in push_backup_write()
  2026-11-03  6:17 ` [Devel] [PATCH hci-8.1 v3 3/6] drivers/md/push_backup: do not re-arm deadline timer in push_backup_write() Andrey Zhadchenko
@ 2026-11-03  6:26   ` no-reply
  2026-11-03  6:49   ` Alexander Ivanov
  1 sibling, 0 replies; 18+ messages in thread
From: no-reply @ 2026-11-03  6:26 UTC (permalink / raw)
  To: Andrey Zhadchenko, alexander.ivanov; +Cc: devel

> pb_timer_func() checks pb->deadline_jiffies every time it fires,
> so we actually don't need to re-arm the timer after setting the
> filed.
> 
> This is also the only place that arms the timer outside the window
> push_backup_stop() waits for with synchronize_rcu(). 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_try() once
> the write path is reachable without the rwsem (next patches).
> 
> 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/20261103061752.720813-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] 18+ messages in thread

* Re: [Devel] [PATCH hci-8.1 v3 4/6] drivers/md/push_backup: fix mod_timer() usage
  2026-11-03  6:17 ` [Devel] [PATCH hci-8.1 v3 4/6] drivers/md/push_backup: fix mod_timer() usage Andrey Zhadchenko
@ 2026-11-03  6:26   ` no-reply
  2026-11-03  6:53   ` Alexander Ivanov
  1 sibling, 0 replies; 18+ messages in thread
From: no-reply @ 2026-11-03  6:26 UTC (permalink / raw)
  To: alexander.ivanov, Andrey Zhadchenko; +Cc: devel

> mod_timer() takes an absolute jiffies value, but both callers pass a
> relative one. After about 5 min uptime this causes the timer to fire
> and re-arm on every tick. This has not exploded because we still have
> checked pb->deadline_jiffies each time.
> 
> 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/20261103061752.720813-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] 18+ messages in thread

* Re: [Devel] [PATCH hci-8.1 v3 5/6] drivers/md/push_backup: arm deadline timer under pb->lock
  2026-11-03  6:17 ` [Devel] [PATCH hci-8.1 v3 5/6] drivers/md/push_backup: arm deadline timer under pb->lock Andrey Zhadchenko
@ 2026-11-03  6:28   ` no-reply
  0 siblings, 0 replies; 18+ messages in thread
From: no-reply @ 2026-11-03  6:28 UTC (permalink / raw)
  To: alexander.ivanov, Andrey Zhadchenko; +Cc: devel

> When push_backup_write acks last delayed request it sets
> deadline_jiffies = S64_MAX and the next fired timer uses this
> for mod_timer(): expiry is set to some very distant future.
> 
> Previously a later push_backup_write() would accidentally
> repair the expiry; since it no longer touches the timer, nothing
> does until stop.
> 
> 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 or
> the backup is not alive. 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 racing with push_backup_stop() was never handled either:
> [ ... ]
> only armed under pb->lock while pb->alive is set, and the callback
> itself is waited for. Drop it.
> 
> 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/20261103061752.720813-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] 18+ messages in thread

* Re: [Devel] [PATCH hci-8.1 v3 6/6] drivers/md/push_backup: add event file descriptor interface
  2026-11-03  6:17 ` [Devel] [PATCH hci-8.1 v3 6/6] drivers/md/push_backup: add event file descriptor interface Andrey Zhadchenko
@ 2026-11-03  6:31   ` no-reply
  2026-11-03  6:41     ` Andrey Zhadchenko
  2026-11-03  7:06   ` Alexander Ivanov
  1 sibling, 1 reply; 18+ messages in thread
From: no-reply @ 2026-11-03  6:31 UTC (permalink / raw)
  To: alexander.ivanov, Andrey Zhadchenko; +Cc: devel

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dm-push-backup: event fd pb_fops has no .poll, so epoll/poll always report ready
--

commit b2845f9ea4dfb915a0897be5e674c310ca9217b3
Author: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>

drivers/md/push_backup: add event file descriptor interface

This adds a "push_backup_get_fd" target message that returns an anon inode
fd. The backup tool can read() that fd to get the next cluster range and
write() it to acknowledge the range. This avoids sleeping inside a dm
message with md->io_barrier held. The patch also adds a kref on struct
push_backup to tie its lifetime to the open fds.

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 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);
> +}

[ ... ]

> +static const struct file_operations pb_fops = {
> +	.owner		= THIS_MODULE,
> +	.read		= pb_fd_read,
> +	.write		= pb_fd_write,
> +	.release	= pb_fd_release,
> +};

[Severity: Medium]
Is the lack of a .poll method in pb_fops intentional?

Without a ->poll handler, poll/select/epoll always report this file as
readable and writable. A tool that adds the fd to an epoll set would get an
immediate EPOLLIN. Its O_NONBLOCK read() would then return -EAGAIN, which
the UAPI header documents:

 *   -EAGAIN       - O_NONBLOCK is set and no writes are pending.

Polling again would return immediately, so the tool would spin. The only
usable way to wait is a blocking read() in a dedicated thread.

pb_has_event() already defines the readiness condition, and pb->waitq is
already woken on the relevant state changes. Would it make sense to add a
.poll method built on pb_has_event() and pb->waitq? Adding it later would
change how this fd behaves for userspace that already uses it.

Also, pb_fops has no .llseek = no_llseek, and the file is not opened with
stream_open() or nonseekable_open() semantics. Should this stream-like fd
be marked non-seekable?

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261103061752.720813-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] 18+ messages in thread

* Re: [Devel] [PATCH hci-8.1 v3 6/6] drivers/md/push_backup: add event file descriptor interface
  2026-11-03  6:31   ` no-reply
@ 2026-11-03  6:41     ` Andrey Zhadchenko
  0 siblings, 0 replies; 18+ messages in thread
From: Andrey Zhadchenko @ 2026-11-03  6:41 UTC (permalink / raw)
  To: no-reply, alexander.ivanov; +Cc: devel



On 11/3/26 07:31, no-reply@sashiko.virtuozzo.com wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] dm-push-backup: event fd pb_fops has no .poll, so epoll/poll always report ready

We are not planning to use it in non-blocking mode, so this is fine. If 
we really need that we will add this later

> --
> 
> commit b2845f9ea4dfb915a0897be5e674c310ca9217b3
> Author: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
> 
> drivers/md/push_backup: add event file descriptor interface
> 
> This adds a "push_backup_get_fd" target message that returns an anon inode
> fd. The backup tool can read() that fd to get the next cluster range and
> write() it to acknowledge the range. This avoids sleeping inside a dm
> message with md->io_barrier held. The patch also adds a kref on struct
> push_backup to tie its lifetime to the open fds.
> 
> 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 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);
>> +}
> 
> [ ... ]
> 
>> +static const struct file_operations pb_fops = {
>> +	.owner		= THIS_MODULE,
>> +	.read		= pb_fd_read,
>> +	.write		= pb_fd_write,
>> +	.release	= pb_fd_release,
>> +};
> 
> [Severity: Medium]
> Is the lack of a .poll method in pb_fops intentional?
> 
> Without a ->poll handler, poll/select/epoll always report this file as
> readable and writable. A tool that adds the fd to an epoll set would get an
> immediate EPOLLIN. Its O_NONBLOCK read() would then return -EAGAIN, which
> the UAPI header documents:
> 
>   *   -EAGAIN       - O_NONBLOCK is set and no writes are pending.
> 
> Polling again would return immediately, so the tool would spin. The only
> usable way to wait is a blocking read() in a dedicated thread.
> 
> pb_has_event() already defines the readiness condition, and pb->waitq is
> already woken on the relevant state changes. Would it make sense to add a
> .poll method built on pb_has_event() and pb->waitq? Adding it later would
> change how this fd behaves for userspace that already uses it.
> 
> Also, pb_fops has no .llseek = no_llseek, and the file is not opened with
> stream_open() or nonseekable_open() semantics. Should this stream-like fd
> be marked non-seekable?
> 

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Devel] [PATCH hci-8.1 v3 3/6] drivers/md/push_backup: do not re-arm deadline timer in push_backup_write()
  2026-11-03  6:17 ` [Devel] [PATCH hci-8.1 v3 3/6] drivers/md/push_backup: do not re-arm deadline timer in push_backup_write() Andrey Zhadchenko
  2026-11-03  6:26   ` no-reply
@ 2026-11-03  6:49   ` Alexander Ivanov
  1 sibling, 0 replies; 18+ messages in thread
From: Alexander Ivanov @ 2026-11-03  6:49 UTC (permalink / raw)
  To: Andrey Zhadchenko; +Cc: devel


On 11/3/26 07:17, Andrey Zhadchenko wrote:
> pb_timer_func() checks pb->deadline_jiffies every time it fires,
> so we actually don't need to re-arm the timer after setting the
> filed.
filed - > field?
>
> This is also the only place that arms the timer outside the window
> push_backup_stop() waits for with synchronize_rcu(). 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_try() once
> the write path is reachable without the rwsem (next patches).
>
> https://virtuozzo.atlassian.net/browse/VSTOR-152912
> Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
> ---
> v2: no changes
> v3: no changes
>
>   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 1ae4ccc208ffa..8ce84a1a696e5 100644
> --- a/drivers/md/dm-push-backup.c
> +++ b/drivers/md/dm-push-backup.c
> @@ -467,8 +467,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)
> @@ -519,8 +519,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;
> @@ -529,11 +528,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, pb->timeout_in_jiffies + 1);
> -	}
>   
>   	return 0;
>   }
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Devel] [PATCH hci-8.1 v3 4/6] drivers/md/push_backup: fix mod_timer() usage
  2026-11-03  6:17 ` [Devel] [PATCH hci-8.1 v3 4/6] drivers/md/push_backup: fix mod_timer() usage Andrey Zhadchenko
  2026-11-03  6:26   ` no-reply
@ 2026-11-03  6:53   ` Alexander Ivanov
  1 sibling, 0 replies; 18+ messages in thread
From: Alexander Ivanov @ 2026-11-03  6:53 UTC (permalink / raw)
  To: Andrey Zhadchenko; +Cc: devel


On 11/3/26 07:17, Andrey Zhadchenko wrote:
> mod_timer() takes an absolute jiffies value, but both callers pass a
> relative one. After about 5 min uptime this causes the timer to fire
> and re-arm on every tick. This has not exploded because we still have
> checked pb->deadline_jiffies each time.
>
> https://virtuozzo.atlassian.net/browse/VSTOR-152912
Fixes: 62e07487348e ("dm: Add push backup driver")
> Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
> ---
> v2: no changes
> v3: no changes
>
>   drivers/md/dm-push-backup.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/md/dm-push-backup.c b/drivers/md/dm-push-backup.c
> index 8ce84a1a696e5..b1861204f6e3e 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)
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Devel] [PATCH hci-8.1 v3 6/6] drivers/md/push_backup: add event file descriptor interface
  2026-11-03  6:17 ` [Devel] [PATCH hci-8.1 v3 6/6] drivers/md/push_backup: add event file descriptor interface Andrey Zhadchenko
  2026-11-03  6:31   ` no-reply
@ 2026-11-03  7:06   ` Alexander Ivanov
  1 sibling, 0 replies; 18+ messages in thread
From: Alexander Ivanov @ 2026-11-03  7:06 UTC (permalink / raw)
  To: Andrey Zhadchenko; +Cc: devel


On 11/3/26 07:17, Andrey Zhadchenko wrote:
> 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
>
> This way we will avoid aforementioned problems.
>
> 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 keep working unchanged for compatibility but we will
> clean it up later.
>
> Also destroy pbio object first before requing in resubmit_pbios():
Typo: requing -> requeuing
> in-flight requests hold the target which we may have already stopped,
> so dm_requeue_original may trigger .dtr and invalidate mempool.
dm_requeue_original - > dm_requeue_original_rq()?
>
> https://virtuozzo.atlassian.net/browse/VSTOR-152912
> Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@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
>
>   drivers/md/dm-push-backup.c         | 212 ++++++++++++++++++++++++----
>   include/uapi/linux/dm-push-backup.h |  39 +++++
>   2 files changed, 222 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 6951e3fcdbfbf..54c33fabba083 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,10 @@ 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 +430,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 +477,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 +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,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 +745,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 +763,30 @@ 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);
> +	kvfree(map);
> +	kvfree(pending_map);
>   	kvfree(pb->pending_htable);
>   	mempool_destroy(pb->pbio_pool);
>   	if (pb->origin_dev)
>   		dm_put_device(pb->ti, pb->origin_dev);
> -	kfree(pb);
> +	pb_put(pb);
>   }
>   
>   /*
> @@ -652,6 +805,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);
> @@ -760,7 +914,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..764899f3525e2
> --- /dev/null
> +++ b/include/uapi/linux/dm-push-backup.h
> @@ -0,0 +1,39 @@
> +/* 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
> + * number is returned in the message result buffer.
> + *
> + * Both read() and write() operate on exactly one struct dm_push_backup_range
> + * and return -EINVAL for any other count.
> + *
> + * 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.
> + *
> + * write(fd, &range, sizeof(range)):
> + *   acknowledge that @range has been backed up; equivalent to the
> + *   "push_backup_write clu:nr" message.
> + */
> +#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 */
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Devel] [PATCH hci-8.1 v3 0/6] dm/push_backup: event fd interface and fixes
  2026-11-03  6:17 [Devel] [PATCH hci-8.1 v3 0/6] dm/push_backup: event fd interface and fixes Andrey Zhadchenko
                   ` (5 preceding siblings ...)
  2026-11-03  6:17 ` [Devel] [PATCH hci-8.1 v3 6/6] drivers/md/push_backup: add event file descriptor interface Andrey Zhadchenko
@ 2026-11-03  7:34 ` Alexander Ivanov
  6 siblings, 0 replies; 18+ messages in thread
From: Alexander Ivanov @ 2026-11-03  7:34 UTC (permalink / raw)
  To: Andrey Zhadchenko; +Cc: devel

LGTM with minor notes.

Reviewed-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>

On 11/3/26 07:17, Andrey Zhadchenko wrote:
> 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 6 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 returns -ENODEV afterwards. The legacy messages are kept for
> compatibility.
>
> Patches 1-5 are fixes found while working on this:
>
>    1. init the lock, waitqueue and timer before anything that can fail so
>       pb_destroy() is safe from any error path;
>    2. clear pb->alive under pb->lock like the other users do;
>    3. drop the redundant mod_timer() in push_backup_write(). The timer
>       re-arms itself from pb->deadline_jiffies, so updating the deadline
>       under the lock is enough. This was also the only place that armed
>       the timer outside the window push_backup_stop() waits for, which
>       matters once writes may arrive via the fd without pb->ctl_rwsem;
>    4. pass absolute expiry to mod_timer(). Both remaining callers passed
>       a relative value, so after ~5 min of uptime 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 a stale S64_MAX could
>       overwrite a fresh deadline and park the timer ~12 days out with
>       delayed writes still pending. Also, timer_delete_sync_try() does not
>       wait for a running callback, so one blocked on pb->lock could outlive
>       push_backup_stop() and expire a later session. With arming done under
>       the lock, the callback checking pb->alive 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.
>
> 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 (6):
>    drivers/md/push_backup: correct initialization
>    drivers/md/push_backup: clear pb->alive under pb->lock
>    drivers/md/push_backup: do not re-arm deadline timer in
>      push_backup_write()
>    drivers/md/push_backup: fix mod_timer() usage
>    drivers/md/push_backup: arm deadline timer under pb->lock
>    drivers/md/push_backup: add event file descriptor interface
>
>   drivers/md/dm-push-backup.c         | 268 +++++++++++++++++++++-------
>   include/uapi/linux/dm-push-backup.h |  39 ++++
>   2 files changed, 246 insertions(+), 61 deletions(-)
>   create mode 100644 include/uapi/linux/dm-push-backup.h
>
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2026-11-03  7:36 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-03  6:17 [Devel] [PATCH hci-8.1 v3 0/6] dm/push_backup: event fd interface and fixes Andrey Zhadchenko
2026-11-03  6:17 ` [Devel] [PATCH hci-8.1 v3 1/6] drivers/md/push_backup: correct initialization Andrey Zhadchenko
2026-11-03  6:25   ` no-reply
2026-11-03  6:17 ` [Devel] [PATCH hci-8.1 v3 2/6] drivers/md/push_backup: clear pb->alive under pb->lock Andrey Zhadchenko
2026-11-03  6:25   ` no-reply
2026-11-03  6:17 ` [Devel] [PATCH hci-8.1 v3 3/6] drivers/md/push_backup: do not re-arm deadline timer in push_backup_write() Andrey Zhadchenko
2026-11-03  6:26   ` no-reply
2026-11-03  6:49   ` Alexander Ivanov
2026-11-03  6:17 ` [Devel] [PATCH hci-8.1 v3 4/6] drivers/md/push_backup: fix mod_timer() usage Andrey Zhadchenko
2026-11-03  6:26   ` no-reply
2026-11-03  6:53   ` Alexander Ivanov
2026-11-03  6:17 ` [Devel] [PATCH hci-8.1 v3 5/6] drivers/md/push_backup: arm deadline timer under pb->lock Andrey Zhadchenko
2026-11-03  6:28   ` no-reply
2026-11-03  6:17 ` [Devel] [PATCH hci-8.1 v3 6/6] drivers/md/push_backup: add event file descriptor interface Andrey Zhadchenko
2026-11-03  6:31   ` no-reply
2026-11-03  6:41     ` Andrey Zhadchenko
2026-11-03  7:06   ` Alexander Ivanov
2026-11-03  7:34 ` [Devel] [PATCH hci-8.1 v3 0/6] dm/push_backup: event fd interface and fixes Alexander Ivanov

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.