[Devel] [PATCH RH7 2/2] ploop: Wait till fsync_thread goes to sleep on ploop_quiesce()

Kirill Tkhai ktkhai at virtuozzo.com
Wed Sep 23 19:12:40 MSK 2020


fsync_thread not only makes sync(), but it also may modify data.
See in kaio_fsync_thread: kaio_resubmit() submits writes, and
this is not agreed with another points of synchronizations.
This is a problem on backup (but not only), as ploop_quiesce()
never waits for fsync_thread, and we may see unexpected data
changes.

This patch makes ploop_quiesce() also wait for fsync_thread:
till last preq was submitted.

https://jira.sw.ru/browse/PSBM-108101

Signed-off-by: Kirill Tkhai <ktkhai at virtuozzo.com>
---
 drivers/block/ploop/dev.c       |   30 ++++++++++++++++++++++++++++++
 drivers/block/ploop/io_direct.c |    9 ++++++++-
 drivers/block/ploop/io_kaio.c   |   23 +++++++++++++++++------
 3 files changed, 55 insertions(+), 7 deletions(-)

diff --git a/drivers/block/ploop/dev.c b/drivers/block/ploop/dev.c
index ac4d142197d9..097ae6c73aac 100644
--- a/drivers/block/ploop/dev.c
+++ b/drivers/block/ploop/dev.c
@@ -224,6 +224,23 @@ static int disable_and_wait_discard(struct ploop_device *plo)
 	return ret;
 }
 
+static bool has_pending_fsync_reqs(struct ploop_device *plo, struct ploop_io *io)
+{
+	unsigned long flags;
+	bool ret;
+
+	spin_lock_irqsave(&plo->lock, flags);
+	ret = !list_empty(&io->fsync_queue);
+	spin_unlock_irqrestore(&plo->lock, flags);
+
+	return ret;
+}
+
+static void wait_fsync_thread(struct ploop_device *plo, struct ploop_io *io)
+{
+	wait_event(plo->waitq, !has_pending_fsync_reqs(plo, io));
+}
+
 static void ploop_init_request(struct ploop_request *preq)
 {
 	preq->eng_state = PLOOP_E_ENTRY;
@@ -3602,6 +3619,7 @@ void ploop_quiesce(struct ploop_device * plo)
 {
 	struct completion qcomp;
 	struct ploop_request * preq;
+	struct ploop_io *io;
 
 	if (!test_bit(PLOOP_S_RUNNING, &plo->state))
 		return;
@@ -3628,6 +3646,18 @@ void ploop_quiesce(struct ploop_device * plo)
 
 	wait_fast_path_reqs(plo);
 	wait_for_completion(&qcomp);
+
+	/*
+	 * Main thread is sleeping, so no one may submit a new preq
+	 * for fsync_thread, except delayed timer. Let it fire.
+	 */
+	io = &ploop_top_delta(plo)->io;
+	if (io->fsync_timer.function) {
+		del_timer_sync(&io->fsync_timer);
+		io->fsync_timer.function(io->fsync_timer.data);
+	}
+	wait_fsync_thread(plo, io);
+
 	plo->quiesce_comp = NULL;
 }
 EXPORT_SYMBOL(ploop_quiesce);
diff --git a/drivers/block/ploop/io_direct.c b/drivers/block/ploop/io_direct.c
index 4f82b8b5ce36..f67ce47e0562 100644
--- a/drivers/block/ploop/io_direct.c
+++ b/drivers/block/ploop/io_direct.c
@@ -785,8 +785,9 @@ static int dio_fsync_thread(void * data)
 
 	spin_lock_irq(&plo->lock);
 	while (!kthread_should_stop() || !list_empty(&io->fsync_queue)) {
-		int err;
+		struct list_head fake_entry;
 		LIST_HEAD(list);
+		int err;
 
 		DEFINE_WAIT(_wait);
 		for (;;) {
@@ -806,6 +807,11 @@ static int dio_fsync_thread(void * data)
 
 		INIT_LIST_HEAD(&list);
 		list_splice_init(&io->fsync_queue, &list);
+		/*
+		 * Not empty io->fsync_queue means fsync_thread has
+		 * pending work. See ploop_quiesce() for details.
+		 */
+		list_add(&fake_entry, &io->fsync_queue);
 		io_count = io->io_count;
 		spin_unlock_irq(&plo->lock);
 
@@ -822,6 +828,7 @@ static int dio_fsync_thread(void * data)
 		 */
 
 		spin_lock_irq(&plo->lock);
+		list_del(&fake_entry);
 
 		if (io_count == io->io_count && !(io_count & 1))
 			clear_bit(PLOOP_IO_FSYNC_DELAYED, &io->io_state);
diff --git a/drivers/block/ploop/io_kaio.c b/drivers/block/ploop/io_kaio.c
index c2b7e8feaedf..2c2fb90d2b53 100644
--- a/drivers/block/ploop/io_kaio.c
+++ b/drivers/block/ploop/io_kaio.c
@@ -497,8 +497,10 @@ static int kaio_fsync_thread(void * data)
 
 	spin_lock_irq(&plo->lock);
 	while (!kthread_should_stop() || !list_empty(&io->fsync_queue)) {
+		struct list_head fake_entry;
+		struct ploop_request *preq;
+		bool skip_wakeup = false;
 		int err;
-		struct ploop_request * preq;
 
 		DEFINE_WAIT(_wait);
 		for (;;) {
@@ -518,6 +520,12 @@ static int kaio_fsync_thread(void * data)
 
 		preq = list_entry(io->fsync_queue.next, struct ploop_request, list);
 		list_del(&preq->list);
+		/*
+		 * Not empty io->fsync_queue means fsync_thread has pending work.
+		 * See ploop_quiesce() for details.
+		 */
+		list_add(&fake_entry, &io->fsync_queue);
+
 		io->fsync_qlen--;
 		if (!preq->prealloc_size)
 			plo->st.bio_fsync++;
@@ -557,20 +565,22 @@ static int kaio_fsync_thread(void * data)
 				preq->req_rw &= ~REQ_FLUSH;
 				/*
 				 * FIXME: We submit some data and main thread
-				 * is not synchronized with this? Also,
-				 * TODO: fsync_thread must care of ploop_quiesce().
+				 * is not synchronized with this?
 				 */
 				if (kaio_resubmit(preq)) {
 					spin_lock_irq(&plo->lock);
-					continue;
+					if (!plo->quiesce_comp)
+						skip_wakeup = true;
+					goto del_fake;
 				}
 			}
 		}
 ready:
 		spin_lock_irq(&plo->lock);
 		list_add_tail(&preq->list, &plo->ready_queue);
-
-		if (waitqueue_active(&plo->waitq))
+del_fake:
+		list_del(&fake_entry);
+		if (waitqueue_active(&plo->waitq) && !skip_wakeup)
 			wake_up_interruptible(&plo->waitq);
 	}
 	spin_unlock_irq(&plo->lock);
@@ -688,6 +698,7 @@ kaio_init(struct ploop_io * io)
 {
 	INIT_LIST_HEAD(&io->fsync_queue);
 	init_waitqueue_head(&io->fsync_waitq);
+	io->fsync_timer.function = NULL;
 
 	return 0;
 }




More information about the Devel mailing list