[Devel] [PATCH hci-8.1 v2 5/6] drivers/md/push_backup: arm deadline timer under pb->lock

[Andrey Zhadchenko]

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

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
re-arming itself is handled by timer_delete_sync_try(). Drop it.

https://virtuozzo.atlassian.net/browse/VSTOR-152912
Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko at virtuozzo.com>
---
v2: new patch

 drivers/md/dm-push-backup.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/md/dm-push-backup.c b/drivers/md/dm-push-backup.c
index b1861204f6e3e..8f2015e8e29a4 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 (deadline == S64_MAX)
+		goto unlock; /* Nothing is delayed, let the timer idle */
 	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,8 +398,6 @@ 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);
 	flush_workqueue(pb->wq);
 
-- 
2.43.5