Re: [Devel] [PATCH hci-8.1 v3 6/6] drivers/md/push_backup: add event file descriptor interface

[Alexander Ivanov <alexander.ivanov@virtuozzo.com>]

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