number_ios is checked on completion, not submission

number_ios is checked on completion, not submission

[Elliott, Robert (Server Storage)]

Since number_ios is checked in io_u.c account_io_completion() rather than a submission function, fio actually runs the requested number of I/Os plus iodepth - 1.

Example: for a job specifying:
number_ios=5000
iodepth=128

the results are:
  read : io=20000KB, bw=222222KB/s, iops=56966, runt=    90msec
  IO depths    : 1=0.1%, 2=0.1%, 4=0.1%, 8=0.2%, 16=0.3%, 32=0.6%, >=64=98.8%
     issued    : total=r=5127/w=0/d=0, short=r=0/w=0/d=0

Should that just be documented as such, or should this logic be moved to submission?
static void account_io_completion(struct thread_data *td, struct io_u *io_u,
...
        if (td->o.number_ios && !--td->o.number_ios)
                td->done = 1;

---
Rob Elliott    HP Server Storage

Re: number_ios is checked on completion, not submission

[Jens Axboe]

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

On 03/10/2014 03:12 PM, Elliott, Robert (Server Storage) wrote:
> Since number_ios is checked in io_u.c account_io_completion() rather than a submission function, fio actually runs the requested number of I/Os plus iodepth - 1.
>
> Example: for a job specifying:
> number_ios=5000
> iodepth=128
>
> the results are:
>    read : io=20000KB, bw=222222KB/s, iops=56966, runt=    90msec
>    IO depths    : 1=0.1%, 2=0.1%, 4=0.1%, 8=0.2%, 16=0.3%, 32=0.6%, >=64=98.8%
>       issued    : total=r=5127/w=0/d=0, short=r=0/w=0/d=0
>
> Should that just be documented as such, or should this logic be moved to submission?
> static void account_io_completion(struct thread_data *td, struct io_u *io_u,
> ...
>          if (td->o.number_ios && !--td->o.number_ios)
>                  td->done = 1;

Can you try the attached patch and see if that makes it behave more like 
expected?

-- 
Jens Axboe


[-- Attachment #2: number_ios.patch --]
[-- Type: text/x-patch, Size: 1420 bytes --]

diff --git a/backend.c b/backend.c
index 992033c5c170..cee185571082 100644
--- a/backend.c
+++ b/backend.c
@@ -625,6 +625,7 @@ reap:
 
 static int io_bytes_exceeded(struct thread_data *td)
 {
+	unsigned long long number_ios = 0;
 	unsigned long long bytes;
 
 	if (td_rw(td))
@@ -636,7 +637,13 @@ static int io_bytes_exceeded(struct thread_data *td)
 	else
 		bytes = td->this_io_bytes[DDIR_TRIM];
 
-	return bytes >= td->o.size;
+	if (td->o.number_ios) {
+		number_ios = ddir_rw_sum(td->this_io_blocks);
+		number_ios += td->io_u_queued;
+	}
+
+	return bytes >= td->o.size ||
+		(number_ios && number_ios >= td->o.number_ios);
 }
 
 /*
@@ -1128,6 +1135,14 @@ static int keep_running(struct thread_data *td)
 		return 1;
 	}
 
+	if (td->o.number_ios) {
+		unsigned long long number_ios = ddir_rw_sum(td->this_io_blocks);
+
+		number_ios += td->io_u_queued;
+		if (number_ios >= td->o.number_ios)
+			return 0;
+	}
+
 	if (td->o.size != -1ULL && ddir_rw_sum(td->io_bytes) < td->o.size) {
 		uint64_t diff;
 
diff --git a/io_u.c b/io_u.c
index 8e27708731c7..0b86d9f3c281 100644
--- a/io_u.c
+++ b/io_u.c
@@ -1595,9 +1595,6 @@ static void account_io_completion(struct thread_data *td, struct io_u *io_u,
 
 	if (!gtod_reduce(td))
 		add_iops_sample(td, idx, bytes, &icd->time);
-
-	if (td->o.number_ios && !--td->o.number_ios)
-		td->done = 1;
 }
 
 static long long usec_for_io(struct thread_data *td, enum fio_ddir ddir)

RE: number_ios is checked on completion, not submission

[Elliott, Robert (Server Storage)]

That patch still exits after 5127 ios.

io_bytes_exceeded is called 5128 times, but it sees td->io_u_queued=0 each time.

keep_running makes it to the if (td->o.number_ios) branch one time, and sees number_ios set to 5127  and td->o.number_ios set to 5000 (causing it to return 0 and trigger the end, after 5127 ios).

> -----Original Message-----
> From: Jens Axboe [mailto:axboe@kernel.dk]
> Sent: Tuesday, 11 March, 2014 10:36 AM
> To: Elliott, Robert (Server Storage); fio@vger.kernel.org
> Subject: Re: number_ios is checked on completion, not submission
> 
> On 03/10/2014 03:12 PM, Elliott, Robert (Server Storage) wrote:
> > Since number_ios is checked in io_u.c account_io_completion() rather than
> a submission function, fio actually runs the requested number of I/Os plus
> iodepth - 1.
> >
> > Example: for a job specifying:
> > number_ios=5000
> > iodepth=128
> >
> > the results are:
> >    read : io=20000KB, bw=222222KB/s, iops=56966, runt=    90msec
> >    IO depths    : 1=0.1%, 2=0.1%, 4=0.1%, 8=0.2%, 16=0.3%, 32=0.6%,
> >=64=98.8%
> >       issued    : total=r=5127/w=0/d=0, short=r=0/w=0/d=0
> >
> > Should that just be documented as such, or should this logic be moved to
> submission?
> > static void account_io_completion(struct thread_data *td, struct io_u
> *io_u,
> > ...
> >          if (td->o.number_ios && !--td->o.number_ios)
> >                  td->done = 1;
> 
> Can you try the attached patch and see if that makes it behave more like
> expected?
> 
> --
> Jens Axboe

Re: number_ios is checked on completion, not submission

[Jens Axboe]

On 03/11/2014 10:08 AM, Elliott, Robert (Server Storage) wrote:
> That patch still exits after 5127 ios.
>
> io_bytes_exceeded is called 5128 times, but it sees td->io_u_queued=0 each time.

Can you check if ->cur_depth or ->io_u_in_flight catches it?

> keep_running makes it to the if (td->o.number_ios) branch one time, and sees number_ios set to 5127  and td->o.number_ios set to 5000 (causing it to return 0 and trigger the end, after 5127 ios).

That's expected. That just controls the outer loop, we want to catch it 
in both places.

-- 
Jens Axboe

Re: number_ios is checked on completion, not submission

[Jens Axboe]

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

On 03/11/2014 10:14 AM, Jens Axboe wrote:
> On 03/11/2014 10:08 AM, Elliott, Robert (Server Storage) wrote:
>> That patch still exits after 5127 ios.
>>
>> io_bytes_exceeded is called 5128 times, but it sees td->io_u_queued=0
>> each time.
>
> Can you check if ->cur_depth or ->io_u_in_flight catches it?

It should, try this one.

-- 
Jens Axboe


[-- Attachment #2: number_ios-v2.patch --]
[-- Type: text/x-patch, Size: 1462 bytes --]

diff --git a/backend.c b/backend.c
index 992033c5c170..bab202662ca5 100644
--- a/backend.c
+++ b/backend.c
@@ -625,6 +625,7 @@ reap:
 
 static int io_bytes_exceeded(struct thread_data *td)
 {
+	unsigned long long number_ios = 0;
 	unsigned long long bytes;
 
 	if (td_rw(td))
@@ -636,7 +637,13 @@ static int io_bytes_exceeded(struct thread_data *td)
 	else
 		bytes = td->this_io_bytes[DDIR_TRIM];
 
-	return bytes >= td->o.size;
+	if (td->o.number_ios) {
+		number_ios = ddir_rw_sum(td->this_io_blocks);
+		number_ios += td->io_u_queued + td->io_u_in_flight;
+	}
+
+	return bytes >= td->o.size ||
+		(number_ios && number_ios >= td->o.number_ios);
 }
 
 /*
@@ -1128,6 +1135,14 @@ static int keep_running(struct thread_data *td)
 		return 1;
 	}
 
+	if (td->o.number_ios) {
+		unsigned long long number_ios = ddir_rw_sum(td->this_io_blocks);
+
+		number_ios += td->io_u_queued + td->io_u_in_flight;
+		if (number_ios >= td->o.number_ios)
+			return 0;
+	}
+
 	if (td->o.size != -1ULL && ddir_rw_sum(td->io_bytes) < td->o.size) {
 		uint64_t diff;
 
diff --git a/io_u.c b/io_u.c
index 8e27708731c7..0b86d9f3c281 100644
--- a/io_u.c
+++ b/io_u.c
@@ -1595,9 +1595,6 @@ static void account_io_completion(struct thread_data *td, struct io_u *io_u,
 
 	if (!gtod_reduce(td))
 		add_iops_sample(td, idx, bytes, &icd->time);
-
-	if (td->o.number_ios && !--td->o.number_ios)
-		td->done = 1;
 }
 
 static long long usec_for_io(struct thread_data *td, enum fio_ddir ddir)

RE: number_ios is checked on completion, not submission

[Elliott, Robert (Server Storage)]

That works.

  IO depths    : 1=0.1%, 2=0.1%, 4=0.1%, 8=0.2%, 16=0.3%, 32=0.6%, >=64=98.7%
     issued    : total=r=5000/w=0/d=0, short=r=0/w=0/d=0


> -----Original Message-----
> From: Jens Axboe [mailto:axboe@kernel.dk]
> Sent: Tuesday, 11 March, 2014 11:16 AM
> To: Elliott, Robert (Server Storage); fio@vger.kernel.org
> Subject: Re: number_ios is checked on completion, not submission
> 
> On 03/11/2014 10:14 AM, Jens Axboe wrote:
> > On 03/11/2014 10:08 AM, Elliott, Robert (Server Storage) wrote:
> >> That patch still exits after 5127 ios.
> >>
> >> io_bytes_exceeded is called 5128 times, but it sees td->io_u_queued=0
> >> each time.
> >
> > Can you check if ->cur_depth or ->io_u_in_flight catches it?
> 
> It should, try this one.
> 
> --
> Jens Axboe

Re: number_ios is checked on completion, not submission

[Jens Axboe]

On 03/11/2014 10:19 AM, Elliott, Robert (Server Storage) wrote:
> That works.
>
>    IO depths    : 1=0.1%, 2=0.1%, 4=0.1%, 8=0.2%, 16=0.3%, 32=0.6%, >=64=98.7%
>       issued    : total=r=5000/w=0/d=0, short=r=0/w=0/d=0

Goodie, committed.

-- 
Jens Axboe