Fix ETA calculations
authorJens Rosenboom <j.rosenboom@x-ion.de>
Thu, 19 Nov 2015 20:36:18 +0000 (13:36 -0700)
committerJens Axboe <axboe@fb.com>
Thu, 19 Nov 2015 20:36:18 +0000 (13:36 -0700)
There are a couple of issues in eta.c:

- The comparison for time- vs. rate-based ETA calculation is done the
  wrong way round.
- The elapsed ramp time should only be subtracted when the thread has
  indeed entered the ramp phase.
- Currently jobs that are stonewalled are not taken into account for ETA
  calculation. This patch at least fixes this for the case that there is
  only a single job per stonewall group.
---

This patch tries to fix some issues I have found, this may
also be related to the ones mentioned earlier in
http://www.spinics.net/lists/fio/msg04187.html

The part regarding stonewalled jobs may require some further work, but
at least it gives proper values now for my standard fio runs containing
just a sequence of singular jobs.

Also, I'm still wondering about the intention behind the way done_secs
is meant to work. It cumulates time spent in finished jobs globally, but
is being used in per-job ETA calculations. This seems to break at
least for the case of stonewalled jobs, too.

Signed-off-by: Jens Axboe <axboe@fb.com>
eta.c

diff --git a/eta.c b/eta.c
index 7eb1c27f6a3c57b46f371b422fd800e480fb15ad..8785540aa6f532f7e718810fa89b041a993d3e9f 100644 (file)
--- a/eta.c
+++ b/eta.c
@@ -214,7 +214,7 @@ static unsigned long thread_eta(struct thread_data *td)
                if (td->o.time_based) {
                        if (timeout) {
                                perc_t = (double) elapsed / (double) timeout;
-                               if (perc_t < perc)
+                               if (perc_t > perc)
                                        perc = perc_t;
                        } else {
                                /*
@@ -250,7 +250,7 @@ static unsigned long thread_eta(struct thread_data *td)
                        t_eta = __timeout + start_delay + ramp_time;
                        t_eta /= 1000000ULL;
 
-                       if (in_ramp_time(td)) {
+                       if ((td->runstate == TD_RAMP) && in_ramp_time(td)) {
                                unsigned long ramp_left;
 
                                ramp_left = mtime_since_now(&td->epoch);
@@ -438,19 +438,25 @@ int calc_thread_status(struct jobs_eta *je, int force)
                }
        }
 
-       if (exitall_on_terminate)
+       if (exitall_on_terminate) {
                je->eta_sec = INT_MAX;
-       else
-               je->eta_sec = 0;
-
-       for_each_td(td, i) {
-               if (exitall_on_terminate) {
+               for_each_td(td, i) {
                        if (eta_secs[i] < je->eta_sec)
                                je->eta_sec = eta_secs[i];
-               } else {
-                       if (eta_secs[i] > je->eta_sec)
-                               je->eta_sec = eta_secs[i];
                }
+       } else {
+               unsigned long eta_stone = 0;
+
+               je->eta_sec = 0;
+               for_each_td(td, i) {
+                       if ((td->runstate == TD_NOT_CREATED) && td->o.stonewall)
+                               eta_stone += eta_secs[i];
+                       else {
+                               if (eta_secs[i] > je->eta_sec)
+                                       je->eta_sec = eta_secs[i];
+                       }
+               }
+               je->eta_sec += eta_stone;
        }
 
        free(eta_secs);