Hello,
scsi_execute_async() has replaced scsi_do_req() a few versions ago,
but it also incurred a change of behavior. I noticed that over-queuing
a SCSI device using that function causes I/Os to be starved from
low-level queuing for no justified reason.
I think it makes much more sense to perserve the original behaviour
of scsi_do_req() and add the request to the tail of the queue.
Signed-off-by: Dan Aloni <[email protected]>
diff -p -urN a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
--- a/drivers/scsi/scsi_lib.c 2006-12-19 01:48:50.000000000 +0200
+++ b/drivers/scsi/scsi_lib.c 2006-12-19 01:49:35.000000000 +0200
@@ -421,7 +421,7 @@ int scsi_execute_async(struct scsi_devic
sioc->data = privdata;
sioc->done = done;
- blk_execute_rq_nowait(req->q, NULL, req, 1, scsi_end_async);
+ blk_execute_rq_nowait(req->q, NULL, req, 0, scsi_end_async);
return 0;
free_req:
- Dan
On Tue, 2006-12-19 at 10:35 +0200, Dan Aloni wrote:
> Hello,
>
> scsi_execute_async() has replaced scsi_do_req() a few versions ago,
> but it also incurred a change of behavior. I noticed that over-queuing
> a SCSI device using that function causes I/Os to be starved from
> low-level queuing for no justified reason.
>
> I think it makes much more sense to perserve the original behaviour
> of scsi_do_req() and add the request to the tail of the queue.
Hi,
some things should really be added to the head of the queue, like
maintenance requests and error handling requests. Are you sure this is
the right change? At least I'd expect 2 apis, one for a head and one for
a "normal" queueing...
Greetings,
Arjan van de Ven
On Tue, Dec 19 2006, Arjan van de Ven wrote:
> On Tue, 2006-12-19 at 10:35 +0200, Dan Aloni wrote:
> > Hello,
> >
> > scsi_execute_async() has replaced scsi_do_req() a few versions ago,
> > but it also incurred a change of behavior. I noticed that over-queuing
> > a SCSI device using that function causes I/Os to be starved from
> > low-level queuing for no justified reason.
> >
> > I think it makes much more sense to perserve the original behaviour
> > of scsi_do_req() and add the request to the tail of the queue.
>
> Hi,
>
> some things should really be added to the head of the queue, like
> maintenance requests and error handling requests. Are you sure this is
> the right change? At least I'd expect 2 apis, one for a head and one for
> a "normal" queueing...
It does sounds broken - head insertion should only be used for careful
internal commands, not be the default way user issued commands. Looking
at the current users, the patch makes sense to me.
--
Jens Axboe
On Tue, Dec 19 2006, Jon Escombe wrote:
> Jens Axboe wrote:
> > On Tue, Dec 19 2006, Arjan van de Ven wrote:
> >> On Tue, 2006-12-19 at 10:35 +0200, Dan Aloni wrote:
> >>> Hello,
> >>>
> >>> scsi_execute_async() has replaced scsi_do_req() a few versions ago,
> >>> but it also incurred a change of behavior. I noticed that over-queuing
> >>> a SCSI device using that function causes I/Os to be starved from
> >>> low-level queuing for no justified reason.
> >>>
> >>> I think it makes much more sense to perserve the original behaviour
> >>> of scsi_do_req() and add the request to the tail of the queue.
> >> Hi,
> >>
> >> some things should really be added to the head of the queue, like
> >> maintenance requests and error handling requests. Are you sure this is
> >> the right change? At least I'd expect 2 apis, one for a head and one for
> >> a "normal" queueing...
> >
> > It does sounds broken - head insertion should only be used for careful
> > internal commands, not be the default way user issued commands. Looking
> > at the current users, the patch makes sense to me.
> >
>
> It's worth noting that the hdaps disk protection patches rely on the
> current behaviour to add 'IDLE IMMEDIATE WITH UNLOAD' commands to the
> head of the queue.. Another function, or a new parameter for queue
> position would be needed to retain this functionality - any preference
> for either?
The hdaps disk protection should not be using the SCSI internal
function, so it should not be an issue. The block layer API exposes both
front/back/sort insertion possibilities.
--
Jens Axboe
Jens Axboe wrote:
> On Tue, Dec 19 2006, Arjan van de Ven wrote:
>> On Tue, 2006-12-19 at 10:35 +0200, Dan Aloni wrote:
>>> Hello,
>>>
>>> scsi_execute_async() has replaced scsi_do_req() a few versions ago,
>>> but it also incurred a change of behavior. I noticed that over-queuing
>>> a SCSI device using that function causes I/Os to be starved from
>>> low-level queuing for no justified reason.
>>>
>>> I think it makes much more sense to perserve the original behaviour
>>> of scsi_do_req() and add the request to the tail of the queue.
>> Hi,
>>
>> some things should really be added to the head of the queue, like
>> maintenance requests and error handling requests. Are you sure this is
>> the right change? At least I'd expect 2 apis, one for a head and one for
>> a "normal" queueing...
>
> It does sounds broken - head insertion should only be used for careful
> internal commands, not be the default way user issued commands. Looking
> at the current users, the patch makes sense to me.
>
It's worth noting that the hdaps disk protection patches rely on the
current behaviour to add 'IDLE IMMEDIATE WITH UNLOAD' commands to the
head of the queue.. Another function, or a new parameter for queue
position would be needed to retain this functionality - any preference
for either?
Regards,
Jon.