[PATCH v2 0/2] Fix thinktime handling and minimal rate calculation

[PATCH v2 0/2] Fix thinktime handling and minimal rate calculation

[HongweiQin]

This patch series fix the thinktime handling procedure and take the thinktime
into consideration when checking the minimal rate.

HongweiQin (2):
  Fix thinktime_blocks
  Fix handle_thinktime

 backend.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

-- 
1.8.3.1

[PATCH v2 1/2] Fix thinktime_blocks

[HongweiQin]

According to the manual, the thinktime_blocks controls how many blocks to issue (instead of complete) before waiting.

Signed-off-by: HongweiQin <glqinhongwei@gmail.com>
---
 backend.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/backend.c b/backend.c
index e20a2e0..c85669c 100644
--- a/backend.c
+++ b/backend.c
@@ -864,7 +864,7 @@ static void handle_thinktime(struct thread_data *td, enum fio_ddir ddir)
 	uint64_t total;
 	int left;
 
-	b = ddir_rw_sum(td->io_blocks);
+	b = ddir_rw_sum(td->io_issues);
 	if (b % td->o.thinktime_blocks)
 		return;
 
-- 
1.8.3.1

Re: [PATCH v2 1/2] Fix thinktime_blocks

[Hongwei Qin]

Hi Sitsofe,

>
> At this point wouldn't it be better to update the help to reflect the
> code rather than the other way around?
>

Consider this configuration:
[global]
thread
kb_base=1000
direct=1
size=28GiB
group_reporting
io_size=64KiB
ioengine=libaio
iodepth=32
bs=4096
filename=/dev/qblkdev

[fio_randwrite]
rw=randwrite
thinktime_blocks=4
thinktime=1s

I write this job file to simulate a data server that periodically
writes back a certain amount of data (In this case,16KiB per second).
Note that we cannot use the rate_iops here because it will send 1 request
per 250ms.
If we keep the code as it is, the job will finish immediately after
the test starts.
And this is usually not what the user wants.

Or, should we introduce a new parameter to distinguish between
"think after finish" and "think after issue"?

Regards,
Hongwei

On Sun, Jan 17, 2021 at 5:54 PM HongweiQin <glqinhongwei@gmail.com> wrote:
>
> According to the manual, the thinktime_blocks controls how many blocks to issue (instead of complete) before waiting.
>
> Signed-off-by: HongweiQin <glqinhongwei@gmail.com>
> ---
>  backend.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/backend.c b/backend.c
> index e20a2e0..c85669c 100644
> --- a/backend.c
> +++ b/backend.c
> @@ -864,7 +864,7 @@ static void handle_thinktime(struct thread_data *td, enum fio_ddir ddir)
>         uint64_t total;
>         int left;
>
> -       b = ddir_rw_sum(td->io_blocks);
> +       b = ddir_rw_sum(td->io_issues);
>         if (b % td->o.thinktime_blocks)
>                 return;
>
> --
> 1.8.3.1
>

Re: [PATCH v2 1/2] Fix thinktime_blocks

[Sitsofe Wheeler]

Hi Hongwei,

On Sun, 17 Jan 2021 at 12:48, Hongwei Qin <glqinhongwei@gmail.com> wrote:
>
> Hi Sitsofe,
>
> >
> > At this point wouldn't it be better to update the help to reflect the
> > code rather than the other way around?
> >
>
> Consider this configuration:
> [global]
> thread
> kb_base=1000
> direct=1
> size=28GiB
> group_reporting
> io_size=64KiB
> ioengine=libaio
> iodepth=32
> bs=4096
> filename=/dev/qblkdev
>
> [fio_randwrite]
> rw=randwrite
> thinktime_blocks=4
> thinktime=1s
>
> I write this job file to simulate a data server that periodically
> writes back a certain amount of data (In this case,16KiB per second).
> Note that we cannot use the rate_iops here because it will send 1 request
> per 250ms.
> If we keep the code as it is, the job will finish immediately after
> the test starts.
> And this is usually not what the user wants.
>
> Or, should we introduce a new parameter to distinguish between
> "think after finish" and "think after issue"?

I like the idea of a new parameter! Maybe it would act as a toggle
because it doesn't make sense to set both together...

-- 
Sitsofe

[PATCH v2 2/2] Fix handle_thinktime

[HongweiQin]

This patch handles thinktime before checking td->bytes_done.
This avoids skipping the thinktime handling when the thread just starts.

Besides, the handle_thinktime also updates the comp_time so that
the rate calculation of check_min_rate() is based on the time after
the thinktime.

Signed-off-by: HongweiQin <glqinhongwei@gmail.com>
---
 backend.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/backend.c b/backend.c
index c85669c..b3e13cb 100644
--- a/backend.c
+++ b/backend.c
@@ -858,7 +858,8 @@ static long long usec_for_io(struct thread_data *td, enum fio_ddir ddir)
 	return 0;
 }
 
-static void handle_thinktime(struct thread_data *td, enum fio_ddir ddir)
+static void handle_thinktime(struct thread_data *td, enum fio_ddir ddir,
+			     struct timespec *time)
 {
 	unsigned long long b;
 	uint64_t total;
@@ -898,6 +899,9 @@ static void handle_thinktime(struct thread_data *td, enum fio_ddir ddir)
 		/* adjust for rate_process=poisson */
 		td->last_usec[ddir] += total;
 	}
+
+	if (time && should_check_rate(td))
+		fio_gettime(time, NULL);
 }
 
 /*
@@ -1076,6 +1080,10 @@ reap:
 		}
 		if (ret < 0)
 			break;
+
+		if (ddir_rw(ddir) && td->o.thinktime)
+			handle_thinktime(td, ddir, &comp_time);
+
 		if (!ddir_rw_sum(td->bytes_done) &&
 		    !td_ioengine_flagged(td, FIO_NOIO))
 			continue;
@@ -1090,9 +1098,6 @@ reap:
 		}
 		if (!in_ramp_time(td) && td->o.latency_target)
 			lat_target_check(td);
-
-		if (ddir_rw(ddir) && td->o.thinktime)
-			handle_thinktime(td, ddir);
 	}
 
 	check_update_rusage(td);
-- 
1.8.3.1