* [PATCH] bsg: Add support for submitting requests at tail of queue
@ 2009-01-21 9:52 Boaz Harrosh
2009-01-21 23:24 ` FUJITA Tomonori
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Boaz Harrosh @ 2009-01-21 9:52 UTC (permalink / raw)
To: FUJITA Tomonori, Jens Axboe, Douglas Gilbert, linux-scsi,
open-osd
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 we want
to keep the queues full but need the requests to execute in order.
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.
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@panasas.com>
CC: Douglas Gilbert <dgilbert@interlog.com>
---
block/bsg.c | 3 ++-
include/linux/bsg.h | 8 ++++++++
2 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/block/bsg.c b/block/bsg.c
index 44a2a0f..43e4344 100644
--- a/block/bsg.c
+++ b/block/bsg.c
@@ -350,6 +350,7 @@ static void bsg_rq_end_io(struct request *rq, int uptodate)
static void bsg_add_command(struct bsg_device *bd, struct request_queue *q,
struct bsg_command *bc, struct request *rq)
{
+ int at_head = (0 == (bc->hdr.flags & BSG_FLAG_Q_AT_TAIL));
/*
* add bc command to busy queue and submit rq for io
*/
@@ -365,7 +366,7 @@ static void bsg_add_command(struct bsg_device *bd, struct request_queue *q,
dprintk("%s: queueing rq %p, bc %p\n", bd->name, rq, bc);
rq->end_io_data = bc;
- blk_execute_rq_nowait(q, NULL, rq, 1, bsg_rq_end_io);
+ blk_execute_rq_nowait(q, NULL, rq, at_head, bsg_rq_end_io);
}
static struct bsg_command *bsg_next_done_cmd(struct bsg_device *bd)
diff --git a/include/linux/bsg.h b/include/linux/bsg.h
index cf0303a..3f0c64a 100644
--- a/include/linux/bsg.h
+++ b/include/linux/bsg.h
@@ -7,6 +7,14 @@
#define BSG_SUB_PROTOCOL_SCSI_TMF 1
#define BSG_SUB_PROTOCOL_SCSI_TRANSPORT 2
+/*
+ * For flags member below
+ * sg.h sg_io_hdr also has bits defined for it's flags member. However
+ * none of these bits are implemented/used by bsg. The bits below are
+ * allocated to not conflict with sg.h ones anyway.
+ */
+#define BSG_FLAG_Q_AT_TAIL 0x10 /* default, == 0 at this bit, is Q_AT_HEAD */
+
struct sg_io_v4 {
__s32 guard; /* [i] 'Q' to differentiate from v3 */
__u32 protocol; /* [i] 0 -> SCSI , .... */
--
1.6.0.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] bsg: Add support for submitting requests at tail of queue
2009-01-21 9:52 [PATCH] bsg: Add support for submitting requests at tail of queue Boaz Harrosh
@ 2009-01-21 23:24 ` FUJITA Tomonori
2009-01-22 8:57 ` Boaz Harrosh
2009-01-22 11:13 ` Jens Axboe
2009-01-25 9:41 ` [PATCH version2] " Boaz Harrosh
2009-01-25 10:07 ` [PATCH version 3] " Boaz Harrosh
2 siblings, 2 replies; 13+ messages in thread
From: FUJITA Tomonori @ 2009-01-21 23:24 UTC (permalink / raw)
To: bharrosh; +Cc: fujita.tomonori, jens.axboe, dougg, linux-scsi, osd-dev
On Wed, 21 Jan 2009 11:52:39 +0200
Boaz Harrosh <bharrosh@panasas.com> wrote:
>
> 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 we want
> to keep the queues full but need the requests to execute in order.
As I wrote, I think that blk_execute_rq_nowait inserts a request and
plugs a queue. So how can you keep the queue full? On the completion
of blk_execute_rq_nowait, the queue is empty.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] bsg: Add support for submitting requests at tail of queue
2009-01-21 23:24 ` FUJITA Tomonori
@ 2009-01-22 8:57 ` Boaz Harrosh
2009-01-22 11:13 ` Jens Axboe
1 sibling, 0 replies; 13+ messages in thread
From: Boaz Harrosh @ 2009-01-22 8:57 UTC (permalink / raw)
To: FUJITA Tomonori; +Cc: jens.axboe, dougg, linux-scsi, osd-dev
FUJITA Tomonori wrote:
> On Wed, 21 Jan 2009 11:52:39 +0200
> Boaz Harrosh <bharrosh@panasas.com> wrote:
>
>> 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 we want
>> to keep the queues full but need the requests to execute in order.
>
> As I wrote, I think that blk_execute_rq_nowait inserts a request and
> plugs a queue. So how can you keep the queue full? On the completion
> of blk_execute_rq_nowait, the queue is empty.
If it's so then why does the "at_head" matters why does it exist?
Let me say it this way, if it does not matter then could we just
reverse it? you just said it does not matter, lets reverse it
to be 0 and not 1.
I don't understand what you are saying. bsg::write can accept an
array of commands lets say I have this sequence in the array:
command1:command2:command3
And for the sake of the argument lets say the hosts queue is full,
has can_queue commands waiting response. Let say from other clients.
if I use at_head==0, the target will surly see this sequence:
command1:command2:command3
But if I use at_head==1, the way I read the code, the target will see"
command3:command2:command1 and some times
command1:command3:command2 or command2:command1:command3
It all depends on the random completions of the other commands
in flight.
This is the way I read the code, If I read it wrong, please explain
what will be the actual behavior and what is the effect of at_head?
Thanks
Boaz
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] bsg: Add support for submitting requests at tail of queue
2009-01-21 23:24 ` FUJITA Tomonori
2009-01-22 8:57 ` Boaz Harrosh
@ 2009-01-22 11:13 ` Jens Axboe
2009-01-22 12:44 ` Boaz Harrosh
1 sibling, 1 reply; 13+ messages in thread
From: Jens Axboe @ 2009-01-22 11:13 UTC (permalink / raw)
To: FUJITA Tomonori; +Cc: bharrosh, dougg, linux-scsi, osd-dev
On Thu, Jan 22 2009, FUJITA Tomonori wrote:
> On Wed, 21 Jan 2009 11:52:39 +0200
> Boaz Harrosh <bharrosh@panasas.com> wrote:
>
> >
> > 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 we want
> > to keep the queues full but need the requests to execute in order.
>
> As I wrote, I think that blk_execute_rq_nowait inserts a request and
> plugs a queue. So how can you keep the queue full? On the completion
> of blk_execute_rq_nowait, the queue is empty.
That's not true at all. If you submit more than one request, request 2
and up would be queued according to the orientation given. It may even
include request 1 as well, what if the queue is busy doing work for
someone else already?
I think the patch makes sense, I also wish that the default would have
been reversed so that at_back would be the default. at_back is more
complex though, since it impacts existing requests for the device (it
drains the scheduler queue).
--
Jens Axboe
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] bsg: Add support for submitting requests at tail of queue
2009-01-22 11:13 ` Jens Axboe
@ 2009-01-22 12:44 ` Boaz Harrosh
2009-01-22 12:46 ` Jens Axboe
0 siblings, 1 reply; 13+ messages in thread
From: Boaz Harrosh @ 2009-01-22 12:44 UTC (permalink / raw)
To: Jens Axboe; +Cc: FUJITA Tomonori, dougg, linux-scsi, osd-dev
Jens Axboe wrote:
> On Thu, Jan 22 2009, FUJITA Tomonori wrote:
>> On Wed, 21 Jan 2009 11:52:39 +0200
>> Boaz Harrosh <bharrosh@panasas.com> wrote:
>>
>>> 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 we want
>>> to keep the queues full but need the requests to execute in order.
>> As I wrote, I think that blk_execute_rq_nowait inserts a request and
>> plugs a queue. So how can you keep the queue full? On the completion
>> of blk_execute_rq_nowait, the queue is empty.
>
> That's not true at all. If you submit more than one request, request 2
> and up would be queued according to the orientation given. It may even
> include request 1 as well, what if the queue is busy doing work for
> someone else already?
>
> I think the patch makes sense, I also wish that the default would have
> been reversed so that at_back would be the default. at_back is more
> complex though, since it impacts existing requests for the device (it
> drains the scheduler queue).
>
bsg only sends BLOCK_PC commands, I think these are not held in the
scheduler queue, right?
I think like you at_back is the default to use, but this is historic
compatibility with SG that had problems with ABORT and such not. With
my patch I give the user a choice and be done with it.
Thanks
Boaz
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] bsg: Add support for submitting requests at tail of queue
2009-01-22 12:44 ` Boaz Harrosh
@ 2009-01-22 12:46 ` Jens Axboe
2009-01-22 22:03 ` FUJITA Tomonori
0 siblings, 1 reply; 13+ messages in thread
From: Jens Axboe @ 2009-01-22 12:46 UTC (permalink / raw)
To: Boaz Harrosh; +Cc: FUJITA Tomonori, dougg, linux-scsi, osd-dev
On Thu, Jan 22 2009, Boaz Harrosh wrote:
> Jens Axboe wrote:
> > On Thu, Jan 22 2009, FUJITA Tomonori wrote:
> >> On Wed, 21 Jan 2009 11:52:39 +0200
> >> Boaz Harrosh <bharrosh@panasas.com> wrote:
> >>
> >>> 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 we want
> >>> to keep the queues full but need the requests to execute in order.
> >> As I wrote, I think that blk_execute_rq_nowait inserts a request and
> >> plugs a queue. So how can you keep the queue full? On the completion
> >> of blk_execute_rq_nowait, the queue is empty.
> >
> > That's not true at all. If you submit more than one request, request 2
> > and up would be queued according to the orientation given. It may even
> > include request 1 as well, what if the queue is busy doing work for
> > someone else already?
> >
> > I think the patch makes sense, I also wish that the default would have
> > been reversed so that at_back would be the default. at_back is more
> > complex though, since it impacts existing requests for the device (it
> > drains the scheduler queue).
> >
>
> bsg only sends BLOCK_PC commands, I think these are not held in the
> scheduler queue, right?
They are not, correct. But that queue may have other requests pending,
which may be "normal" file system requests. Then an at_back bsg command
would act almost like a barrier, draining the entire queue to dispatch.
> I think like you at_back is the default to use, but this is historic
> compatibility with SG that had problems with ABORT and such not. With
> my patch I give the user a choice and be done with it.
at_head is definitely useful and required for some situations, but that
doesn't mean it's a good default :-). But yes, we are stuck with it. I
think the patch makes sense.
--
Jens Axboe
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] bsg: Add support for submitting requests at tail of queue
2009-01-22 12:46 ` Jens Axboe
@ 2009-01-22 22:03 ` FUJITA Tomonori
2009-01-22 22:27 ` James Bottomley
0 siblings, 1 reply; 13+ messages in thread
From: FUJITA Tomonori @ 2009-01-22 22:03 UTC (permalink / raw)
To: jens.axboe; +Cc: bharrosh, fujita.tomonori, dougg, linux-scsi, osd-dev
On Thu, 22 Jan 2009 13:46:41 +0100
Jens Axboe <jens.axboe@oracle.com> wrote:
> On Thu, Jan 22 2009, Boaz Harrosh wrote:
> > Jens Axboe wrote:
> > > On Thu, Jan 22 2009, FUJITA Tomonori wrote:
> > >> On Wed, 21 Jan 2009 11:52:39 +0200
> > >> Boaz Harrosh <bharrosh@panasas.com> wrote:
> > >>
> > >>> 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 we want
> > >>> to keep the queues full but need the requests to execute in order.
> > >> As I wrote, I think that blk_execute_rq_nowait inserts a request and
> > >> plugs a queue. So how can you keep the queue full? On the completion
> > >> of blk_execute_rq_nowait, the queue is empty.
> > >
> > > That's not true at all. If you submit more than one request, request 2
> > > and up would be queued according to the orientation given. It may even
> > > include request 1 as well, what if the queue is busy doing work for
> > > someone else already?
> > >
> > > I think the patch makes sense, I also wish that the default would have
> > > been reversed so that at_back would be the default. at_back is more
> > > complex though, since it impacts existing requests for the device (it
> > > drains the scheduler queue).
> > >
> >
> > bsg only sends BLOCK_PC commands, I think these are not held in the
> > scheduler queue, right?
Right, that's what I meant.
> They are not, correct. But that queue may have other requests pending,
> which may be "normal" file system requests. Then an at_back bsg command
> would act almost like a barrier, draining the entire queue to dispatch.
Theoretically, yes. But for what do you want to mix pc and fs commands
with a single device and send a pc command 'at the end of the queue'?
And as you know, you can assume that Boaz always cares about only
OSD. ;) There is no way in mainline now to send fs commands to OSD.
So, Boaz, what do you want to do exactly? It should have in the patch
description. I don't want to add something that nobody uses.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] bsg: Add support for submitting requests at tail of queue
2009-01-22 22:03 ` FUJITA Tomonori
@ 2009-01-22 22:27 ` James Bottomley
2009-01-23 6:14 ` FUJITA Tomonori
0 siblings, 1 reply; 13+ messages in thread
From: James Bottomley @ 2009-01-22 22:27 UTC (permalink / raw)
To: FUJITA Tomonori; +Cc: jens.axboe, bharrosh, dougg, linux-scsi, osd-dev
On Fri, 2009-01-23 at 07:03 +0900, FUJITA Tomonori wrote:
> So, Boaz, what do you want to do exactly? It should have in the patch
> description. I don't want to add something that nobody uses.
OK, can we step back a bit from this? Everyone seems to be talking past
each other. The original complaint was that multiple commands against
the same device issued by SG_IO could be executed "out of order". This
is really irrelevant because we never guarantee execution order in the
first place.
However, if you consider our current at head insertion policy coupled
with a multi-threaded application issuing hundreds of SG_IO requests at
once, you can see we have a potential starvation issue: Commands at the
tail of the queue end up pushed further and further back as more
commands are added to the head. This starvation issue is worth
addressing, I think, and it can only be addressed by allowing tail
insertion.
The original reason for at head is, as you surmise, inherited from sg
and the rationale is largely for error handling: you need error
handling commands to pre-empt everything in the current queue.
James
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] bsg: Add support for submitting requests at tail of queue
2009-01-22 22:27 ` James Bottomley
@ 2009-01-23 6:14 ` FUJITA Tomonori
2009-01-25 9:17 ` Boaz Harrosh
0 siblings, 1 reply; 13+ messages in thread
From: FUJITA Tomonori @ 2009-01-23 6:14 UTC (permalink / raw)
To: James.Bottomley
Cc: fujita.tomonori, jens.axboe, bharrosh, dougg, linux-scsi, osd-dev
On Thu, 22 Jan 2009 16:27:59 -0600
James Bottomley <James.Bottomley@HansenPartnership.com> wrote:
> On Fri, 2009-01-23 at 07:03 +0900, FUJITA Tomonori wrote:
> > So, Boaz, what do you want to do exactly? It should have in the patch
> > description. I don't want to add something that nobody uses.
>
> OK, can we step back a bit from this? Everyone seems to be talking past
> each other. The original complaint was that multiple commands against
> the same device issued by SG_IO could be executed "out of order". This
> is really irrelevant because we never guarantee execution order in the
> first place.
>
> However, if you consider our current at head insertion policy coupled
> with a multi-threaded application issuing hundreds of SG_IO requests at
> once, you can see we have a potential starvation issue: Commands at the
> tail of the queue end up pushed further and further back as more
> commands are added to the head. This starvation issue is worth
> addressing, I think, and it can only be addressed by allowing tail
> insertion.
Ah, I see. Thanks. We could see this with something busy.
BTW, bsg write interface enables you to send a command asynchronously
so a single-thread-ed application could cause this.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] bsg: Add support for submitting requests at tail of queue
2009-01-23 6:14 ` FUJITA Tomonori
@ 2009-01-25 9:17 ` Boaz Harrosh
0 siblings, 0 replies; 13+ messages in thread
From: Boaz Harrosh @ 2009-01-25 9:17 UTC (permalink / raw)
To: FUJITA Tomonori; +Cc: James.Bottomley, jens.axboe, dougg, linux-scsi, osd-dev
FUJITA Tomonori wrote:
> On Thu, 22 Jan 2009 16:27:59 -0600
> James Bottomley <James.Bottomley@HansenPartnership.com> wrote:
>
>> On Fri, 2009-01-23 at 07:03 +0900, FUJITA Tomonori wrote:
>>> So, Boaz, what do you want to do exactly? It should have in the patch
>>> description. I don't want to add something that nobody uses.
>> OK, can we step back a bit from this? Everyone seems to be talking past
>> each other. The original complaint was that multiple commands against
>> the same device issued by SG_IO could be executed "out of order". This
>> is really irrelevant because we never guarantee execution order in the
>> first place.
out-of-order execution happens very rarely and I can live with that,
as long as statistically, over an iscsi connection, they are submitted
in order, then there are optimizations that can take advantage of this.
(Actually I have never observed an out of order submission to a target)
>>
>> However, if you consider our current at head insertion policy coupled
>> with a multi-threaded application issuing hundreds of SG_IO requests at
>> once, you can see we have a potential starvation issue: Commands at the
>> tail of the queue end up pushed further and further back as more
>> commands are added to the head. This starvation issue is worth
>> addressing, I think, and it can only be addressed by allowing tail
>> insertion.
>
> Ah, I see. Thanks. We could see this with something busy.
That was my point. Submitting when queues are full.
>
> BTW, bsg write interface enables you to send a command asynchronously
> so a single-thread-ed application could cause this.
However ...
The patch I submitted is not good enough. I have only added control for
the write/read path, Not the SG_IO path. The last one was left at_tail,
as before. But this is very bad, we should absolutely keep both interfaces
the same, as much as we can.
Now here we have a problem. write/read was default "at_head" SG_IO was default
"at_tail". If we want to absolutely keep backward compatibility with old
applications we need two bits. One - BSG_FLAG_Q_AT_TAIL but another one
BSG_FLAG_Q_FLAG_VALID.
In the light that I found a bad bug with SG_IO just recently, and for a fact
I know that Pete and all his osd guys only used the write/read method. I would
say that SG_IO applications are very rare, and could be disregarded.
TOMO, Jens, Please decide what you want to do. I'll post as reply to first patch
the one-bit-clean version. If you decide I will be happy to send a two-bit-compatible
version ASAP.
Thanks
Boaz
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH version2] bsg: Add support for submitting requests at tail of queue
2009-01-21 9:52 [PATCH] bsg: Add support for submitting requests at tail of queue Boaz Harrosh
2009-01-21 23:24 ` FUJITA Tomonori
@ 2009-01-25 9:41 ` Boaz Harrosh
2009-01-25 9:44 ` Boaz Harrosh
2009-01-25 10:07 ` [PATCH version 3] " Boaz Harrosh
2 siblings, 1 reply; 13+ messages in thread
From: Boaz Harrosh @ 2009-01-25 9:41 UTC (permalink / raw)
To: FUJITA Tomonori, Jens Axboe, Douglas Gilbert
Cc: linux-scsi, open-osd mailing-list, James Bottomley
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 but requests need to execute in order. Failing to do that can
cause the first submitted commands to never execute.
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@panasas.com>
CC: Douglas Gilbert <dgilbert@interlog.com>
---
block/bsg.c | 9 +++++++--
include/linux/bsg.h | 8 ++++++++
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/block/bsg.c b/block/bsg.c
index 44a2a0f..2e55655 100644
--- a/block/bsg.c
+++ b/block/bsg.c
@@ -350,6 +350,8 @@ static void bsg_rq_end_io(struct request *rq, int uptodate)
static void bsg_add_command(struct bsg_device *bd, struct request_queue *q,
struct bsg_command *bc, struct request *rq)
{
+ int at_head = (0 == (bc->hdr.flags & BSG_FLAG_Q_AT_TAIL));
+
/*
* add bc command to busy queue and submit rq for io
*/
@@ -365,7 +367,7 @@ static void bsg_add_command(struct bsg_device *bd, struct request_queue *q,
dprintk("%s: queueing rq %p, bc %p\n", bd->name, rq, bc);
rq->end_io_data = bc;
- blk_execute_rq_nowait(q, NULL, rq, 1, bsg_rq_end_io);
+ blk_execute_rq_nowait(q, NULL, rq, at_head, bsg_rq_end_io);
}
static struct bsg_command *bsg_next_done_cmd(struct bsg_device *bd)
@@ -921,6 +923,7 @@ static long bsg_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
struct request *rq;
struct bio *bio, *bidi_bio = NULL;
struct sg_io_v4 hdr;
+ int at_head;
u8 sense[SCSI_SENSE_BUFFERSIZE];
if (copy_from_user(&hdr, uarg, sizeof(hdr)))
@@ -933,7 +936,9 @@ static long bsg_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
bio = rq->bio;
if (rq->next_rq)
bidi_bio = rq->next_rq->bio;
- blk_execute_rq(bd->queue, NULL, rq, 0);
+
+ at_head = (0 == (bc->hdr.flags & BSG_FLAG_Q_AT_TAIL));
+ blk_execute_rq(bd->queue, NULL, rq, at_head);
ret = blk_complete_sgv4_hdr_rq(rq, &hdr, bio, bidi_bio);
if (copy_to_user(uarg, &hdr, sizeof(hdr)))
diff --git a/include/linux/bsg.h b/include/linux/bsg.h
index cf0303a..3f0c64a 100644
--- a/include/linux/bsg.h
+++ b/include/linux/bsg.h
@@ -7,6 +7,14 @@
#define BSG_SUB_PROTOCOL_SCSI_TMF 1
#define BSG_SUB_PROTOCOL_SCSI_TRANSPORT 2
+/*
+ * For flags member below
+ * sg.h sg_io_hdr also has bits defined for it's flags member. However
+ * none of these bits are implemented/used by bsg. The bits below are
+ * allocated to not conflict with sg.h ones anyway.
+ */
+#define BSG_FLAG_Q_AT_TAIL 0x10 /* default, == 0 at this bit, is Q_AT_HEAD */
+
struct sg_io_v4 {
__s32 guard; /* [i] 'Q' to differentiate from v3 */
__u32 protocol; /* [i] 0 -> SCSI , .... */
--
1.6.0.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH version2] bsg: Add support for submitting requests at tail of queue
2009-01-25 9:41 ` [PATCH version2] " Boaz Harrosh
@ 2009-01-25 9:44 ` Boaz Harrosh
0 siblings, 0 replies; 13+ messages in thread
From: Boaz Harrosh @ 2009-01-25 9:44 UTC (permalink / raw)
To: FUJITA Tomonori, Jens Axboe, Douglas Gilbert
Cc: linux-scsi, open-osd mailing-list, James Bottomley
Boaz Harrosh wrote:
> 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 but requests need to execute in order. Failing to do that can
> cause the first submitted commands to never execute.
>
> 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@panasas.com>
> CC: Douglas Gilbert <dgilbert@interlog.com>
> ---
> block/bsg.c | 9 +++++++--
> include/linux/bsg.h | 8 ++++++++
> 2 files changed, 15 insertions(+), 2 deletions(-)
>
Sorrrrrrry bad patch, teach me to never send a patch in the morning
before coffee.
Will send a version 3
Boaz
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH version 3] bsg: Add support for submitting requests at tail of queue
2009-01-21 9:52 [PATCH] bsg: Add support for submitting requests at tail of queue Boaz Harrosh
2009-01-21 23:24 ` FUJITA Tomonori
2009-01-25 9:41 ` [PATCH version2] " Boaz Harrosh
@ 2009-01-25 10:07 ` Boaz Harrosh
2 siblings, 0 replies; 13+ messages in thread
From: Boaz Harrosh @ 2009-01-25 10:07 UTC (permalink / raw)
To: FUJITA Tomonori, Jens Axboe
Cc: Douglas Gilbert, linux-scsi, open-osd mailing-list,
James Bottomley
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@panasas.com>
CC: Douglas Gilbert <dgilbert@interlog.com>
---
block/bsg.c | 9 +++++++--
include/linux/bsg.h | 8 ++++++++
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/block/bsg.c b/block/bsg.c
index 44a2a0f..206060e 100644
--- a/block/bsg.c
+++ b/block/bsg.c
@@ -350,6 +350,8 @@ static void bsg_rq_end_io(struct request *rq, int uptodate)
static void bsg_add_command(struct bsg_device *bd, struct request_queue *q,
struct bsg_command *bc, struct request *rq)
{
+ int at_head = (0 == (bc->hdr.flags & BSG_FLAG_Q_AT_TAIL));
+
/*
* add bc command to busy queue and submit rq for io
*/
@@ -365,7 +367,7 @@ static void bsg_add_command(struct bsg_device *bd, struct request_queue *q,
dprintk("%s: queueing rq %p, bc %p\n", bd->name, rq, bc);
rq->end_io_data = bc;
- blk_execute_rq_nowait(q, NULL, rq, 1, bsg_rq_end_io);
+ blk_execute_rq_nowait(q, NULL, rq, at_head, bsg_rq_end_io);
}
static struct bsg_command *bsg_next_done_cmd(struct bsg_device *bd)
@@ -921,6 +923,7 @@ static long bsg_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
struct request *rq;
struct bio *bio, *bidi_bio = NULL;
struct sg_io_v4 hdr;
+ int at_head;
u8 sense[SCSI_SENSE_BUFFERSIZE];
if (copy_from_user(&hdr, uarg, sizeof(hdr)))
@@ -933,7 +936,9 @@ static long bsg_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
bio = rq->bio;
if (rq->next_rq)
bidi_bio = rq->next_rq->bio;
- blk_execute_rq(bd->queue, NULL, rq, 0);
+
+ at_head = (0 == (hdr.flags & BSG_FLAG_Q_AT_TAIL));
+ blk_execute_rq(bd->queue, NULL, rq, at_head);
ret = blk_complete_sgv4_hdr_rq(rq, &hdr, bio, bidi_bio);
if (copy_to_user(uarg, &hdr, sizeof(hdr)))
diff --git a/include/linux/bsg.h b/include/linux/bsg.h
index cf0303a..3f0c64a 100644
--- a/include/linux/bsg.h
+++ b/include/linux/bsg.h
@@ -7,6 +7,14 @@
#define BSG_SUB_PROTOCOL_SCSI_TMF 1
#define BSG_SUB_PROTOCOL_SCSI_TRANSPORT 2
+/*
+ * For flags member below
+ * sg.h sg_io_hdr also has bits defined for it's flags member. However
+ * none of these bits are implemented/used by bsg. The bits below are
+ * allocated to not conflict with sg.h ones anyway.
+ */
+#define BSG_FLAG_Q_AT_TAIL 0x10 /* default, == 0 at this bit, is Q_AT_HEAD */
+
struct sg_io_v4 {
__s32 guard; /* [i] 'Q' to differentiate from v3 */
__u32 protocol; /* [i] 0 -> SCSI , .... */
--
1.6.0.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
end of thread, other threads:[~2009-01-25 10:07 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-01-21 9:52 [PATCH] bsg: Add support for submitting requests at tail of queue Boaz Harrosh
2009-01-21 23:24 ` FUJITA Tomonori
2009-01-22 8:57 ` Boaz Harrosh
2009-01-22 11:13 ` Jens Axboe
2009-01-22 12:44 ` Boaz Harrosh
2009-01-22 12:46 ` Jens Axboe
2009-01-22 22:03 ` FUJITA Tomonori
2009-01-22 22:27 ` James Bottomley
2009-01-23 6:14 ` FUJITA Tomonori
2009-01-25 9:17 ` Boaz Harrosh
2009-01-25 9:41 ` [PATCH version2] " Boaz Harrosh
2009-01-25 9:44 ` Boaz Harrosh
2009-01-25 10:07 ` [PATCH version 3] " Boaz Harrosh
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox