verify_only broken? (was RE: )

verify_only broken? (was RE: )

[Justin Eno (jeno)]

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

On Wed, Jan 28, 2015 at 14:39:29, Justin Eno (jeno) <jeno@micron.com> wrote:
> Hi Jens,
> 
> It appears do_dry_run(), i.e., verify_only, is broken by the last 
> update to backend.c
> 
> I have attached a working patch, though you may prefer a different approach.
> 
> Thanks,
> Justin

I just realized that I sent this original message without a subject. Perhaps it was dropped for that reason.

I've re-attached the same patch. If it was NAKed, please just let me know and I will work on an alternative.

Also, do you prefer inline or attached patches?

Thanks,
Justin


[-- Attachment #2: verify_only.patch --]
[-- Type: application/octet-stream, Size: 2464 bytes --]

From dbdcd0d5d75c0bb2ceda264db41ce19fe1aadb7a Mon Sep 17 00:00:00 2001
From: Justin Eno <jeno@micron.com>
Date: Wed, 28 Jan 2015 14:13:28 -0800
Subject: [PATCH] Fix for verify_only (do_dry_run()) broken by 74d6277f

Previous commit to backend.c tracks io_limit more closely by
counting submitted (in-flight) I/O instead of completed I/O.
do_dry_run() does not submit I/O, so its I/O is not counted,
and it loops forever.
Signed-off-by: Justin Eno <jeno@micron.com>
---
 backend.c |   27 ++++++++++++++++++++++++---
 1 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/backend.c b/backend.c
index 9012140..7ec8d2a 100644
--- a/backend.c
+++ b/backend.c
@@ -662,7 +662,7 @@ static unsigned int exceeds_number_ios(struct thread_data *td)
 	return number_ios >= td->o.number_ios;
 }
 
-static int io_bytes_exceeded(struct thread_data *td)
+static int io_issue_bytes_exceeded(struct thread_data *td)
 {
 	unsigned long long bytes, limit;
 
@@ -683,6 +683,27 @@ static int io_bytes_exceeded(struct thread_data *td)
 	return bytes >= limit || exceeds_number_ios(td);
 }
 
+static int io_complete_bytes_exceeded(struct thread_data *td)
+{
+	unsigned long long bytes, limit;
+
+	if (td_rw(td))
+		bytes = td->this_io_bytes[DDIR_READ] + td->this_io_bytes[DDIR_WRITE];
+	else if (td_write(td))
+		bytes = td->this_io_bytes[DDIR_WRITE];
+	else if (td_read(td))
+		bytes = td->this_io_bytes[DDIR_READ];
+	else
+		bytes = td->this_io_bytes[DDIR_TRIM];
+
+	if (td->o.io_limit)
+		limit = td->o.io_limit;
+	else
+		limit = td->o.size;
+
+	return bytes >= limit || exceeds_number_ios(td);
+}
+
 /*
  * Main IO worker function. It retrieves io_u's to process and queues
  * and reaps them, checking for rate and errors along the way.
@@ -714,7 +735,7 @@ static uint64_t do_io(struct thread_data *td)
 		total_bytes += td->o.size;
 
 	while ((td->o.read_iolog_file && !flist_empty(&td->io_log_list)) ||
-		(!flist_empty(&td->trim_list)) || !io_bytes_exceeded(td) ||
+		(!flist_empty(&td->trim_list)) || !io_issue_bytes_exceeded(td) ||
 		td->o.time_based) {
 		struct timeval comp_time;
 		struct io_u *io_u;
@@ -1231,7 +1252,7 @@ static uint64_t do_dry_run(struct thread_data *td)
 	td_set_runstate(td, TD_RUNNING);
 
 	while ((td->o.read_iolog_file && !flist_empty(&td->io_log_list)) ||
-		(!flist_empty(&td->trim_list)) || !io_bytes_exceeded(td)) {
+		(!flist_empty(&td->trim_list)) || !io_complete_bytes_exceeded(td)) {
 		struct io_u *io_u;
 		int ret;
 
-- 
1.7.1

Re: verify_only broken? (was RE: )

[Jens Axboe]

On 02/04/2015 02:58 PM, Justin Eno (jeno) wrote:
> On Wed, Jan 28, 2015 at 14:39:29, Justin Eno (jeno) <jeno@micron.com> wrote:
>> Hi Jens,
>>
>> It appears do_dry_run(), i.e., verify_only, is broken by the last
>> update to backend.c
>>
>> I have attached a working patch, though you may prefer a different approach.
>>
>> Thanks,
>> Justin
>
> I just realized that I sent this original message without a subject. Perhaps it was dropped for that reason.
>
> I've re-attached the same patch. If it was NAKed, please just let me know and I will work on an alternative.
>
> Also, do you prefer inline or attached patches?

Thanks, didn't see the original. I prefer attached patches, that has a 
lower risk of some mailer or MTA corrupting it along the way. But unless 
it's a huge patch, it's not a big deal.

I have applied your patch.

-- 
Jens Axboe