All Virtuozzo development lists (kernel + QEMU)
 help / color / mirror / Atom feed
* [Devel] [PATCH hci-8.1 v4 0/6] dm/push_backup: event fd interface and fixes
@ 2026-11-03  8:03 Andrey Zhadchenko
  2026-11-03  8:03 ` [Devel] [PATCH hci-8.1 v4 1/6] drivers/md/push_backup: correct initialization Andrey Zhadchenko
                   ` (6 more replies)
  0 siblings, 7 replies; 21+ messages in thread
From: Andrey Zhadchenko @ 2026-11-03  8:03 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 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.

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

* [Devel] [PATCH hci-8.1 v4 1/6] drivers/md/push_backup: correct initialization
  2026-11-03  8:03 [Devel] [PATCH hci-8.1 v4 0/6] dm/push_backup: event fd interface and fixes Andrey Zhadchenko
@ 2026-11-03  8:03 ` Andrey Zhadchenko
  2026-11-03  8:11   ` no-reply
  2026-11-03 10:11   ` Konstantin Khorenko
  2026-11-03  8:03 ` [Devel] [PATCH hci-8.1 v4 2/6] drivers/md/push_backup: clear pb->alive under pb->lock Andrey Zhadchenko
                   ` (5 subsequent siblings)
  6 siblings, 2 replies; 21+ messages in thread
From: Andrey Zhadchenko @ 2026-11-03  8:03 UTC (permalink / raw)
  To: khorenko, 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>
Reviewed-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
v2: no changes
v3: no changes
v4: 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] 21+ messages in thread

* [Devel] [PATCH hci-8.1 v4 2/6] drivers/md/push_backup: clear pb->alive under pb->lock
  2026-11-03  8:03 [Devel] [PATCH hci-8.1 v4 0/6] dm/push_backup: event fd interface and fixes Andrey Zhadchenko
  2026-11-03  8:03 ` [Devel] [PATCH hci-8.1 v4 1/6] drivers/md/push_backup: correct initialization Andrey Zhadchenko
@ 2026-11-03  8:03 ` Andrey Zhadchenko
  2026-11-03  8:11   ` no-reply
  2026-11-03 10:11   ` Konstantin Khorenko
  2026-11-03  8:03 ` [Devel] [PATCH hci-8.1 v4 3/6] drivers/md/push_backup: do not re-arm deadline timer in push_backup_write() Andrey Zhadchenko
                   ` (4 subsequent siblings)
  6 siblings, 2 replies; 21+ messages in thread
From: Andrey Zhadchenko @ 2026-11-03  8:03 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

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

* [Devel] [PATCH hci-8.1 v4 3/6] drivers/md/push_backup: do not re-arm deadline timer in push_backup_write()
  2026-11-03  8:03 [Devel] [PATCH hci-8.1 v4 0/6] dm/push_backup: event fd interface and fixes Andrey Zhadchenko
  2026-11-03  8:03 ` [Devel] [PATCH hci-8.1 v4 1/6] drivers/md/push_backup: correct initialization Andrey Zhadchenko
  2026-11-03  8:03 ` [Devel] [PATCH hci-8.1 v4 2/6] drivers/md/push_backup: clear pb->alive under pb->lock Andrey Zhadchenko
@ 2026-11-03  8:03 ` Andrey Zhadchenko
  2026-11-03  8:13   ` no-reply
  2026-11-03 10:11   ` Konstantin Khorenko
  2026-11-03  8:03 ` [Devel] [PATCH hci-8.1 v4 4/6] drivers/md/push_backup: fix mod_timer() usage Andrey Zhadchenko
                   ` (3 subsequent siblings)
  6 siblings, 2 replies; 21+ messages in thread
From: Andrey Zhadchenko @ 2026-11-03  8:03 UTC (permalink / raw)
  To: khorenko, 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
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>
Reviewed-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
v2: no changes
v3: no changes
v4: commit message typo

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

* [Devel] [PATCH hci-8.1 v4 4/6] drivers/md/push_backup: fix mod_timer() usage
  2026-11-03  8:03 [Devel] [PATCH hci-8.1 v4 0/6] dm/push_backup: event fd interface and fixes Andrey Zhadchenko
                   ` (2 preceding siblings ...)
  2026-11-03  8:03 ` [Devel] [PATCH hci-8.1 v4 3/6] drivers/md/push_backup: do not re-arm deadline timer in push_backup_write() Andrey Zhadchenko
@ 2026-11-03  8:03 ` Andrey Zhadchenko
  2026-11-03  8:12   ` no-reply
  2026-11-03 10:11   ` Konstantin Khorenko
  2026-11-03  8:03 ` [Devel] [PATCH hci-8.1 v4 5/6] drivers/md/push_backup: arm deadline timer under pb->lock Andrey Zhadchenko
                   ` (2 subsequent siblings)
  6 siblings, 2 replies; 21+ messages in thread
From: Andrey Zhadchenko @ 2026-11-03  8:03 UTC (permalink / raw)
  To: khorenko, 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.

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

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

* [Devel] [PATCH hci-8.1 v4 5/6] drivers/md/push_backup: arm deadline timer under pb->lock
  2026-11-03  8:03 [Devel] [PATCH hci-8.1 v4 0/6] dm/push_backup: event fd interface and fixes Andrey Zhadchenko
                   ` (3 preceding siblings ...)
  2026-11-03  8:03 ` [Devel] [PATCH hci-8.1 v4 4/6] drivers/md/push_backup: fix mod_timer() usage Andrey Zhadchenko
@ 2026-11-03  8:03 ` Andrey Zhadchenko
  2026-11-03  8:14   ` no-reply
  2026-11-03 10:11   ` Konstantin Khorenko
  2026-11-03  8:03 ` [Devel] [PATCH hci-8.1 v4 6/6] drivers/md/push_backup: add event file descriptor interface Andrey Zhadchenko
  2026-11-03 10:11 ` [Devel] [PATCH hci-8.1 v4 0/6] dm/push_backup: event fd interface and fixes Konstantin Khorenko
  6 siblings, 2 replies; 21+ messages in thread
From: Andrey Zhadchenko @ 2026-11-03  8:03 UTC (permalink / raw)
  To: khorenko, 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>
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

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

* [Devel] [PATCH hci-8.1 v4 6/6] drivers/md/push_backup: add event file descriptor interface
  2026-11-03  8:03 [Devel] [PATCH hci-8.1 v4 0/6] dm/push_backup: event fd interface and fixes Andrey Zhadchenko
                   ` (4 preceding siblings ...)
  2026-11-03  8:03 ` [Devel] [PATCH hci-8.1 v4 5/6] drivers/md/push_backup: arm deadline timer under pb->lock Andrey Zhadchenko
@ 2026-11-03  8:03 ` Andrey Zhadchenko
  2026-11-03  8:17   ` no-reply
  2026-11-03 10:11   ` Konstantin Khorenko
  2026-11-03 10:11 ` [Devel] [PATCH hci-8.1 v4 0/6] dm/push_backup: event fd interface and fixes Konstantin Khorenko
  6 siblings, 2 replies; 21+ messages in thread
From: Andrey Zhadchenko @ 2026-11-03  8:03 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

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 requeuing in resubmit_pbios():
in-flight requests hold the target which we may have already stopped,
so dm_requeue_original_rq() may trigger .dtr and invalidate mempool.

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

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

* Re: [Devel] [PATCH hci-8.1 v4 1/6] drivers/md/push_backup: correct initialization
  2026-11-03  8:03 ` [Devel] [PATCH hci-8.1 v4 1/6] drivers/md/push_backup: correct initialization Andrey Zhadchenko
@ 2026-11-03  8:11   ` no-reply
  2026-11-03 10:11   ` Konstantin Khorenko
  1 sibling, 0 replies; 21+ messages in thread
From: no-reply @ 2026-11-03  8:11 UTC (permalink / raw)
  To: Andrey Zhadchenko, alexander.ivanov, khorenko; +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/20261103080358.723186-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] 21+ messages in thread

* Re: [Devel] [PATCH hci-8.1 v4 2/6] drivers/md/push_backup: clear pb->alive under pb->lock
  2026-11-03  8:03 ` [Devel] [PATCH hci-8.1 v4 2/6] drivers/md/push_backup: clear pb->alive under pb->lock Andrey Zhadchenko
@ 2026-11-03  8:11   ` no-reply
  2026-11-03 10:11   ` Konstantin Khorenko
  1 sibling, 0 replies; 21+ messages in thread
From: no-reply @ 2026-11-03  8:11 UTC (permalink / raw)
  To: Andrey Zhadchenko, alexander.ivanov, khorenko; +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/20261103080358.723186-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] 21+ messages in thread

* Re: [Devel] [PATCH hci-8.1 v4 4/6] drivers/md/push_backup: fix mod_timer() usage
  2026-11-03  8:03 ` [Devel] [PATCH hci-8.1 v4 4/6] drivers/md/push_backup: fix mod_timer() usage Andrey Zhadchenko
@ 2026-11-03  8:12   ` no-reply
  2026-11-03 10:11   ` Konstantin Khorenko
  1 sibling, 0 replies; 21+ messages in thread
From: no-reply @ 2026-11-03  8:12 UTC (permalink / raw)
  To: alexander.ivanov, Andrey Zhadchenko, khorenko; +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.
> 
> 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/20261103080358.723186-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] 21+ messages in thread

* Re: [Devel] [PATCH hci-8.1 v4 3/6] drivers/md/push_backup: do not re-arm deadline timer in push_backup_write()
  2026-11-03  8:03 ` [Devel] [PATCH hci-8.1 v4 3/6] drivers/md/push_backup: do not re-arm deadline timer in push_backup_write() Andrey Zhadchenko
@ 2026-11-03  8:13   ` no-reply
  2026-11-03 10:11   ` Konstantin Khorenko
  1 sibling, 0 replies; 21+ messages in thread
From: no-reply @ 2026-11-03  8:13 UTC (permalink / raw)
  To: Andrey Zhadchenko, khorenko, 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
> 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>

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

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

* Re: [Devel] [PATCH hci-8.1 v4 5/6] drivers/md/push_backup: arm deadline timer under pb->lock
  2026-11-03  8:03 ` [Devel] [PATCH hci-8.1 v4 5/6] drivers/md/push_backup: arm deadline timer under pb->lock Andrey Zhadchenko
@ 2026-11-03  8:14   ` no-reply
  2026-11-03 10:11   ` Konstantin Khorenko
  1 sibling, 0 replies; 21+ messages in thread
From: no-reply @ 2026-11-03  8:14 UTC (permalink / raw)
  To: alexander.ivanov, Andrey Zhadchenko, khorenko; +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/20261103080358.723186-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] 21+ messages in thread

* Re: [Devel] [PATCH hci-8.1 v4 6/6] drivers/md/push_backup: add event file descriptor interface
  2026-11-03  8:03 ` [Devel] [PATCH hci-8.1 v4 6/6] drivers/md/push_backup: add event file descriptor interface Andrey Zhadchenko
@ 2026-11-03  8:17   ` no-reply
  2026-11-03 10:11   ` Konstantin Khorenko
  1 sibling, 0 replies; 21+ messages in thread
From: no-reply @ 2026-11-03  8:17 UTC (permalink / raw)
  To: khorenko, Andrey Zhadchenko, alexander.ivanov; +Cc: devel

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Low] dm-push-backup: pb_fops has no .poll, so the event fd cannot be polled
- [Low] dm-push-backup: push_backup_write() checks range before -ENODEV/-ESTALE
- [Low] dm push_backup: fd_install() in message handler precedes ioctl result copy-out
--

commit 34e71a14b728d9c4ab310c5d9fc752f38ad72889
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 range to back up from the fd
and write() to acknowledge it, instead of sleeping inside a dm message. A
kref on struct push_backup ties the lifetime to the number of 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

[ ... ]

> @@ -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->destroyed) {
> +		spin_unlock_irq(&pb->lock);
> +		return -ENODEV;
> +	}

[Severity: Low]
Can the -ENODEV and -ESTALE results documented for write() on the fd be
hidden here?

In push_backup_write(), the range check returns -E2BIG before pb->lock is
taken and before pb->destroyed and "!pb->map || !pb->alive" are tested:

	if (clu >= nr_clus || nr > nr_clus - clu)
		return -E2BIG;

	spin_lock_irq(&pb->lock);
	if (pb->destroyed) {

A caller of pb_fd_write() that passes an out of range value to a stopped or
destroyed target gets -E2BIG instead of -ESTALE or -ENODEV. Would it make
sense to test the target state first, or to document the ordering in the
UAPI header?

[ ... ]

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

[Severity: Low]
This isn't a bug, but pb_fops has no .poll callback. The UAPI header
documents -EAGAIN for O_NONBLOCK read(), yet the fd cannot be used with
poll, select or epoll.

Without it, how is a single-threaded tool expected to wait for readiness
on this fd? It would need a dedicated blocking thread or a busy loop on
O_NONBLOCK read().

pb->waitq and pb_has_event() already exist, so would a .poll implementation
based on poll_wait(file, &pb->waitq, wait) and pb_has_event() be
reasonable here?

> +static int push_backup_get_fd(struct push_backup *pb,
> +			      char *result, unsigned int maxlen)
> +{

[ ... ]

> +	fd_install(fd, file);
> +	return 1;
> +}

[Severity: Low]
Can this leave an installed fd whose number is never delivered to
userspace?

push_backup_get_fd() calls fd_install() and returns 1 from within the
message handler. The result buffer is copied out by dm-ioctl only after
the target returns:

drivers/md/dm-ioctl.c:ctl_ioctl() {
    ...
	if (!r && copy_to_user(user, param, param->data_size))
		r = -EFAULT;
    ...
}

If this copy_to_user() fails, for example because the user buffer is
unmapped or read-only, the ioctl returns -EFAULT. The fd stays installed
in the caller's fd table and nothing rolls it back.

The fd holds a kref on struct push_backup and a module reference on
dm-push-backup through anon_inode_getfile(). The target and the module
therefore stay pinned until the process exits or closes the fd by guessing
its number.

Only a CAP_SYS_ADMIN caller passing a bad buffer can reach this. Would it
be worth documenting this behaviour, or is there a way to defer
fd_install() until the result has been delivered?

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

* Re: [Devel] [PATCH hci-8.1 v4 1/6] drivers/md/push_backup: correct initialization
  2026-11-03  8:03 ` [Devel] [PATCH hci-8.1 v4 1/6] drivers/md/push_backup: correct initialization Andrey Zhadchenko
  2026-11-03  8:11   ` no-reply
@ 2026-11-03 10:11   ` Konstantin Khorenko
  1 sibling, 0 replies; 21+ messages in thread
From: Konstantin Khorenko @ 2026-11-03 10:11 UTC (permalink / raw)
  To: Andrey Zhadchenko; +Cc: devel

On 11/3/26 11:03, Andrey Zhadchenko wrote:
> 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).

Hi Andrey,

The code change is fine. On the message:

- "from anywhere" is broader than the patch: it makes pb_destroy() safe
  on every error path of pb_ctr().
- "luckily fine" holds for release kernels only. Our debug config has
  CONFIG_DEBUG_OBJECTS_TIMERS=y, and there debug_assert_init() reports
  the zeroed timer with an ODEBUG warning.

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

Not introduced here, but one more error path of the same function is
broken. Further down:

	ret = dm_get_device(ti, argv[1], dm_table_get_mode(ti->table),
			    &pb->origin_dev);
	...
	pb->wq = alloc_ordered_workqueue("dm-" DM_MSG_PREFIX, WQ_MEM_RECLAIM);
	if (!pb->wq) {
		ti->error = "could not create workqueue for metadata object";
		goto err;
	}

ret is still 0 from dm_get_device(), so a failed
alloc_ordered_workqueue() frees pb and pb_ctr() returns success. The
table keeps ti->private pointing to freed memory, and pb_dtr() destroys
it a second time. It dates back to 62e07487348e ("dm: Add push backup
driver"). Could you set ret = -ENOMEM there, in this patch or a
separate one?

With the message adjusted:

Reviewed-by: Konstantin Khorenko <khorenko@virtuozzo.com>

-- 
Konstantin Khorenko,
Virtuozzo Linux Kernel Team

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

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

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

On 11/3/26 11:03, Andrey Zhadchenko wrote:
> Patches 1-5 are fixes found while working on this:

Hi Andrey,

Thanks, the direction is right and the fd interface itself holds up.
Remarks are inline in the patches, the main ones:

- 5/6: the !pb->alive bail-out in pb_timer_func() can leave guest
  writes delayed for good after an interrupted "push_backup_stop".
  Checking pb->nr_delayed instead keeps everything the patch needs.
- 3/6 opens a window that only 5/6 closes, so please put 5/6 first or
  fold 3/6 into it.
- 6/6: the fd is not bound to a backup session; please decide this
  before the UAPI is fixed.
- pb_ctr() returns 0 with a freed pb when alloc_ordered_workqueue()
  fails. It is older than the series, but 1/6 is about exactly these
  error paths.

Series-wide:
- Every vz commit needs a "Feature:" line. The earlier dm-push-backup
  commits carry "Feature: dm-backup: push backup target driver".
- The subject prefix of the driver history and of this cover letter is
  "dm/push_backup:", the patches use "drivers/md/push_backup:". The 3/6
  subject is 75 characters.

>   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;

Same as in 3/6: pb_timer_func() also re-arms outside that window, so
"the only place" does not hold.

>   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;

On 64-bit this happens from boot, not after 5 minutes, see 4/6.

>   5. arm the timer under pb->lock and wait for its callback properly.

This item is accurate and is the description 5/6 itself should carry.
It is one paragraph of six sentences though; splitting it into the
race, the stop race and the RCU removal would read easier.

-- 
Konstantin Khorenko,
Virtuozzo Linux Kernel Team

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

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

* Re: [Devel] [PATCH hci-8.1 v4 2/6] drivers/md/push_backup: clear pb->alive under pb->lock
  2026-11-03  8:03 ` [Devel] [PATCH hci-8.1 v4 2/6] drivers/md/push_backup: clear pb->alive under pb->lock Andrey Zhadchenko
  2026-11-03  8:11   ` no-reply
@ 2026-11-03 10:11   ` Konstantin Khorenko
  1 sibling, 0 replies; 21+ messages in thread
From: Konstantin Khorenko @ 2026-11-03 10:11 UTC (permalink / raw)
  To: Andrey Zhadchenko; +Cc: devel

On 11/3/26 11:03, Andrey Zhadchenko wrote:
> Most other users handle this under the pb->lock so let's
> make it a bit more consistent.

Reviewed-by: Konstantin Khorenko <khorenko@virtuozzo.com>

-- 
Konstantin Khorenko,
Virtuozzo Linux Kernel Team

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

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

* Re: [Devel] [PATCH hci-8.1 v4 5/6] drivers/md/push_backup: arm deadline timer under pb->lock
  2026-11-03  8:03 ` [Devel] [PATCH hci-8.1 v4 5/6] drivers/md/push_backup: arm deadline timer under pb->lock Andrey Zhadchenko
  2026-11-03  8:14   ` no-reply
@ 2026-11-03 10:11   ` Konstantin Khorenko
  1 sibling, 0 replies; 21+ messages in thread
From: Konstantin Khorenko @ 2026-11-03 10:11 UTC (permalink / raw)
  To: Andrey Zhadchenko; +Cc: devel

On 11/3/26 11:03, Andrey Zhadchenko wrote:
> 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.

Hi Andrey,

When the callback parks the timer this way, nothing is delayed, and the
next delayed write takes the nr_delayed == 1 branch in
postpone_if_required_for_backup(), which arms the timer itself. So this
case is harmless and "nothing does until stop" does not hold. The real
bug is the race item 5 of your cover letter describes: the callback
reads S64_MAX under the lock, postpone arms a fresh deadline, and the
late mod_timer() of the callback overwrites it while writes are
delayed. Could these two paragraphs describe that instead?

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

A small precision: synchronize_rcu() already waited for a callback that
was running, or spinning on pb->lock, when it started, because the
timer softirq is an RCU reader. What escaped is a firing that starts
after the grace period.

> @@ -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;

pb->alive is not cleared by cleanup_backup() only. pb_message() clears
it for "push_backup_stop" before it takes ctl_rwsem, on purpose since
6cd7e75f682f ("dm/push_backup: Allow to stop push_backup at any
time"):

	spin_lock_irq(&pb->lock);
	pb->alive = false;
	spin_unlock_irq(&pb->lock);
	...
	ret = down_write_killable(&pb->ctl_rwsem);
	...
	if (unlikely(ret))
		goto out;

If the stopping tool gets a fatal signal while the rwsem is held by
another message, push_backup_stop() never runs. alive is false,
nr_delayed is non-zero and the deadline is real. Before this patch the
timer still expired it and do_pb_work() released the guest writes.
With this hunk the callback returns and does not re-arm, read() and
write() on the fd return -ESTALE, and the writes hang until somebody
sends "push_backup_stop" again. dm suspend, "@stats_create" and remove
wait for them meanwhile.

Checking nr_delayed instead keeps what the patch relies on:

	if (!pb->nr_delayed)
		goto unlock;

cleanup_backup() zeroes nr_delayed under the same lock hold that clears
alive, and postpone only delays while alive. So after stop the callback
still neither re-arms nor queues work, and in the state above it expires
the deadline as before. While alive, deadline == S64_MAX is the same as
nr_delayed == 0.

> -	WARN_ON_ONCE(timer_delete_sync_try(&pb->deadline_timer) < 0);
> +	timer_shutdown_sync(&pb->deadline_timer);

This also fixes a use-after-free: removing the target while the
callback ran made pb_destroy() WARN and go on to destroy_workqueue()
and kfree(pb) under it. Both that and the stop race came with the
driver, so please add:

Fixes: 62e07487348e ("dm: Add push backup driver")

-- 
Konstantin Khorenko,
Virtuozzo Linux Kernel Team

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

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

* Re: [Devel] [PATCH hci-8.1 v4 4/6] drivers/md/push_backup: fix mod_timer() usage
  2026-11-03  8:03 ` [Devel] [PATCH hci-8.1 v4 4/6] drivers/md/push_backup: fix mod_timer() usage Andrey Zhadchenko
  2026-11-03  8:12   ` no-reply
@ 2026-11-03 10:11   ` Konstantin Khorenko
  1 sibling, 0 replies; 21+ messages in thread
From: Konstantin Khorenko @ 2026-11-03 10:11 UTC (permalink / raw)
  To: Andrey Zhadchenko; +Cc: devel

On 11/3/26 11:03, 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.

Hi Andrey,

The fix is right, the "5 min" is not. INITIAL_JIFFIES is
(unsigned long)(unsigned int)(-300*HZ): on 64-bit jiffies start at
about 2^32 - 300*HZ, and after 5 minutes only the low 32 bits wrap,
the unsigned long keeps growing. The relative values here are at most
5h*HZ, below the start value, so they are in the past from boot, and
the timer fires and re-arms on every tick whenever a write is delayed.
The late first expiry before the wrap exists on 32-bit only. Maybe:

  mod_timer() takes an absolute jiffies value, but both 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.

With that:

Reviewed-by: Konstantin Khorenko <khorenko@virtuozzo.com>

-- 
Konstantin Khorenko,
Virtuozzo Linux Kernel Team

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

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

* Re: [Devel] [PATCH hci-8.1 v4 3/6] drivers/md/push_backup: do not re-arm deadline timer in push_backup_write()
  2026-11-03  8:03 ` [Devel] [PATCH hci-8.1 v4 3/6] drivers/md/push_backup: do not re-arm deadline timer in push_backup_write() Andrey Zhadchenko
  2026-11-03  8:13   ` no-reply
@ 2026-11-03 10:11   ` Konstantin Khorenko
  1 sibling, 0 replies; 21+ messages in thread
From: Konstantin Khorenko @ 2026-11-03 10:11 UTC (permalink / raw)
  To: Andrey Zhadchenko; +Cc: devel

On 11/3/26 11:03, 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
> field.

Hi Andrey,

That holds only while the timer is pending whenever writes are delayed,
and at this point of the series it is not always so. Both arming sites
still decide under pb->lock and call mod_timer() after dropping it:

  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 write is delayed with the timer parked for days. Before this patch
the next push_backup_write() that left requests delayed re-armed the
timer and repaired it. After it nothing does until nr_delayed drops to
zero or stop. 5/6 closes the race, so please put 5/6 before this patch
or fold the two, so that no bisect point carries the window.

> This is also the only place that arms the timer outside the window
> push_backup_stop() waits for with synchronize_rcu().

pb_timer_func() re-arms the timer outside that window too: a firing
that starts after synchronize_rcu() runs concurrently with
timer_delete_sync_try(). Please drop "the only place" here and in the
cover letter.

-- 
Konstantin Khorenko,
Virtuozzo Linux Kernel Team

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

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

* Re: [Devel] [PATCH hci-8.1 v4 6/6] drivers/md/push_backup: add event file descriptor interface
  2026-11-03  8:03 ` [Devel] [PATCH hci-8.1 v4 6/6] drivers/md/push_backup: add event file descriptor interface Andrey Zhadchenko
  2026-11-03  8:17   ` no-reply
@ 2026-11-03 10:11   ` Konstantin Khorenko
  2026-11-03 12:38     ` Andrey Zhadchenko
  1 sibling, 1 reply; 21+ messages in thread
From: Konstantin Khorenko @ 2026-11-03 10:11 UTC (permalink / raw)
  To: Andrey Zhadchenko; +Cc: devel

On 11/3/26 11:03, Andrey Zhadchenko wrote:
> The legacy messages keep working unchanged for compatibility but we will
> clean it up later.

Hi Andrey,

I found no locking or lifetime bug here: the kref, pb->destroyed and
the reserve -> emit -> check -> install order in push_backup_get_fd()
hold up. A few things to settle before the UAPI is fixed.

The legacy path does change, for the better: pb_message() parsed u64
and passed it on as unsigned int, so "push_backup_write 4294967296:1"
acked cluster 0 and now gets -E2BIG, and push_backup_read printed "%u"
on devices with 2^32 or more clusters. Please say so instead of
"unchanged", or move these into a separate fix of 62e07487348e.

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

The requests pin the mapped device (dm_start_request() -> dm_get()),
not the target. Only the fd write path needs this, as it holds no md
reference. This could also be a small preparatory patch.

> +	ret = -ESTALE;
> +	if (!pb->map || !pb->alive)

The fd is bound to the target, not to a backup session. After
"push_backup_stop" and a new "push_backup_start" an old fd keeps
working on the new session. A reader blocked across the stop can even
miss -ESTALE: if start has set alive again before it re-checks
pb_has_event(), it sleeps on. A leftover tool then acks ranges of the
new backup, every ack clears map bits, and the new backup silently
loses its point-in-time consistency. The legacy messages behave the
same, so it is not a regression, but a new UAPI cannot be tightened
later. Could the file record a session generation, bumped on start and
stop, and return -ESTALE once it is over? If you keep it as is, please
say in the header that the fd survives stop/start.

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

read() honours O_NONBLOCK, but without .poll the fd cannot go into
epoll and poll() reports it always ready. pb->waitq and pb_has_event()
are already there, so .poll is a few lines.

On the header: userspace gets -1 and errno, not "-EAGAIN"; read() can
also fail with EBADMSG, EFAULT and EINTR; the fd is O_CLOEXEC. The fd
is installed before dm-ioctl copies the result out, so a failed
copy-out leaves it behind, and "dmsetup message ... push_backup_get_fd"
is of no use. One line on each would help tool authors.

After pb_destroy() the struct may live on through an fd with ti,
origin_dev, wq, pending_htable and pbio_pool dangling. Every fd path
checks pb->destroyed first, so this is fine today; NULLing them would
keep it so.

>  	while ((pbio = list_first_entry_or_null(list, struct pb_bio, list)) != NULL) {
> +		struct request *rq = pbio->rq;
>  		list_del_init(&pbio->list);

checkpatch wants a blank line after the declaration.

Commit message: "This way we will avoid aforementioned problems." can
go. Please also mention the target version bump to 1.1.0. Is that what
the tool checks to find the fd interface, or does it just try
"push_backup_get_fd" and fall back on -ENOTSUPP?

-- 
Konstantin Khorenko,
Virtuozzo Linux Kernel Team

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

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

* Re: [Devel] [PATCH hci-8.1 v4 6/6] drivers/md/push_backup: add event file descriptor interface
  2026-11-03 10:11   ` Konstantin Khorenko
@ 2026-11-03 12:38     ` Andrey Zhadchenko
  0 siblings, 0 replies; 21+ messages in thread
From: Andrey Zhadchenko @ 2026-11-03 12:38 UTC (permalink / raw)
  To: Konstantin Khorenko; +Cc: devel



On 11/3/26 11:11, Konstantin Khorenko wrote:
> On 11/3/26 11:03, Andrey Zhadchenko wrote:
>> The legacy messages keep working unchanged for compatibility but we will
>> clean it up later.
> 
> Hi Andrey,
> 
> I found no locking or lifetime bug here: the kref, pb->destroyed and
> the reserve -> emit -> check -> install order in push_backup_get_fd()
> hold up. A few things to settle before the UAPI is fixed.
> 
> The legacy path does change, for the better: pb_message() parsed u64
> and passed it on as unsigned int, so "push_backup_write 4294967296:1"
> acked cluster 0 and now gets -E2BIG, and push_backup_read printed "%u"
> on devices with 2^32 or more clusters. Please say so instead of
> "unchanged", or move these into a separate fix of 62e07487348e.
> 
>> Also destroy pbio object first before requeuing in resubmit_pbios():
>> in-flight requests hold the target which we may have already stopped,
>> so dm_requeue_original_rq() may trigger .dtr and invalidate mempool.
> 
> The requests pin the mapped device (dm_start_request() -> dm_get()),
> not the target. Only the fd write path needs this, as it holds no md
> reference. This could also be a small preparatory patch.
> 
>> +	ret = -ESTALE;
>> +	if (!pb->map || !pb->alive)
> 
> The fd is bound to the target, not to a backup session. After
> "push_backup_stop" and a new "push_backup_start" an old fd keeps
> working on the new session. A reader blocked across the stop can even
> miss -ESTALE: if start has set alive again before it re-checks
> pb_has_event(), it sleeps on. A leftover tool then acks ranges of the
> new backup, every ack clears map bits, and the new backup silently
> loses its point-in-time consistency. The legacy messages behave the
> same, so it is not a regression, but a new UAPI cannot be tightened
> later. Could the file record a session generation, bumped on start and
> stop, and return -ESTALE once it is over? If you keep it as is, please
> say in the header that the fd survives stop/start.

Hypothetical situation that has little to do with kernel code in my 
opinion. Surprisingly the userspace code must also be competent to not 
blow itself. I do not believe this justifies extra complexity in 
handling this fds.

> 
>> +static const struct file_operations pb_fops = {
>> +	.owner		= THIS_MODULE,
>> +	.read		= pb_fd_read,
>> +	.write		= pb_fd_write,
>> +	.release	= pb_fd_release,
>> +};
> 
> read() honours O_NONBLOCK, but without .poll the fd cannot go into
> epoll and poll() reports it always ready. pb->waitq and pb_has_event()
> are already there, so .poll is a few lines.

It is the few lines no one will use. We have a separate threads and this 
is better already: poll adds one more sycall; polling different disks in 
one consumer increases latency for everyone.

> 
> On the header: userspace gets -1 and errno, not "-EAGAIN"; read() can
> also fail with EBADMSG, EFAULT and EINTR; the fd is O_CLOEXEC. The fd
> is installed before dm-ioctl copies the result out, so a failed
> copy-out leaves it behind, and "dmsetup message ... push_backup_get_fd"
> is of no use. One line on each would help tool authors.
> 
> After pb_destroy() the struct may live on through an fd with ti,
> origin_dev, wq, pending_htable and pbio_pool dangling. Every fd path
> checks pb->destroyed first, so this is fine today; NULLing them would
> keep it so.
> 
>>   	while ((pbio = list_first_entry_or_null(list, struct pb_bio, list)) != NULL) {
>> +		struct request *rq = pbio->rq;
>>   		list_del_init(&pbio->list);
> 
> checkpatch wants a blank line after the declaration.
> 
> Commit message: "This way we will avoid aforementioned problems." can
> go. Please also mention the target version bump to 1.1.0. Is that what
> the tool checks to find the fd interface, or does it just try
> "push_backup_get_fd" and fall back on -ENOTSUPP?
> 

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

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

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

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-03  8:03 [Devel] [PATCH hci-8.1 v4 0/6] dm/push_backup: event fd interface and fixes Andrey Zhadchenko
2026-11-03  8:03 ` [Devel] [PATCH hci-8.1 v4 1/6] drivers/md/push_backup: correct initialization Andrey Zhadchenko
2026-11-03  8:11   ` no-reply
2026-11-03 10:11   ` Konstantin Khorenko
2026-11-03  8:03 ` [Devel] [PATCH hci-8.1 v4 2/6] drivers/md/push_backup: clear pb->alive under pb->lock Andrey Zhadchenko
2026-11-03  8:11   ` no-reply
2026-11-03 10:11   ` Konstantin Khorenko
2026-11-03  8:03 ` [Devel] [PATCH hci-8.1 v4 3/6] drivers/md/push_backup: do not re-arm deadline timer in push_backup_write() Andrey Zhadchenko
2026-11-03  8:13   ` no-reply
2026-11-03 10:11   ` Konstantin Khorenko
2026-11-03  8:03 ` [Devel] [PATCH hci-8.1 v4 4/6] drivers/md/push_backup: fix mod_timer() usage Andrey Zhadchenko
2026-11-03  8:12   ` no-reply
2026-11-03 10:11   ` Konstantin Khorenko
2026-11-03  8:03 ` [Devel] [PATCH hci-8.1 v4 5/6] drivers/md/push_backup: arm deadline timer under pb->lock Andrey Zhadchenko
2026-11-03  8:14   ` no-reply
2026-11-03 10:11   ` Konstantin Khorenko
2026-11-03  8:03 ` [Devel] [PATCH hci-8.1 v4 6/6] drivers/md/push_backup: add event file descriptor interface Andrey Zhadchenko
2026-11-03  8:17   ` no-reply
2026-11-03 10:11   ` Konstantin Khorenko
2026-11-03 12:38     ` Andrey Zhadchenko
2026-11-03 10:11 ` [Devel] [PATCH hci-8.1 v4 0/6] 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.