[PATCH 0/1] number_ios and io_size behave differently for trimwrite

[PATCH 0/1] number_ios and io_size behave differently for trimwrite

[Ankit Kumar]

Add changes for number_ios such that we actually do trim and then writes
for those number of I/O's. This will make the behaviour similar to
io_size.
Update the documentation to clarify the exact behaviour for trimwrite
workloads.

Ankit Kumar (1):
  backend: number of ios not as expected for trimwrite

 HOWTO.rst | 6 +++++-
 backend.c | 6 ++++--
 fio.1     | 5 ++++-
 3 files changed, 13 insertions(+), 4 deletions(-)

-- 
2.17.1

[PATCH 1/1] backend: number of ios not as expected for trimwrite

[Ankit Kumar]

number_ios should be twice for trimwrite, just like size or
io_size. Update the documentation for "rw=trimwrite" to explain the
changes.

Signed-off-by: Ankit Kumar <ankit.kumar@samsung.com>
---
 HOWTO.rst | 6 +++++-
 backend.c | 6 ++++--
 fio.1     | 5 ++++-
 3 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/HOWTO.rst b/HOWTO.rst
index 2c6c6dbe..924f5ed9 100644
--- a/HOWTO.rst
+++ b/HOWTO.rst
@@ -1129,7 +1129,11 @@ I/O type
 				Random mixed reads and writes.
 		**trimwrite**
 				Sequential trim+write sequences. Blocks will be trimmed first,
-				then the same blocks will be written to.
+				then the same blocks will be written to. So if ``io_size=64K``
+				is specified, Fio will trim a total of 64K bytes and also
+				write 64K bytes on the same trimmed blocks. This behaviour
+				will be consistent with ``number_ios`` or other Fio options
+				limiting the total bytes or number of I/O's.
 
 	Fio defaults to read if the option is not specified.  For the mixed I/O
 	types, the default is to split them 50/50.  For certain types of I/O the
diff --git a/backend.c b/backend.c
index fe614f6e..ec535bcc 100644
--- a/backend.c
+++ b/backend.c
@@ -971,9 +971,11 @@ static void do_io(struct thread_data *td, uint64_t *bytes_done)
 		total_bytes += td->o.size;
 
 	/* In trimwrite mode, each byte is trimmed and then written, so
-	 * allow total_bytes to be twice as big */
-	if (td_trimwrite(td))
+	 * allow total_bytes or number of ios to be twice as big */
+	if (td_trimwrite(td)) {
 		total_bytes += td->total_io_size;
+		td->o.number_ios *= 2;
+	}
 
 	while ((td->o.read_iolog_file && !flist_empty(&td->io_log_list)) ||
 		(!flist_empty(&td->trim_list)) || !io_issue_bytes_exceeded(td) ||
diff --git a/fio.1 b/fio.1
index 67d7c710..c67bd464 100644
--- a/fio.1
+++ b/fio.1
@@ -900,7 +900,10 @@ Random mixed reads and writes.
 .TP
 .B trimwrite
 Sequential trim+write sequences. Blocks will be trimmed first,
-then the same blocks will be written to.
+then the same blocks will be written to. So if `io_size=64K' is specified,
+Fio will trim a total of 64K bytes and also write 64K bytes on the same
+trimmed blocks. This behaviour will be consistent with `number_ios' or
+other Fio options limiting the total bytes or number of I/O's.
 .RE
 .P
 Fio defaults to read if the option is not specified. For the mixed I/O
-- 
2.17.1

Re: [PATCH 1/1] backend: number of ios not as expected for trimwrite

[Vincent Fu]

On 9/13/22 06:45, Ankit Kumar wrote:
> number_ios should be twice for trimwrite, just like size or
> io_size. Update the documentation for "rw=trimwrite" to explain the
> changes.
> 
> Signed-off-by: Ankit Kumar <ankit.kumar@samsung.com>
> ---
>   HOWTO.rst | 6 +++++-
>   backend.c | 6 ++++--
>   fio.1     | 5 ++++-
>   3 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/HOWTO.rst b/HOWTO.rst
> index 2c6c6dbe..924f5ed9 100644
> --- a/HOWTO.rst
> +++ b/HOWTO.rst
> @@ -1129,7 +1129,11 @@ I/O type
>   				Random mixed reads and writes.
>   		**trimwrite**
>   				Sequential trim+write sequences. Blocks will be trimmed first,
> -				then the same blocks will be written to.
> +				then the same blocks will be written to. So if ``io_size=64K``
> +				is specified, Fio will trim a total of 64K bytes and also
> +				write 64K bytes on the same trimmed blocks. This behaviour
> +				will be consistent with ``number_ios`` or other Fio options
> +				limiting the total bytes or number of I/O's.
>   
>   	Fio defaults to read if the option is not specified.  For the mixed I/O
>   	types, the default is to split them 50/50.  For certain types of I/O the
> diff --git a/backend.c b/backend.c
> index fe614f6e..ec535bcc 100644
> --- a/backend.c
> +++ b/backend.c
> @@ -971,9 +971,11 @@ static void do_io(struct thread_data *td, uint64_t *bytes_done)
>   		total_bytes += td->o.size;
>   
>   	/* In trimwrite mode, each byte is trimmed and then written, so
> -	 * allow total_bytes to be twice as big */
> -	if (td_trimwrite(td))
> +	 * allow total_bytes or number of ios to be twice as big */
> +	if (td_trimwrite(td)) {
>   		total_bytes += td->total_io_size;
> +		td->o.number_ios *= 2;
> +	}
>   
>   	while ((td->o.read_iolog_file && !flist_empty(&td->io_log_list)) ||
>   		(!flist_empty(&td->trim_list)) || !io_issue_bytes_exceeded(td) ||
> diff --git a/fio.1 b/fio.1
> index 67d7c710..c67bd464 100644
> --- a/fio.1
> +++ b/fio.1
> @@ -900,7 +900,10 @@ Random mixed reads and writes.
>   .TP
>   .B trimwrite
>   Sequential trim+write sequences. Blocks will be trimmed first,
> -then the same blocks will be written to.
> +then the same blocks will be written to. So if `io_size=64K' is specified,
> +Fio will trim a total of 64K bytes and also write 64K bytes on the same
> +trimmed blocks. This behaviour will be consistent with `number_ios' or
> +other Fio options limiting the total bytes or number of I/O's.
>   .RE
>   .P
>   Fio defaults to read if the option is not specified. For the mixed I/O

This is the right behavior because io_size and number_ios should be 
consistent. But this will require folks who have worked around the 
inconsistent behavior to adjust. I'll let Jens make the final call on 
this one.

Reviewed-by: Vincent Fu <vincent.fu@samsung.com>

Re: [PATCH 1/1] backend: number of ios not as expected for trimwrite

[Jens Axboe]

On 9/13/22 10:07 AM, Vincent Fu wrote:
> On 9/13/22 06:45, Ankit Kumar wrote:
>> number_ios should be twice for trimwrite, just like size or
>> io_size. Update the documentation for "rw=trimwrite" to explain the
>> changes.
>>
>> Signed-off-by: Ankit Kumar <ankit.kumar@samsung.com>
>> ---
>>   HOWTO.rst | 6 +++++-
>>   backend.c | 6 ++++--
>>   fio.1     | 5 ++++-
>>   3 files changed, 13 insertions(+), 4 deletions(-)
>>
>> diff --git a/HOWTO.rst b/HOWTO.rst
>> index 2c6c6dbe..924f5ed9 100644
>> --- a/HOWTO.rst
>> +++ b/HOWTO.rst
>> @@ -1129,7 +1129,11 @@ I/O type
>>                   Random mixed reads and writes.
>>           **trimwrite**
>>                   Sequential trim+write sequences. Blocks will be trimmed first,
>> -                then the same blocks will be written to.
>> +                then the same blocks will be written to. So if ``io_size=64K``
>> +                is specified, Fio will trim a total of 64K bytes and also
>> +                write 64K bytes on the same trimmed blocks. This behaviour
>> +                will be consistent with ``number_ios`` or other Fio options
>> +                limiting the total bytes or number of I/O's.
>>         Fio defaults to read if the option is not specified.  For the mixed I/O
>>       types, the default is to split them 50/50.  For certain types of I/O the
>> diff --git a/backend.c b/backend.c
>> index fe614f6e..ec535bcc 100644
>> --- a/backend.c
>> +++ b/backend.c
>> @@ -971,9 +971,11 @@ static void do_io(struct thread_data *td, uint64_t *bytes_done)
>>           total_bytes += td->o.size;
>>         /* In trimwrite mode, each byte is trimmed and then written, so
>> -     * allow total_bytes to be twice as big */
>> -    if (td_trimwrite(td))
>> +     * allow total_bytes or number of ios to be twice as big */
>> +    if (td_trimwrite(td)) {
>>           total_bytes += td->total_io_size;
>> +        td->o.number_ios *= 2;
>> +    }
>>         while ((td->o.read_iolog_file && !flist_empty(&td->io_log_list)) ||
>>           (!flist_empty(&td->trim_list)) || !io_issue_bytes_exceeded(td) ||
>> diff --git a/fio.1 b/fio.1
>> index 67d7c710..c67bd464 100644
>> --- a/fio.1
>> +++ b/fio.1
>> @@ -900,7 +900,10 @@ Random mixed reads and writes.
>>   .TP
>>   .B trimwrite
>>   Sequential trim+write sequences. Blocks will be trimmed first,
>> -then the same blocks will be written to.
>> +then the same blocks will be written to. So if `io_size=64K' is specified,
>> +Fio will trim a total of 64K bytes and also write 64K bytes on the same
>> +trimmed blocks. This behaviour will be consistent with `number_ios' or
>> +other Fio options limiting the total bytes or number of I/O's.
>>   .RE
>>   .P
>>   Fio defaults to read if the option is not specified. For the mixed I/O
> 
> This is the right behavior because io_size and number_ios should be consistent. But this will require folks who have worked around the inconsistent behavior to adjust. I'll let Jens make the final call on this one.
> 
> Reviewed-by: Vincent Fu <vincent.fu@samsung.com>

For cases like this, I do still prefer to fix it up so that it's consistent.

-- 
Jens Axboe

Re: [PATCH 0/1] number_ios and io_size behave differently for trimwrite

[Jens Axboe]

On Tue, 13 Sep 2022 16:15:26 +0530, Ankit Kumar wrote:
> Add changes for number_ios such that we actually do trim and then writes
> for those number of I/O's. This will make the behaviour similar to
> io_size.
> Update the documentation to clarify the exact behaviour for trimwrite
> workloads.
> 
> Ankit Kumar (1):
>   backend: number of ios not as expected for trimwrite
> 
> [...]

Applied, thanks!

[1/1] backend: number of ios not as expected for trimwrite
      commit: 08996af41b2566565cbcdee71766030a2c8ba377

Best regards,
-- 
Jens Axboe