[PATCH v2] sg: add SG_FLAG_Q_AT_TAIL flag

[PATCH v2] sg: add SG_FLAG_Q_AT_TAIL flag

[Douglas Gilbert]

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

When the SG_IO ioctl was copied into the block layer and
later into the bsg driver, subtle differences emerged.

One difference is the way injected commands are queued through
the block layer (i.e. this is not SCSI device queueing nor SATA
NCQ). Summarizing:
   - SG_IO in the block layer: blk_exec*(at_head=false)
   - sg SG_IO: at_head=true
   - bsg SG_IO: at_head=true

Some time ago Boaz Harrosh introduced a sg v4 flag called
BSG_FLAG_Q_AT_TAIL to override the bsg driver default.
This patch does the equivalent for the sg driver.


ChangeLog:
     Introduce SG_FLAG_Q_AT_TAIL flag to cause commands
     to be injected into the block layer with
     at_head=false.

Changes since v1:
     Make guard condition (only take sg v3 interface or later
     invocations) clearer.

Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>

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

diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 177f755..616ed81 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -740,7 +740,7 @@ static int
 sg_common_write(Sg_fd * sfp, Sg_request * srp,
 		unsigned char *cmnd, int timeout, int blocking)
 {
-	int k, data_dir;
+	int k, data_dir, at_head;
 	Sg_device *sdp = sfp->parentdp;
 	sg_io_hdr_t *hp = &srp->header;
 
@@ -784,11 +784,16 @@ sg_common_write(Sg_fd * sfp, Sg_request * srp,
 		break;
 	}
 	hp->duration = jiffies_to_msecs(jiffies);
+	if (hp->interface_id != '\0' &&	/* v3 (or later) interface */
+	    (SG_FLAG_Q_AT_TAIL & hp->flags))
+		at_head = 0;
+	else
+		at_head = 1;
 
 	srp->rq->timeout = timeout;
 	kref_get(&sfp->f_ref); /* sg_rq_end_io() does kref_put(). */
 	blk_execute_rq_nowait(sdp->device->request_queue, sdp->disk,
-			      srp->rq, 1, sg_rq_end_io);
+			      srp->rq, at_head, sg_rq_end_io);
 	return 0;
 }
 
diff --git a/include/scsi/sg.h b/include/scsi/sg.h
index d8c0c43..9859355 100644
--- a/include/scsi/sg.h
+++ b/include/scsi/sg.h
@@ -86,6 +86,7 @@ typedef struct sg_io_hdr
 #define SG_FLAG_MMAP_IO 4       /* request memory mapped IO */
 #define SG_FLAG_NO_DXFER 0x10000 /* no transfer of kernel buffers to/from */
 				/* user space (debug indirect IO) */
+#define SG_FLAG_Q_AT_TAIL 0x10  /* default is Q_AT_HEAD */
 
 /* following 'info' values are "or"-ed together */
 #define SG_INFO_OK_MASK 0x1

Re: [PATCH v2] sg: add SG_FLAG_Q_AT_TAIL flag

[Christoph Hellwig]

On Wed, Jun 04, 2014 at 10:58:30AM -0400, Douglas Gilbert wrote:
> When the SG_IO ioctl was copied into the block layer and
> later into the bsg driver, subtle differences emerged.
> 
> One difference is the way injected commands are queued through
> the block layer (i.e. this is not SCSI device queueing nor SATA
> NCQ). Summarizing:
>   - SG_IO in the block layer: blk_exec*(at_head=false)
>   - sg SG_IO: at_head=true
>   - bsg SG_IO: at_head=true
> 
> Some time ago Boaz Harrosh introduced a sg v4 flag called
> BSG_FLAG_Q_AT_TAIL to override the bsg driver default.
> This patch does the equivalent for the sg driver.

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Any chance to get a patch for the block-layer SG_IO code, too?

Re: [PATCH v2] sg: add SG_FLAG_Q_AT_TAIL flag

[Boaz Harrosh]

On 06/04/2014 05:58 PM, Douglas Gilbert wrote:
> When the SG_IO ioctl was copied into the block layer and
> later into the bsg driver, subtle differences emerged.
> 
> One difference is the way injected commands are queued through
> the block layer (i.e. this is not SCSI device queueing nor SATA
> NCQ). Summarizing:
>    - SG_IO in the block layer: blk_exec*(at_head=false)
>    - sg SG_IO: at_head=true
>    - bsg SG_IO: at_head=true
> 
> Some time ago Boaz Harrosh introduced a sg v4 flag called
> BSG_FLAG_Q_AT_TAIL to override the bsg driver default.
> This patch does the equivalent for the sg driver.
> 

Yep

> 
> ChangeLog:
>      Introduce SG_FLAG_Q_AT_TAIL flag to cause commands
>      to be injected into the block layer with
>      at_head=false.
> 
> Changes since v1:
>      Make guard condition (only take sg v3 interface or later
>      invocations) clearer.
> 
> Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
> 

Douglas Hi

Please I'm just curious? up until now all application users
used "SG_FLAG_Q_AT_HEAD". Therefor I deduce that you guys
came across a new application that can make use of the new
SG_FLAG_Q_AT_TAIL facility.

What was the application's writer considerations for using
the old sg interface and not use the newer bsg interface
that already has this support. For me I can see 2 main areas
that I find bsg missing.

1. aio "scatter_gather" type io. 
   (ie multiple pointers multiple length buffers that are
    written / read from same linear range on device)
  [The async aspect of aio can be implemented via bsg
   with the write+read system calls]
2. mmap of direct device range to user vm memory

Which of these, or other, considerations where cardinal
in using of the old interface in new code?

(For me personally both above shortcomings are very
 much missed]

Thanks
Boaz

Re: [PATCH v2] sg: add SG_FLAG_Q_AT_TAIL flag

[Jeremy Linton]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 6/5/2014 10:27 AM, Boaz Harrosh wrote:

> 1. aio "scatter_gather" type io. (ie multiple pointers multiple length
> buffers that are written / read from same linear range on device) [The
> async aspect of aio can be implemented via bsg with the write+read system
> calls] 2. mmap of direct device range to user vm memory

	I suspect that belies a bit of a gap in the understanding of the kinds of
applications that use pass-through (vs just using sd, or using it for a guest OS).

	These use cases don't tend to be useful for things like SCSI changers, tape
devices, or SES devices. What is useful is the ability to reset devices, or
maybe some of the other "edge" features provided by SG that never managed to
make it into bsg. Nor are they useful for the monitoring type applications
that use pass-through to pull some vendor specific statistic or device state.

	Furthermore, i've see a fair number of cases where people slap together shell
scripts using the /dev/sg* handles instead of the /dev/bsg/* ones probably
because its simply more convenient.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (MingW32)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBAgAGBQJTkKYLAAoJEL5i86xrzcy78ZEIAK9s8hcgtX3bloYbW+09OHWu
M12ySzk6hEOvJcGZwoBobkG5q9cHPk1ehaCtzaTE5MlBaSOSfg+AAHVUusr3PUZR
REmwS+eBZu6wRghXPE6c0oLuBulQ1FeJXkDsfuRhkaoBfZxfc/BiTEb67CCbHPm4
gT34VCiVRB0G0Sp5rnu9S9f1LvRmF2DoMCK+CmCBNh0q/dD3EskQJOh5c9sAKHKJ
0TO1LyuRj5jUILgOma/gHX3LHa7JN9EE+DKK5mm8s75vMKwv8FpWMc6B9LeOfcIn
XDDMM5tdrtbXMvZ6M5jp+bhbnoydxhRHgXBpiTMe3ze4VZXXLdmSBX/am9oVhKA=
=TdvH
-----END PGP SIGNATURE-----

Re: [PATCH v2] sg: add SG_FLAG_Q_AT_TAIL flag

[Douglas Gilbert]

On 14-06-05 11:27 AM, Boaz Harrosh wrote:
> On 06/04/2014 05:58 PM, Douglas Gilbert wrote:
>> When the SG_IO ioctl was copied into the block layer and
>> later into the bsg driver, subtle differences emerged.
>>
>> One difference is the way injected commands are queued through
>> the block layer (i.e. this is not SCSI device queueing nor SATA
>> NCQ). Summarizing:
>>     - SG_IO in the block layer: blk_exec*(at_head=false)
>>     - sg SG_IO: at_head=true
>>     - bsg SG_IO: at_head=true
>>
>> Some time ago Boaz Harrosh introduced a sg v4 flag called
>> BSG_FLAG_Q_AT_TAIL to override the bsg driver default.
>> This patch does the equivalent for the sg driver.
>>
>
> Yep
>
>>
>> ChangeLog:
>>       Introduce SG_FLAG_Q_AT_TAIL flag to cause commands
>>       to be injected into the block layer with
>>       at_head=false.
>>
>> Changes since v1:
>>       Make guard condition (only take sg v3 interface or later
>>       invocations) clearer.
>>
>> Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
>>
>
> Douglas Hi
>
> Please I'm just curious? up until now all application users
> used "SG_FLAG_Q_AT_HEAD". Therefor I deduce that you guys
> came across a new application that can make use of the new
> SG_FLAG_Q_AT_TAIL facility.

sg_dd and more recently ddpt request the equivalent of
SG_FLAG_Q_AT_TAIL on SCSI READs and WRITEs. So that is
the default with a Linux block device, implemented with
a bsg device, and ignored with a sg device. When you
think of dd with POSIX threading (e.g. sgp_dd) then
SG_FLAG_Q_AT_HEAD seems rather counter-productive.

Also WRITE_ATOMIC with SG_FLAG_Q_AT_HEAD seems to be asking
for trouble (unless power failure was imminent). OTOH
an INQUIRY with SG_FLAG_Q_AT_TAIL is a bit strange as
well (as it is implicit "head of queue" through SCSI device
queues).

> What was the application's writer considerations for using
> the old sg interface and not use the newer bsg interface
> that already has this support. For me I can see 2 main areas
> that I find bsg missing.
>
> 1. aio "scatter_gather" type io.
>     (ie multiple pointers multiple length buffers that are
>      written / read from same linear range on device)
>    [The async aspect of aio can be implemented via bsg
>     with the write+read system calls]
> 2. mmap of direct device range to user vm memory
>
> Which of these, or other, considerations where cardinal
> in using of the old interface in new code?
>
> (For me personally both above shortcomings are very
>   much missed]

Not a subject that I wanted to bring up by since you ask ...


The bsg driver first appeared in the kernel in lk 2.6.23 which
was released in October 2007. It probably took a few releases to
be usable, lets say it has been in place for 6 years. In that
time there have been no new features added to the sg driver while
several new features have been added to bsg (e.g. async support)
but in the last year or so, it has lost its active maintainer:
Fujita Tomonori (aka Tomo). Various bugs have been reported
against the bsg driver (actually some against the sg driver and
tests showed the bsg driver had the same problem of worse). No
fixes have been presented for any of bsg's recently reported
problems as far as I am aware.

Consider me a part-time kernel driver maintainer so the sg and
scsi_debug drivers are more than enough for me. I have no desire
to pick up the bsg driver. Volunteers can contact Jens.


As a tool maker I get another view of various SCSI (and related)
pass-throughs across several OSes. sg3_utils has been ambivalent
about which Linux SCSI pass-through it uses since version 1.27
[20090411], about 5 years. Judging from my email, Linux users
demonstrate problems and suggestions using the these devices,
ordered by frequency:
   - block devices (e.g. /dev/sdc)
   - sg devices
   - bsg devices

I could be wrong about the order of the first two, but bsg devices
are a long way last (e.g. a handful in the last two years). At a
low level, bsg is not helped by only supporting the v4 interface
while block devices and the sg devices only support the v3
interface. To most sg3_utils users this is almost invisible since
its library chooses between the two interfaces dynamically. Only
issuing something like VERIFY(32) via sg_verify to a Linux sg
device would demonstrate a difference (i.e. it would work with
a bsg device, fail with a sg device).

I also maintain the SCSI side of smartmontools and adding bsg
support is on my ToDo list. The most active maintainer has
responded: why bother? And that seems to be a common response
from those familiar with these issues.

So the bsg driver, IMO, has become a niche player, with some
important niches. OSD users need bi-directional, variable
length commands. Several transports use the bsg driver as a
transport layer pass-through. For example my smp_utils package
uses the bsg driver in that fashion, thus avoiding several
proprietary interfaces.


The O_EXCL issue is instructive. Vaughan Cao brought this up
last year (or earlier ?). Basically the sg driver's handling
of the O_EXCL flag was flaky, not obviously, but if you
threw enough threads at it, bad things happened. So I wrote
some test programs (in sg3_utils: examples/sg_tst_excl.cpp and
friends). The Linux block device interface doesn't survive
these tests either. And the bsg driver: it just accepts and
ignores the O_EXCL flag! [You might argue that POSIX doesn't
define O_EXCL semantics on a device node.] Obviously some
folks out there care and have taken my patches (based on
Vaughan's work) into their kernels. Those patches are not
in the mainline yet; they may be re-presented soon.


So here is a list:
   Pro bsg:
     - better interface (v4), should be able to handle most
       request/response, data_in/data_out type protocols
     - has several mature transport layer pass-throughs: can
       do more than issue SCSI commands
     - faster (about the same speed as the block layer)
     - handles bi-directional SCSI commands
     - handles cdb length > 16 bytes (sg patch pending)

   Pro sg:
     - slower! (checking more things takes time, locking needs
       cleanup, no blk_execute_rq() fastpath)
     - its interface has been stable for more than 10 years
       with backward compatibility to earlier interfaces that
       are over 20 years old
     - async interface that works
     - supports things like O_EXCL (imperfectly)
     - has better debugging ('cat /proc/scsi/sg/debug') and
       error reporting (very few instances of EIO)
     - mmap and related options for moving data
     - aio "scatter_gather" type io
     - more familiar device nodes (e.g. /dev/sg3)
     - supports a few specials like those to reset SCSI devices,
       targets and hosts


I'm not proposing any major changes. Microsoft has two variants
of its SCSI pass-through, so if the Linux pass-through users
are relatively happy with the current situation, then it ain't
broke so doesn't need fixing.


All that said, I sometimes toy with the idea of adding the v4
interface to the sg driver. After all it must be about 10 years
since Jens said that wasn't going to happen ...

Doug Gilbert

Re: [PATCH v2] sg: add SG_FLAG_Q_AT_TAIL flag

[Ewan Milne]

On Wed, 2014-06-04 at 10:58 -0400, Douglas Gilbert wrote:
> When the SG_IO ioctl was copied into the block layer and
> later into the bsg driver, subtle differences emerged.
> 
> One difference is the way injected commands are queued through
> the block layer (i.e. this is not SCSI device queueing nor SATA
> NCQ). Summarizing:
>    - SG_IO in the block layer: blk_exec*(at_head=false)
>    - sg SG_IO: at_head=true
>    - bsg SG_IO: at_head=true
> 
> Some time ago Boaz Harrosh introduced a sg v4 flag called
> BSG_FLAG_Q_AT_TAIL to override the bsg driver default.
> This patch does the equivalent for the sg driver.
> 
> 
> ChangeLog:
>      Introduce SG_FLAG_Q_AT_TAIL flag to cause commands
>      to be injected into the block layer with
>      at_head=false.
> 
> Changes since v1:
>      Make guard condition (only take sg v3 interface or later
>      invocations) clearer.
> 
> Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>

Looks good.

Reviewed-by: Ewan D. Milne <emilne@redhat.com>

Re: [PATCH v2] sg: add SG_FLAG_Q_AT_TAIL flag

[Mike Christie]

On 06/04/2014 09:58 AM, Douglas Gilbert wrote:
> When the SG_IO ioctl was copied into the block layer and
> later into the bsg driver, subtle differences emerged.
> 
> One difference is the way injected commands are queued through
> the block layer (i.e. this is not SCSI device queueing nor SATA
> NCQ). Summarizing:
>   - SG_IO in the block layer: blk_exec*(at_head=false)
>   - sg SG_IO: at_head=true
>   - bsg SG_IO: at_head=true
> 
> Some time ago Boaz Harrosh introduced a sg v4 flag called
> BSG_FLAG_Q_AT_TAIL to override the bsg driver default.
> This patch does the equivalent for the sg driver.
> 
> 
> ChangeLog:
>     Introduce SG_FLAG_Q_AT_TAIL flag to cause commands
>     to be injected into the block layer with
>     at_head=false.
> 
> Changes since v1:
>     Make guard condition (only take sg v3 interface or later
>     invocations) clearer.
> 
> Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>

Looks ok to me.

Reviewed-by: Mike Christie <michaelc@cs.wisc.edu>