On 03/21/2010 07:56 PM, Douglas Gilbert wrote: > Boaz, > Thanks for the review. > > Now I decided to check the SG_IO ioctl used directly > against block devices and it calls: > blk_execute_rq(q, bd_disk, rq, 0); > > The last argument is 'at_head' so it has been queuing > at_tail for some time. How is that for compatibility?? > > That almost suggests there should be a > #define SG_FLAG_Q_AT_HEAD 0x20 > > added to sg.h to cover all the bases. > > Doug Gilbert > > OK I dug up the original patch and actually It was the same with bsg. See the commit log: commit 05378940caf979a8655c18b18a17213dcfa52412 Author: Boaz Harrosh <bharrosh@xxxxxxxxxxx> Date: Tue Mar 24 12:23:40 2009 +0100 bsg: add support for tail queuing Currently inherited from sg.c bsg will submit asynchronous request at the head-of-the-queue, (using "at_head" set in the call to blk_execute_rq_nowait()). This is bad in situation where the queues are full, requests will execute out of order, and can cause starvation of the first submitted requests. The sg_io_v4->flags member is used and a bit is allocated to denote the Q_AT_TAIL. Zero is to queue at_head as before, to be compatible with old code at the write/read path. SG_IO code path behavior was changed so to be the same as write/read behavior. SG_IO was very rarely used and breaking compatibility with it is OK at this stage. sg_io_hdr at sg.h also has a flags member and uses 3 bits from the first nibble and one bit from the last nibble. Even though none of these bits are supported by bsg, The second nibble is allocated for use by bsg. Just in case. Signed-off-by: Boaz Harrosh <bharrosh@xxxxxxxxxxx> CC: Douglas Gilbert <dgilbert@xxxxxxxxxxxx> Signed-off-by: Jens Axboe <jens.axboe@xxxxxxxxxx> (See second paragraph) So bsg had the same compatibility problem between SG_IO and "write". What happened is that I found a nasty bug in bsg's SG_IO, just at the same time, which proves that SG_IO was never used, (since it would just crash), so I decided that it would be cost-less to just break compatibility with some thing that never work, for the sake of simple API sameness. I do realize now that I have made it hard for sg and SG_IO to comply to new API but retain back compatibility. (Sorry) Here is my suggestion: - Allocate 2 bits at flags, [mask 0x30] - Have an enum for these two flags with this meaning: Q_AT_API_COMPATIBLE = 0, This means for sg and SG_IO to keep their old behaviour. sg.c - at_head SG_IO to device - at_tail bsg both cases - it is at_head. Q_AT_TAIL = 1, This means at_tail for all APIs Q_AT_HEAD = 2, This means at_head for all APIs Q_AT_RESERVED = 3, Should not be used So this is effectively your idea as well but just elaborated on more. If we do that I think we should maybe join headers, by moving common defines to sg.h and #include sg.h from bsg.h. Sigh! Boaz -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html