[Devel] [PATCH hci-8.1 0/2] dm-ploop: fix writes held on ENOSPC never being retried

[Devel] [PATCH hci-8.1 0/2] dm-ploop: fix writes held on ENOSPC never being retried

[Denis V. Lunev]

A write that fails with ENOSPC on the image is held and retried by
enospc_timer, so that I/O resumes once space is freed. The timer only
sets a flag that the dispatcher consumes, and nothing wakes the
dispatcher, so the held writes are retried only when other I/O reaches
the device. On a device whose only I/O is the held writes, the writers
stay in D state for good after the space is freed (patch 2,
VSTOR-154182).

Patch 1 makes the destructor shut the timer down first. Nothing stops
it there today, and with patch 2 the callback dereferences the
dispatcher thread as well as struct ploop.

Found in a vmcore: 16 writers in D state, submit_enospc set, all 16
pios on enospc_pios, the timer expired 12 s before the crash.

Tested on 6.12.0-211.52.1.22.19.vz10, dm-ploop built out of tree:
- 16 O_DIRECT writers held on a full host ext4, then the space freed
  with no other I/O on the device. Stock: still stuck after 60 s, one
  unrelated read releases them. With the series: done 8 s after the
  space is freed, on the next timer tick;
- ploop test suite, make -j48 test on dev-cl04, green for ploop and
  qcow2 with this series together with the VSTOR-154096 one.

Denis V. Lunev (2):
  dm-ploop: stop enospc_timer in ploop_destroy()
  dm-ploop: fix the ENOSPC retry timer not waking the dispatcher

 drivers/md/dm-ploop-map.c    | 9 +++++----
 drivers/md/dm-ploop-target.c | 1 +
 2 files changed, 6 insertions(+), 4 deletions(-)

-- 
2.53.0

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

[Devel] [PATCH hci-8.1 1/2] dm-ploop: stop enospc_timer in ploop_destroy()

[Denis V. Lunev]

From: Denis V. Lunev <den@openvz.org>

The destructor never stops enospc_timer. ploop_presuspend() deletes it
with timer_delete_sync_try(), which does not wait for a callback that
runs on another CPU, and ploop_try_delay_enospc() tests wants_suspend
without deferred_lock, so a write that fails with ENOSPC around the
presuspend can arm the timer again. The callback dereferences the
struct ploop that ploop_dtr() frees.

Shut the timer down at the start of ploop_destroy(), which waits for a
running callback and keeps the timer from being armed again. The timer
is set up before kt_worker is created, so it is valid wherever
ploop_destroy() finds kt_worker set.

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

Fixes: 6ddad5a4d226 ("dm-ploop: Add ploop target driver")
Feature: dm-ploop: ploop target driver
Signed-off-by: Denis V. Lunev <den@openvz.org>
---
 drivers/md/dm-ploop-target.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/md/dm-ploop-target.c b/drivers/md/dm-ploop-target.c
index d8279c79ef56..5ab25cbd5c32 100644
--- a/drivers/md/dm-ploop-target.c
+++ b/drivers/md/dm-ploop-target.c
@@ -164,6 +164,7 @@ static void ploop_destroy(struct ploop *ploop)
 	int i;
 
 	if (ploop->kt_worker) {
+		timer_shutdown_sync(&ploop->enospc_timer);
 		ploop->force_md_writeback = 1;
 		wake_up_process(ploop->kt_worker->task);
 		/* try to send all pending - if we have partial io and enospc end bellow */
-- 
2.53.0

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

Re: [Devel] [PATCH hci-8.1 1/2] dm-ploop: stop enospc_timer in ploop_destroy()

[no-reply]

> From: Denis V. Lunev <den@openvz.org>
> 
> The destructor never stops enospc_timer. ploop_presuspend() deletes it
> with timer_delete_sync_try(), which does not wait for a callback that
> runs on another CPU, and ploop_try_delay_enospc() tests wants_suspend
> without deferred_lock, so a write that fails with ENOSPC around the
> presuspend can arm the timer again. The callback dereferences the
> struct ploop that ploop_dtr() frees.
> 
> Shut the timer down at the start of ploop_destroy(), which waits for a
> running callback and keeps the timer from being armed again. The timer
> is set up before kt_worker is created, so it is valid wherever
> ploop_destroy() finds kt_worker set.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-154182
> 
> Fixes: 6ddad5a4d226 ("dm-ploop: Add ploop target driver")
> Feature: dm-ploop: ploop target driver
> Signed-off-by: Denis V. Lunev <den@openvz.org>

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

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261109190657.1851487-1-den@openvz.org?part=1


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

[Devel] [PATCH hci-8.1 2/2] dm-ploop: fix the ENOSPC retry timer not waking the dispatcher

[Denis V. Lunev]

From: Denis V. Lunev <den@openvz.org>

A write that fails with ENOSPC on the image is held on enospc_pios and
retried when enospc_timer fires, so that I/O resumes once space is
freed. ploop_enospc_timer() only sets submit_enospc, which only
do_ploop_run_work() consumes, and nothing wakes the dispatcher for it.
ploop_worker() also goes to sleep with the flag set, as its sleep
condition checks the pio lists only. The held writes are retried only
when other I/O reaches the device: on a device whose only I/O is the
held writes, the writers stay in D state after the space is freed.

Wake the dispatcher from the timer, and keep ploop_worker() awake while
submit_enospc is set, so that a wakeup that comes while the dispatcher
is busy is not lost. Test and clear the flag with xchg() in
do_ploop_run_work(), as the timer now sets it from another CPU.

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

Fixes: afc60480b037 ("dm-ploop: resubmit enospc pios from dispatcher thread")
Feature: dm-ploop: ploop target driver
Signed-off-by: Denis V. Lunev <den@openvz.org>
---
 drivers/md/dm-ploop-map.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/md/dm-ploop-map.c b/drivers/md/dm-ploop-map.c
index ae3e287604b7..a4e17aa090a6 100644
--- a/drivers/md/dm-ploop-map.c
+++ b/drivers/md/dm-ploop-map.c
@@ -165,7 +165,9 @@ static void ploop_init_prq_and_embedded_pio(struct ploop *ploop,
 void ploop_enospc_timer(struct timer_list *timer)
 {
 	struct ploop *ploop = timer_container_of(ploop, timer, enospc_timer);
-	ploop->submit_enospc = true;
+
+	WRITE_ONCE(ploop->submit_enospc, true);
+	wake_up_process(ploop->kt_worker->task);
 }
 
 void do_ploop_event_work(struct work_struct *ws)
@@ -2241,10 +2243,8 @@ static void do_ploop_run_work(struct ploop *ploop)
 
 	current->flags |= PLOOP_PFLAGS;
 
-	if (ploop->submit_enospc) {
-		ploop->submit_enospc = false;
+	if (xchg(&ploop->submit_enospc, false))
 		ploop_resubmit_enospc_pios(ploop);
-	}
 
 	llembedded_pios = llist_del_all(&ploop->pios[PLOOP_LIST_PREPARE]);
 
@@ -2447,6 +2447,7 @@ int ploop_worker(void *data)
 		    llist_empty(&ploop->pios[PLOOP_LIST_DISCARD]) &&
 		    llist_empty(&ploop->pios[PLOOP_LIST_COW]) &&
 		    llist_empty(&ploop->llresubmit_pios) &&
+		    !READ_ONCE(ploop->submit_enospc) &&
 		    !ploop->force_md_writeback) {
 			if (kthread_should_stop()) {
 				wait_event_interruptible(
-- 
2.53.0

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

Re: [Devel] [PATCH hci-8.1 2/2] dm-ploop: fix the ENOSPC retry timer not waking the dispatcher

[no-reply]

> From: Denis V. Lunev <den@openvz.org>
> 
> A write that fails with ENOSPC on the image is held on enospc_pios and
> retried when enospc_timer fires, so that I/O resumes once space is
> freed. ploop_enospc_timer() only sets submit_enospc, which only
> do_ploop_run_work() consumes, and nothing wakes the dispatcher for it.
> ploop_worker() also goes to sleep with the flag set, as its sleep
> condition checks the pio lists only. The held writes are retried only
> when other I/O reaches the device: on a device whose only I/O is the
> held writes, the writers stay in D state after the space is freed.
> 
> Wake the dispatcher from the timer, and keep ploop_worker() awake while
> submit_enospc is set, so that a wakeup that comes while the dispatcher
> is busy is not lost. Test and clear the flag with xchg() in
> do_ploop_run_work(), as the timer now sets it from another CPU.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-154182
> 
> Fixes: afc60480b037 ("dm-ploop: resubmit enospc pios from dispatcher thread")
> Feature: dm-ploop: ploop target driver
> Signed-off-by: Denis V. Lunev <den@openvz.org>

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

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261109190657.1851487-1-den@openvz.org?part=2


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