[Devel] [PATCH hci-8.1 v3 1/5] dm-ploop: fix lost wakeup in ploop_allocator()

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

ploop_allocator() checks prealloc_size and kthread_should_stop() before
setting TASK_INTERRUPTIBLE. If ploop_req_prealloc() sets prealloc_size
and calls wake_up_process() in between, the wakeup finds the task
running and is lost, so the allocator goes to sleep with a pending
preallocation request. Later requests for the same size return early
without waking it up, so the request may stay pending forever and
waiters on dispatcher_wq_prealloc hang. The same applies to
kthread_stop().

Set the task state before checking the conditions, as the canonical
kthread loop does. set_current_state() implies a full barrier, which
pairs with the one in try_to_wake_up().

https://virtuozzo.atlassian.net/browse/VSTOR-148171
Feature: dm-ploop: ploop target driver

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
 drivers/md/dm-ploop-map.c | 49 ++++++++++++++++++++++-----------------
 1 file changed, 28 insertions(+), 21 deletions(-)

diff --git a/drivers/md/dm-ploop-map.c b/drivers/md/dm-ploop-map.c
index 48466435dd19..a3a5393e533d 100644
--- a/drivers/md/dm-ploop-map.c
+++ b/drivers/md/dm-ploop-map.c
@@ -2347,31 +2347,38 @@ int ploop_allocator(void *data)
 	struct file *file;
 
 	for (;;) {
-		/* we do not care about exact number here - risk one extra schedule wrt taking the lock */
-		if (ploop->prealloc_size) {
-			__set_current_state(TASK_RUNNING);
-			file = ploop_top_delta(ploop)->file;
-			current->flags |= PLOOP_PFLAGS;
-			ret = ploop_preallocate_cluster(ploop, file);
-			if (ret) {
-				/*
-				 * threads requesting prealloc must check
-				 * if size actually changed
-				 */
-				/* ENOSPC is only for fallocate */
-				pr_err_ratelimited(
-					PL_FMT("failed to allocate space: %d\n"),
-					       ploop_device_name(ploop), ret);
-			}
-			wake_up_interruptible(&ploop->dispatcher_wq_prealloc);
-			current_restore_flags(old_flags, PLOOP_PFLAGS);
-		}
+		/*
+		 * Set the state before checking the conditions, otherwise
+		 * wake_up_process() from ploop_req_prealloc() or
+		 * kthread_stop() may come in between and get lost.
+		 */
+		set_current_state(TASK_INTERRUPTIBLE);
 		if (kthread_should_stop()) {
 			__set_current_state(TASK_RUNNING);
 			break;
 		}
-		set_current_state(TASK_INTERRUPTIBLE);
-		schedule();
+		/* we do not care about exact number here - risk one extra schedule wrt taking the lock */
+		if (!READ_ONCE(ploop->prealloc_size)) {
+			schedule();
+			continue;
+		}
+		__set_current_state(TASK_RUNNING);
+
+		file = ploop_top_delta(ploop)->file;
+		current->flags |= PLOOP_PFLAGS;
+		ret = ploop_preallocate_cluster(ploop, file);
+		if (ret) {
+			/*
+			 * threads requesting prealloc must check
+			 * if size actually changed
+			 */
+			/* ENOSPC is only for fallocate */
+			pr_err_ratelimited(
+				PL_FMT("failed to allocate space: %d\n"),
+				       ploop_device_name(ploop), ret);
+		}
+		wake_up_interruptible(&ploop->dispatcher_wq_prealloc);
+		current_restore_flags(old_flags, PLOOP_PFLAGS);
 	}
 
 	return 0;
-- 
2.53.0

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