Skip to contentSkip to conversation

Allowing REQ_FAILFAST to be set from SG_IO

PATCH/RFC Hannes Reinecke 4 participants 7 messages May 10, 2006 – May 11, 2006

Subsystems (3)
SCSI Block layer SCSI SG driver
lore
Share & tools
lore

Hannes Reinecke

author
#0 lore raw

Hi all,

this patch allows to set the REQ_FAILFAST flag to be set from SG_IO commands. With it one can send generic SCSI commands which behave exactly like normal requests send via the block layer.

Comments etc welcome.

Cheers,

Hannes

Signature · 4 lines
Dr. Hannes Reinecke			[email protected]
SuSE Linux Products GmbH		S390 & zSeries
Maxfeldstraße 5				+49 911 74053 688
90409 Nürnberg				http://www.suse.de

Allow FAILFAST flag to be set for SG_IO commands

This patch allows the FAILFAST flag to be set for SG_IO commands.

Signed-off-by Hannes Reinecke <[email protected]>
block/scsi_ioctl.c +2 -0
index b33eda2..658b1b5 100644
@@ -295,6 +295,8 @@ static int sg_io(struct file *file, requ
 	rq->sense_len = 0;
 
 	rq->flags |= REQ_BLOCK_PC;
+	if (hdr->flags & SG_FLAGS_FAILFAST)
+		rq->flags |= REQ_FAILFAST;
 	bio = rq->bio;
 
 	/*
include/scsi/sg.h +1 -0
index 0a487fe..9cc466a 100644
@@ -142,6 +142,7 @@ typedef struct sg_io_hdr
 #define SG_FLAG_UNUSED_LUN_INHIBIT 2   /* default is overwrite lun in SCSI */
 				/* command block (when <= SCSI_2) */
 #define SG_FLAG_MMAP_IO 4       /* request memory mapped IO */
+#define SG_FLAG_FAILFAST 8      /* request should set FAILFAST flag */
 #define SG_FLAG_NO_DXFER 0x10000 /* no transfer of kernel buffers to/from */
 				/* user space (debug indirect IO) */
#1 lore raw
Quoted 7 lines · Hannes Reinecke

Hi all,

this patch allows to set the REQ_FAILFAST flag to be set from SG_IO commands. With it one can send generic SCSI commands which behave exactly like normal requests send via the block layer.

Comments etc welcome.

Hannes,
Looks good. There is no proposed change to sg.c . Could SG_FLAG_FAILFAST be made to work via sg device nodes?

Doug Gilbert

Quoted 33 lines

------------------------------------------------------------------------

Allow FAILFAST flag to be set for SG_IO commands

This patch allows the FAILFAST flag to be set for SG_IO commands.

Signed-off-by Hannes Reinecke <[email protected]>
block/scsi_ioctl.c +2 -0
index b33eda2..658b1b5 100644
@@ -295,6 +295,8 @@ static int sg_io(struct file *file, requ
 	rq->sense_len = 0;
 
 	rq->flags |= REQ_BLOCK_PC;
+	if (hdr->flags & SG_FLAGS_FAILFAST)
+		rq->flags |= REQ_FAILFAST;
 	bio = rq->bio;
 
 	/*
include/scsi/sg.h +1 -0
index 0a487fe..9cc466a 100644
@@ -142,6 +142,7 @@ typedef struct sg_io_hdr
 #define SG_FLAG_UNUSED_LUN_INHIBIT 2   /* default is overwrite lun in SCSI */
 				/* command block (when <= SCSI_2) */
 #define SG_FLAG_MMAP_IO 4       /* request memory mapped IO */
+#define SG_FLAG_FAILFAST 8      /* request should set FAILFAST flag */
 #define SG_FLAG_NO_DXFER 0x10000 /* no transfer of kernel buffers to/from */
 				/* user space (debug indirect IO) */
#2 lore raw
Quoted 3 lines · Douglas Gilbert

Looks good. There is no proposed change to sg.c . Could SG_FLAG_FAILFAST be made to work via sg device nodes?

Rather than having to maintain two separate paths for SG_IO and hence keep running into this issue, what will it take for sg to use the block scsi_ioctl.c?

James

#3 lore raw
Quoted 10 lines · James Bottomley
Quoted 3 lines · Douglas Gilbert

Looks good. There is no proposed change to sg.c . Could SG_FLAG_FAILFAST be made to work via sg device nodes?

Rather than having to maintain two separate paths for SG_IO and hence keep running into this issue, what will it take for sg to use the block scsi_ioctl.c?

Methinks a large amount of code and a lot of testing. Hannes is proposing a new SG_FLAG value that would only be active in the block layer SG_IO. The existing SG_FLAG values are only implemented by the sg driver. Around that level in the sg driver, the command injection and response processing are asynchronous and that can be exposed to the sg user. The block SG_IO is synchronous with no easy way to break it down to its component parts.

On further thought if anything in the scsi midlayer (or the block layer) is slowing down an error report getting back to a sg user then that is most likely a bug. The sg driver sends commands with retries set to zero. Also settings in the read/write error recovery mode page can effect how long a disk (or cd/drive) spends trying to recovery from errors (and SG_FLAG_FASTFAIL has no effect at the device (lu) end).

Doug Gilbert

author
#4 lore raw
Quoted 21 lines · Douglas Gilbert
Quoted 9 lines · James Bottomley
Quoted 3 lines · Douglas Gilbert

Looks good. There is no proposed change to sg.c . Could SG_FLAG_FAILFAST be made to work via sg device nodes?

Rather than having to maintain two separate paths for SG_IO and hence keep running into this issue, what will it take for sg to use the block scsi_ioctl.c?

Methinks a large amount of code and a lot of testing. Hannes is proposing a new SG_FLAG value that would only be active in the block layer SG_IO. The existing SG_FLAG values are only implemented by the sg driver. Around that level in the sg driver, the command injection and response processing are asynchronous and that can be exposed to the sg user. The block SG_IO is synchronous with no easy way to break it down to its component parts.

Quoted 9 lines

On further thought if anything in the scsi midlayer (or the block layer) is slowing down an error report getting back to a sg user then that is most likely a bug. The sg driver sends commands with retries set to zero. Also settings in the read/write error recovery mode page can effect how long a disk (or cd/drive) spends trying to recovery from errors (and SG_FLAG_FASTFAIL has no effect at the device (lu) end).

That doesn't apply here. The error recovery in scsi_unjam_host() will be triggered regardless of the retry setting. Setting the retry to zero will only cause the command _not_ to be retried; if the device still reports an error condition the error handler will still be invoked.

One might argue that no error recovery is required for sg.c commands as the user will have full control (up to and including bus / device reset) so he's responsible to implement his own error recovery. In that case we should always send command from sg.c down with the failfast flag set.

But that's a different discussion.

Cheers,

Hannes

Signature · 8 lines
Dr. Hannes Reinecke			[email protected]
SuSE Linux Products GmbH		S390 & zSeries
Maxfeldstraße 5				+49 911 74053 688
90409 Nürnberg				http://www.suse.de
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#5 lore raw
In reply to James Bottomley
Quoted 8 lines · James Bottomley
Quoted 3 lines · Douglas Gilbert

Looks good. There is no proposed change to sg.c . Could SG_FLAG_FAILFAST be made to work via sg device nodes?

Rather than having to maintain two separate paths for SG_IO and hence keep running into this issue, what will it take for sg to use the block scsi_ioctl.c?

The plan still remains to use bsg to consolidate the two.

Signature · 1 line
Jens Axboe
author
#6 lore raw
In reply to Douglas Gilbert
Quoted 7 lines · Douglas Gilbert
Quoted 6 lines · Hannes Reinecke

Hi all,

this patch allows to set the REQ_FAILFAST flag to be set from SG_IO commands. With it one can send generic SCSI commands which behave exactly like normal requests send via the block layer.

Quoted 7 lines
Quoted 1 line

Comments etc welcome.

Hannes,
Looks good. There is no proposed change to sg.c . Could SG_FLAG_FAILFAST be made to work via sg device nodes?

That will be tricky. sg.c is using scsi_execute_async() which doesn't provide for any flags to be passed.

Cheers,

Hannes

Signature · 8 lines
Dr. Hannes Reinecke			[email protected]
SuSE Linux Products GmbH		S390 & zSeries
Maxfeldstraße 5				+49 911 74053 688
90409 Nürnberg				http://www.suse.de
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html