[PATCHv2 2/4] backend: don't mark a job as running before it has set up

[Keith Busch <kbusch@meta.com>]

From: Keith Busch <kbusch@kernel.org>

run_threads() moves a job from TD_INITIALIZED straight to TD_RUNNING and
only then releases it. The job still has all of its setup left to do:
exec_prerun, pre_read_files(), setup_files(), init_io_u(),
rate_submit_init() and finally set_epoch_time(). The job claims to be
running for that entire window when it has not issued any IO and has no
epoch to measure itself against.

Anything that reads the state during that window gets a wrong answer.
thread_eta() is the most readily observable  one: it derives elapsed
from td->epoch, which is still zeroed, so a job with a three second
exec_prerun reports

  Jobs: 1 (f=0): [R(1)][-.-%][eta 00m:00s]
  Jobs: 1 (f=0): [R(1)][50.0%][eta 00m:03s]

before doing any work at all. Running, half done and no time left, none
of which is true.

Promote to TD_SETTING_UP instead, which is what that state is for, and
let the job promote itself to TD_RAMP or TD_RUNNING once setup is over
and it has recorded its epoch. The same job now reports

  Jobs: 1 (f=1): [I(1)][0.0%][eta 00m:03s]

Widen the TERMINATE_STONEWALL check to match. It tests for runstate >=
TD_RUNNING to find jobs worth terminating, and a job in the setup window
used to satisfy that. Without this, exit_what=stonewall stops reaching a
job that is still setting up, and a test where the short job is reaped
while a longer one sits in exec_prerun goes from 5.5s to 20.5s. Note
TD_RAMP sorts below TD_SETTING_UP, so ramping jobs remain excluded from
that check exactly as before.

Signed-off-by: Keith Busch <kbusch@kernel.org>
---
 backend.c | 21 ++++++++++++++++-----
 libfio.c  |  3 ++-
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/backend.c b/backend.c
index 7f41bdfa..46bff828 100644
--- a/backend.c
+++ b/backend.c
@@ -2178,6 +2178,17 @@ static void *thread_main(void *data)
 		goto err;
 
 	set_epoch_time(td, o->log_alternate_epoch_clock_id, o->job_start_clock_id);
+
+	/*
+	 * Setup is done and the job now has an epoch to measure itself
+	 * against, so it is finally safe to call it running. Everything
+	 * above this point ran as TD_SETTING_UP.
+	 */
+	if (in_ramp_period(td))
+		td_set_runstate(td, TD_RAMP);
+	else
+		td_set_runstate(td, TD_RUNNING);
+
 	fio_getrusage(&td->ru_start);
 	memcpy(&td->bw_sample_time, &td->epoch, sizeof(td->epoch));
 	memcpy(&td->iops_sample_time, &td->epoch, sizeof(td->epoch));
@@ -2888,16 +2899,16 @@ reap:
 		}
 
 		/*
-		 * start created threads (TD_INITIALIZED -> TD_RUNNING).
+		 * start created threads (TD_INITIALIZED -> TD_SETTING_UP).
+		 * The job has plenty of setup left to do before it issues any
+		 * IO, so it promotes itself to TD_RAMP or TD_RUNNING once that
+		 * is done and it has recorded its epoch.
 		 */
 		for_each_td(td) {
 			if (td->runstate != TD_INITIALIZED)
 				continue;
 
-			if (in_ramp_period(td))
-				td_set_runstate(td, TD_RAMP);
-			else
-				td_set_runstate(td, TD_RUNNING);
+			td_set_runstate(td, TD_SETTING_UP);
 			nr_running++;
 			nr_started--;
 			m_rate += ddir_rw_sum(td->o.ratemin);
diff --git a/libfio.c b/libfio.c
index a57ede4f..322906c0 100644
--- a/libfio.c
+++ b/libfio.c
@@ -270,7 +270,8 @@ void fio_terminate_threads(unsigned int group_id, unsigned int terminate)
 	for_each_td(td) {
 		if ((terminate == TERMINATE_GROUP && group_id == TERMINATE_ALL) ||
 		    (terminate == TERMINATE_GROUP && group_id == td->groupid) ||
-		    (terminate == TERMINATE_STONEWALL && td->runstate >= TD_RUNNING) ||
+		    (terminate == TERMINATE_STONEWALL &&
+		     td->runstate >= TD_SETTING_UP) ||
 		    (terminate == TERMINATE_ALL)) {
 			dprint(FD_PROCESS, "setting terminate on %s/%d\n",
 						td->o.name, (int) td->pid);
-- 
2.53.0-Meta