[Devel] [PATCH hci-8.1 v6 0/7] dm/push_backup: event fd interface and fixes

[Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>]

The push_backup target currently makes the backup tool wait for the next
cluster range inside the "push_backup_read" target message. dm runs
target_message() under dm_get_live_table(), i.e. with md->io_barrier SRCU
read-locked, so while the tool sleeps there anything that needs
synchronize_srcu(&md->io_barrier) on that device is stuck: suspend, table
reload/remove and dm_internal_suspend_fast() from "@stats_create". An
unrelated dm-stats user hangs, and everything that later queues behind
md->suspend_lock hangs with it.

Patch 7 adds a file descriptor based interface instead. The tool gets an
anon inode fd via the "push_backup_get_fd" message and then uses plain
read()/write() of struct dm_push_backup_range to fetch the next range and
to acknowledge it, without going through dm messages on the data path. The
fd pins struct push_backup with a kref so it survives target removal and
simply fails with ENODEV afterwards. The fd is bound to the target, not to
a backup session. The legacy messages are kept for compatibility.

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

  1. init the lock, waitqueue and timer before anything that can fail so
     pb_destroy() is safe on every error path of pb_ctr();
  2. return -ENOMEM when the workqueue allocation fails. pb_ctr()
     returned 0 after freeing pb, and pb_dtr() freed it again;
  3. clear pb->alive under pb->lock like the other users do;
  4. pass absolute expiry to mod_timer(). All callers passed a relative
     value, which is always in the past, so the timer fired and re-armed
     on every tick while a write was delayed;
  5. arm the timer under pb->lock and wait for its callback properly.
     pb_timer_func() and postpone_if_required_for_backup() both decided
     under the lock and armed after dropping it, so the callback's stale
     S64_MAX could overwrite a fresh deadline while writes were delayed.
     timer_delete_sync_try() does not wait for a running callback, so a
     firing after the synchronize_rcu() grace period could outlive
     push_backup_stop() and expire a later session, or run under
     pb_destroy() and use pb after it is freed. With arming done under
     the lock, the callback checking pb->nr_delayed and stop/destroy
     waiting via timer_delete_sync()/timer_shutdown_sync(), the RCU dance
     between postpone and stop is no longer needed and is removed;
  6. drop the redundant mod_timer() in push_backup_write(). The timer
     re-arms itself from pb->deadline_jiffies while writes are delayed,
     so updating the deadline under the lock is enough. The call was not
     under pb->lock, which matters once writes may arrive via the fd
     without pb->ctl_rwsem.

v6:
  - all: subject prefix changed to dm/push_backup, added Feature: tag.
  - 7/7: dropped O_NONBLOCK support, read() always blocks.

v5:
  - 1/7: commit message: safe on every error path of pb_ctr(), the
    zeroed timer triggers an ODEBUG warning on debug kernels.
  - 2/7: new patch.
  - 4/7, 5/7, 6/7: reordered so that the timer is armed under pb->lock
    before push_backup_write() stops re-arming it; no bisect point has
    the race window. 4/7 now also fixes the mod_timer() call in
    push_backup_write().
  - 4/7: commit message: the relative expiry is always in the past.
  - 5/7: pb_timer_func() checks pb->nr_delayed instead of pb->alive, so
    a "push_backup_stop" interrupted before taking pb->ctl_rwsem still
    expires the deadline. Commit message describes the race, the
    synchronize_rcu() window and the pb_destroy() use-after-free. Added
    Fixes: tag.
  - 6/7: dropped the "only place" claim.
  - 7/7: header documents that the fd survives stop/start, errno
    semantics, the EINTR/EFAULT/EBADMSG and write() errors, O_CLOEXEC
    and that "dmsetup message" is of no use. pb_destroy() NULLs the
    pointers it frees. checkpatch blank line. Commit message mentions
    the legacy u64 parsing changes and the version bump, and corrects
    the resubmit_pbios() rationale.

Patch numbers below refer to the 6-patch v4 numbering.

v4:
  - 3/6, 6/6: commit message typos.
  - 4/6: added Fixes: tag.

v3:
  - 5/6: pb_timer_func() bails out when !pb->alive; push_backup_stop()
    uses timer_delete_sync() and pb_destroy() timer_shutdown_sync()
    instead of WARNing on timer_delete_sync_try() < 0, which never waited
    for a running callback. Commit message corrected accordingly.
  - 6/6: push_backup_get_fd() installs the fd only after its number fits
    into the result buffer (reserve -> emit -> check -> install), so a
    too small buffer no longer leaks the fd with its pb kref and module
    reference. pb->dead renamed to pb->destroyed. UAPI type namespaced
    as struct dm_push_backup_range. read() now requires exactly
    sizeof(range) like write(). Subject prefix aligned with the rest of
    the series.

v2:
  - 2/6: reworded the commit message, dropped the KCSAN claim.
  - 5/6: new patch: arm the deadline timer under pb->lock and drop the
    rcu_read_lock()/synchronize_rcu() between
    postpone_if_required_for_backup() and push_backup_stop().
  - 6/6: resubmit_pbios() now frees the pbio before
    dm_requeue_original_rq(). The requeue drops the md reference held by
    the in-flight request, after which __dm_destroy() may run .dtr and
    destroy pbio_pool under the fd writer's feet.

https://virtuozzo.atlassian.net/browse/VSTOR-152912

Andrey Zhadchenko (7):
  dm/push_backup: correct initialization
  dm/push_backup: return error if workqueue allocation fails
  dm/push_backup: clear pb->alive under pb->lock
  dm/push_backup: fix mod_timer() usage
  dm/push_backup: arm deadline timer under pb->lock
  dm/push_backup: do not re-arm deadline timer in push_backup_write()
  dm/push_backup: add event file descriptor interface

 drivers/md/dm-push-backup.c         | 273 +++++++++++++++++++++-------
 include/uapi/linux/dm-push-backup.h |  47 +++++
 2 files changed, 259 insertions(+), 61 deletions(-)
 create mode 100644 include/uapi/linux/dm-push-backup.h

-- 
2.43.5

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