[PATCH] engines/sg: improve error handling

[PATCH] engines/sg: improve error handling

[Vincent Fu]

From: Vincent Fu <Vincent.Fu@wdc.com>

The Linux sg driver accepts only 16 SCSI commands in flight
at a time per file descriptor. fio does not exit gracefully
when it attempts to queue up 17 or more trim commands via
the sg ioengine. This patch improves error handling in the
sg ioengine commit function to achieve a graceful exit if
fio attempts to queue up too many trim commands. The key to
this is calling clear_io_u on each io_u in the 17th trim
command. With this patch fio no longer loops forever
waiting for the IOs in the (not succesffully submitted)
17th trim command to complete.
---
 engines/sg.c | 49 +++++++++++++++++++++++++------------------------
 1 file changed, 25 insertions(+), 24 deletions(-)

diff --git a/engines/sg.c b/engines/sg.c
index 7741f83..3cc068f 100644
--- a/engines/sg.c
+++ b/engines/sg.c
@@ -675,36 +675,37 @@ static int fio_sgio_commit(struct thread_data *td)
 
 	ret = fio_sgio_rw_doio(io_u->file, io_u, 0);
 
-	if (ret < 0)
-		for (i = 0; i < st->unmap_range_count; i++)
-			st->trim_io_us[i]->error = errno;
-	else if (hdr->status)
-		for (i = 0; i < st->unmap_range_count; i++) {
-			st->trim_io_us[i]->resid = hdr->resid;
-			st->trim_io_us[i]->error = EIO;
+	if (ret < 0 || hdr->status) {
+		int error;
+
+		if (ret < 0)
+			error = errno;
+		else {
+			error = EIO;
+			ret = -EIO;
 		}
-	else {
-		if (fio_fill_issue_time(td)) {
-			fio_gettime(&now, NULL);
-			for (i = 0; i < st->unmap_range_count; i++) {
-				struct io_u *io_u = st->trim_io_us[i];
-
-				memcpy(&io_u->issue_time, &now, sizeof(now));
-				io_u_queued(td, io_u);
-			}
+
+		for (i = 0; i < st->unmap_range_count; i++) {
+			st->trim_io_us[i]->error = error;
+			clear_io_u(td, st->trim_io_us[i]);
+			if (hdr->status)
+				st->trim_io_us[i]->resid = hdr->resid;
 		}
-		io_u_mark_submit(td, st->unmap_range_count);
+
+		td_verror(td, error, "xfer");
+		return ret;
 	}
 
-	if (io_u->error) {
-		td_verror(td, io_u->error, "xfer");
-		return 0;
+	if (fio_fill_issue_time(td)) {
+		fio_gettime(&now, NULL);
+		for (i = 0; i < st->unmap_range_count; i++) {
+			memcpy(&st->trim_io_us[i]->issue_time, &now, sizeof(now));
+			io_u_queued(td, io_u);
+		}
 	}
+	io_u_mark_submit(td, st->unmap_range_count);
 
-	if (ret == FIO_Q_QUEUED)
-		return 0;
-	else
-		return ret;
+	return 0;
 }
 
 static struct io_u *fio_sgio_event(struct thread_data *td, int event)
-- 
2.7.4

Re: [PATCH] engines/sg: improve error handling

[Sitsofe Wheeler]

On Mon, 27 Aug 2018 at 21:41, Vincent Fu <vincentfu@gmail.com> wrote:
>
> From: Vincent Fu <Vincent.Fu@wdc.com>
>
> The Linux sg driver accepts only 16 SCSI commands in flight
> at a time per file descriptor. fio does not exit gracefully
> when it attempts to queue up 17 or more trim commands via
> the sg ioengine. This patch improves error handling in the

Does this mean the engine should refuse iodepths greater than 16 in
general or are there multiple descriptors?

-- 
Sitsofe | http://sucs.org/~sits/

Re: [PATCH] engines/sg: improve error handling

[Vincent Fu]

On 08/27/2018 06:41 PM, Sitsofe Wheeler wrote:
> On Mon, 27 Aug 2018 at 21:41, Vincent Fu <vincentfu@gmail.com> wrote:
>> From: Vincent Fu <Vincent.Fu@wdc.com>
>>
>> The Linux sg driver accepts only 16 SCSI commands in flight
>> at a time per file descriptor. fio does not exit gracefully
>> when it attempts to queue up 17 or more trim commands via
>> the sg ioengine. This patch improves error handling in the
> Does this mean the engine should refuse iodepths greater than 16 in
> general or are there multiple descriptors?
>
The engine should not refuse iodepths greater than 16 in general. The
purpose of the recent sg changes was to support high QD trim workloads
by sending multiple ranges in a single SCSI command. --rw=randtrim
--iodepth=256 --iodepth_batch=16 will work whereas --rw=randtrim
--iodepth=257 --iodepth_batch=16 will fail.

Re: [PATCH] engines/sg: improve error handling

[Jens Axboe]

On 8/27/18 2:40 PM, Vincent Fu wrote:
> From: Vincent Fu <Vincent.Fu@wdc.com>
> 
> The Linux sg driver accepts only 16 SCSI commands in flight
> at a time per file descriptor. fio does not exit gracefully
> when it attempts to queue up 17 or more trim commands via
> the sg ioengine. This patch improves error handling in the
> sg ioengine commit function to achieve a graceful exit if
> fio attempts to queue up too many trim commands. The key to
> this is calling clear_io_u on each io_u in the 17th trim
> command. With this patch fio no longer loops forever
> waiting for the IOs in the (not succesffully submitted)
> 17th trim command to complete.

Applied, thanks Vincent.

-- 
Jens Axboe