From: Bart Van Assche <bvanassche@acm.org>
Date: Sat, 13 Jun 2020 18:23:39 +0000 (-0700)
Subject: Fix a potentially infinite loop in check_overlap()
X-Git-Tag: fio-3.21~15^2~2
X-Git-Url: https://git.kernel.dk/?a=commitdiff_plain;h=c76b661c50fdd085f8ac08c0028b5083b238b8e8;p=fio.git

Fix a potentially infinite loop in check_overlap()

If the following happens:
* check_overlap() finds an overlap.
* All other threads finish after the overlap has been found and before
  the next iteration of the do/while loop starts.

Then the do/while loop in check_overlap() will iterate forever. Fix this
by rewriting check_overlap() such that this cannot happen.

This patch fixes the following Coverity complaint:

CID 184174 (#2 of 2): Double lock (LOCK)

Fixes: c06379a65d46 ("fio: enable overlap checking with offload submission")
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---

diff --git a/rate-submit.c b/rate-submit.c
index cf00d9bc..3bcb5053 100644
--- a/rate-submit.c
+++ b/rate-submit.c
@@ -13,38 +13,36 @@ static void check_overlap(struct io_u *io_u)
 {
 	int i;
 	struct thread_data *td;
-	bool overlap = false;
 
-	do {
-		/*
-		 * Allow only one thread to check for overlap at a
-		 * time to prevent two threads from thinking the coast
-		 * is clear and then submitting IOs that overlap with
-		 * each other
-		 *
-		 * If an overlap is found, release the lock and
-		 * re-acquire it before checking again to give other
-		 * threads a chance to make progress
-		 *
-		 * If an overlap is not found, release the lock when the
-		 * io_u's IO_U_F_FLIGHT flag is set so that this io_u
-		 * can be checked by other threads as they assess overlap
-		 */
+	/*
+	 * Allow only one thread to check for overlap at a time to prevent two
+	 * threads from thinking the coast is clear and then submitting IOs
+	 * that overlap with each other.
+	 *
+	 * If an overlap is found, release the lock and re-acquire it before
+	 * checking again to give other threads a chance to make progress.
+	 *
+	 * If no overlap is found, release the lock when the io_u's
+	 * IO_U_F_FLIGHT flag is set so that this io_u can be checked by other
+	 * threads as they assess overlap.
+	 */
+	pthread_mutex_lock(&overlap_check);
+
+retry:
+	for_each_td(td, i) {
+		if (td->runstate <= TD_SETTING_UP ||
+		    td->runstate >= TD_FINISHING ||
+		    !td->o.serialize_overlap ||
+		    td->o.io_submit_mode != IO_MODE_OFFLOAD)
+			continue;
+
+		if (!in_flight_overlap(&td->io_u_all, io_u))
+			continue;
+
+		pthread_mutex_unlock(&overlap_check);
 		pthread_mutex_lock(&overlap_check);
-		for_each_td(td, i) {
-			if (td->runstate <= TD_SETTING_UP ||
-				td->runstate >= TD_FINISHING ||
-				!td->o.serialize_overlap ||
-				td->o.io_submit_mode != IO_MODE_OFFLOAD)
-				continue;
-
-			overlap = in_flight_overlap(&td->io_u_all, io_u);
-			if (overlap) {
-				pthread_mutex_unlock(&overlap_check);
-				break;
-			}
-		}
-	} while (overlap);
+		goto retry;
+	}
 }
 
 static int io_workqueue_fn(struct submit_worker *sw,