[PATCHv2 0/4] fio: fix eta

[PATCHv2 0/4] fio: fix eta

[Keith Busch]

From: Keith Busch <kbusch@kernel.org>

Changes:

Fix up the state machine rather than hack around it.

v1: https://lore.kernel.org/fio/20260903172101.1886315-1-kbusch@meta.com/

Keith Busch (4):
  eta: count a job that is setting up as running
  backend: don't mark a job as running before it has set up
  eta: cap the ETA at the job's own remaining runtime
  eta: remove now unused done_secs

 backend.c | 23 ++++++++++++++++-------
 eta.c     | 20 +++++++++++++++-----
 fio.h     |  1 -
 libfio.c  |  4 ++--
 4 files changed, 33 insertions(+), 15 deletions(-)

-- 
2.53.0-Meta

[PATCHv2 1/4] eta: count a job that is setting up as running

[Keith Busch]

From: Keith Busch <kbusch@kernel.org>

TD_RAMP increments both nr_running and nr_ramp, so nr_ramp is a subset
of nr_running and display_thread_status() can compare the two. The
TD_SETTING_UP side would increment only nr_setting_up, yet the
percentage calculation scales by nr_setting_up / nr_running, a ratio
that only means anything if setting up jobs are counted in nr_running as
well: the existing ratio can drive the multiplier negative.

A job that is setting up is started and doing work, it just has not
issued IO yet, so count it in nr_running and treat nr_setting_up as the
subset as intended. Today this only covers the brief windows where
setup_files() and pre_read_file() bump the state, but it means the
status line reports such a job instead of suppressing the whole line.

Signed-off-by: Keith Busch <kbusch@kernel.org>
---
 eta.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/eta.c b/eta.c
index c6e3cffb..70efc208 100644
--- a/eta.c
+++ b/eta.c
@@ -466,9 +466,10 @@ static bool calc_thread_status(struct jobs_eta *je, int force)
 		} else if (td->runstate == TD_RAMP) {
 			je->nr_running++;
 			je->nr_ramp++;
-		} else if (td->runstate == TD_SETTING_UP)
+		} else if (td->runstate == TD_SETTING_UP) {
+			je->nr_running++;
 			je->nr_setting_up++;
-		else if (td->runstate < TD_RUNNING)
+		} else if (td->runstate < TD_RUNNING)
 			je->nr_pending++;
 
 		if (je->elapsed_sec >= 3)
-- 
2.53.0-Meta

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

[Keith Busch]

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

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

[Vincent Fu]

On Thu, Sep 10, 2026 at 12:13 PM Keith Busch <kbusch@meta.com> wrote:
>
<snip>
> 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.
>

I wonder if we should actually swap the order of TD_RAMP and TD_SETTING up in
the state machine. A job that is in its ramp time is part of the currently
running stonewall group, so it should be terminated when exitall=stonewall.

Vincent

[PATCHv2 3/4] eta: cap the ETA at the job's own remaining runtime

[Keith Busch]

From: Keith Busch <kbusch@kernel.org>

The ETA of a running job is capped at "timeout + done_secs - elapsed".
done_secs is a global accumulator of the runtime of every job reaped so
far, so the cap grows every time a job finishes. Whenever the cap is
what actually gets reported, the ETA jumps back up by the runtime of
everything that has already completed.

The cap is what gets reported when the progress based estimate exceeds
the remaining runtime, that is when perc is below elapsed/timeout. Any
job that is behind on bytes relative to its runtime is in that state,
so this covers the common "size the job to the whole device, bound it
with runtime" pattern. A job that would complete its size early stays
at or above elapsed/timeout and never reaches the cap.

time_based makes no difference either way. It only lowers perc to
min(perc, elapsed/timeout), so a time_based job that cannot finish its
size within the runtime is affected exactly like a size based one.

It is most visible with stonewalled jobs, where the ETA climbs back to
the full run time at every batch boundary instead of counting down:

  fio --name=global --filename=/dev/zero --runtime=5 --size=10T \
      --stonewall --name=a --name=b --name=c --name=d --name=e

  before: 22 21 20  24 23 22 21 20  24 23 22 21 20  24 ...
  after:  22 21 20 19 18 17 16 15 14 13 ... 02 01 00

Shrinking size until the jobs complete it within the runtime makes the
symptom disappear, which is a good way to confirm the cap is what is
being reported.

It is not specific to stonewall. Two concurrent jobs with runtime=5 and
runtime=20 show the same jump when the short one is reaped at t=5.

done_secs made sense when it was introduced: thread_eta() was handed
the global elapsed time back then, so "timeout + done_secs" was this
job's projected finish time relative to the start of the whole run.
b29ee5b3 switched elapsed to be per job, measured from td->epoch, but
left the done_secs term behind.

A job is terminated once utime_since(&td->epoch, now) reaches
td->o.timeout, so with a per job elapsed the cap is simply
"timeout - elapsed". Use that, and clamp at zero instead of relying on
the unsigned subtraction wrapping.

Fixes: b29ee5b3dee4 ("Update ramp_time")
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
 eta.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/eta.c b/eta.c
index 70efc208..d17b8c3f 100644
--- a/eta.c
+++ b/eta.c
@@ -253,9 +253,18 @@ static unsigned long thread_eta(struct thread_data *td)
 			eta_sec = (unsigned long) (elapsed * (1.0 / perc)) - elapsed;
 		}
 
-		if (td->o.timeout &&
-		    eta_sec > (timeout + done_secs - elapsed))
-			eta_sec = timeout + done_secs - elapsed;
+		/*
+		 * A job never runs for longer than its own timeout, which is
+		 * measured from its own epoch. Cap the estimate at whatever
+		 * time this job has left.
+		 */
+		if (td->o.timeout) {
+			unsigned long timeout_left;
+
+			timeout_left = timeout > elapsed ? timeout - elapsed : 0;
+			if (eta_sec > timeout_left)
+				eta_sec = timeout_left;
+		}
 	} else if (td->runstate == TD_NOT_CREATED || td->runstate == TD_CREATED
 			|| td->runstate == TD_INITIALIZED
 			|| td->runstate == TD_SETTING_UP
-- 
2.53.0-Meta

[PATCHv2 4/4] eta: remove now unused done_secs

[Keith Busch]

From: Keith Busch <kbusch@kernel.org>

Capping the ETA at the job's own remaining runtime removed the last
reader of done_secs. Drop the variable, along with the gettime call
that maintained it on every reap.

Signed-off-by: Keith Busch <kbusch@kernel.org>
---
 backend.c | 2 --
 fio.h     | 1 -
 libfio.c  | 1 -
 3 files changed, 4 deletions(-)

diff --git a/backend.c b/backend.c
index 46bff828..131cda01 100644
--- a/backend.c
+++ b/backend.c
@@ -71,7 +71,6 @@ unsigned int nr_segments = 0;
 unsigned int cur_segment = 0;
 unsigned int stat_number = 0;
 int temp_stall_ts;
-unsigned long done_secs = 0;
 #ifdef PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP
 pthread_mutex_t overlap_check = PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP;
 #else
@@ -2505,7 +2504,6 @@ reaped:
 		if (td->error)
 			exit_value++;
 
-		done_secs += mtime_since_now(&td->epoch) / 1000;
 		profile_td_exit(td);
 		flow_exit_job(td);
 	} end_for_each();
diff --git a/fio.h b/fio.h
index 494959a6..11a0d59d 100644
--- a/fio.h
+++ b/fio.h
@@ -597,7 +597,6 @@ extern bool read_only;
 extern int eta_print;
 extern int eta_new_line;
 extern unsigned int eta_interval_msec;
-extern unsigned long done_secs;
 extern int fio_gtod_offload;
 extern int fio_gtod_cpu;
 extern enum fio_cs fio_clock_source;
diff --git a/libfio.c b/libfio.c
index 322906c0..5fc48a1d 100644
--- a/libfio.c
+++ b/libfio.c
@@ -187,7 +187,6 @@ void reset_fio_state(void)
 	for (i = 0; i < nr_segments; i++)
 		segments[i].nr_threads = 0;
 	stat_number = 0;
-	done_secs = 0;
 }
 
 const char *fio_get_os_string(int nr)
-- 
2.53.0-Meta

Re: [PATCHv2 0/4] fio: fix eta

[Vincent Fu]

On Thu, Sep 10, 2026 at 12:13 PM Keith Busch <kbusch@meta.com> wrote:
>
> From: Keith Busch <kbusch@kernel.org>
>
> Changes:
>
> Fix up the state machine rather than hack around it.
>
> v1: https://lore.kernel.org/fio/20260903172101.1886315-1-kbusch@meta.com/
>
> Keith Busch (4):
>   eta: count a job that is setting up as running
>   backend: don't mark a job as running before it has set up
>   eta: cap the ETA at the job's own remaining runtime
>   eta: remove now unused done_secs
>
>  backend.c | 23 ++++++++++++++++-------
>  eta.c     | 20 +++++++++++++++-----
>  fio.h     |  1 -
>  libfio.c  |  4 ++--
>  4 files changed, 33 insertions(+), 15 deletions(-)
>
> --
> 2.53.0-Meta
>

Applied. Thanks.

Vincent

Re: [PATCHv2 0/4] fio: fix eta

[fiotestbot]

[-- Attachment #1: Type: text/plain, Size: 144 bytes --]


The result of fio's continuous integration tests was: failure

For more details see https://github.com/fiotestbot/fio/actions/runs/34637696308