diff mbox

IO-Controller: clear ioq wait flag if a request goes into that ioq

Message ID 4A8CAAE2.1030804@cn.fujitsu.com (mailing list archive)
State New, archived
Headers show

Commit Message

Gui Jianfeng Aug. 20, 2009, 1:46 a.m. UTC
Vivek Goyal wrote:
...
>  		/*
>  		 * Remember that we saw a request from this process, but
> @@ -1940,7 +2013,7 @@ void elv_ioq_request_add(struct request_queue *q, struct request *rq)
>  		 * has other work pending, don't risk delaying until the
>  		 * idle timer unplug to continue working.
>  		 */
> -		if (elv_ioq_wait_request(ioq)) {
> +		if (group_wait || elv_ioq_wait_request(ioq)) {

Hi Vivek,

I guess we need to clear ioq_wait_request flags if there are requests to 
go in this ioq. Otherwise, once waitting request on ioq, it will go into
this path every time when a request is enqueued.

Signed-off-by: Gui Jianfeng <guijianfeng@cn.fujitsu.com>
---
 block/elevator-fq.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

Comments

Vivek Goyal Aug. 20, 2009, 1:42 p.m. UTC | #1
On Thu, Aug 20, 2009 at 09:46:10AM +0800, Gui Jianfeng wrote:
> Vivek Goyal wrote:
> ...
> >  		/*
> >  		 * Remember that we saw a request from this process, but
> > @@ -1940,7 +2013,7 @@ void elv_ioq_request_add(struct request_queue *q, struct request *rq)
> >  		 * has other work pending, don't risk delaying until the
> >  		 * idle timer unplug to continue working.
> >  		 */
> > -		if (elv_ioq_wait_request(ioq)) {
> > +		if (group_wait || elv_ioq_wait_request(ioq)) {
> 
> Hi Vivek,
> 
> I guess we need to clear ioq_wait_request flags if there are requests to 
> go in this ioq. Otherwise, once waitting request on ioq, it will go into
> this path every time when a request is enqueued.
> 
> Signed-off-by: Gui Jianfeng <guijianfeng@cn.fujitsu.com>

Hi Gui,

It does sound like that we need to clear ioq wait_request flag here. In
fact upstream CFQ code is also not clearing this flag. Looking at the
code, can't think why it should not be cleared here.

Can you please also generate a patch for CFQ and post to lkml.

Thanks
Vivek
 
> ---
>  block/elevator-fq.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/block/elevator-fq.c b/block/elevator-fq.c
> index b3c387d..201543e 100644
> --- a/block/elevator-fq.c
> +++ b/block/elevator-fq.c
> @@ -2708,6 +2708,9 @@ void elv_ioq_request_add(struct request_queue *q, struct request *rq)
>  				__blk_run_queue(q);
>  			else
>  				elv_mark_ioq_must_dispatch(ioq);
> +
> +			if (elv_ioq_wait_request(ioq))
> +				elv_clear_ioq_wait_request(ioq);
>  		}
>  	} else if (elv_should_preempt(q, ioq, rq)) {
>  		/*
> -- 
> 1.5.4.rc3

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
Gui Jianfeng Aug. 21, 2009, 12:57 a.m. UTC | #2
Vivek Goyal wrote:
> On Thu, Aug 20, 2009 at 09:46:10AM +0800, Gui Jianfeng wrote:
>> Vivek Goyal wrote:
>> ...
>>>  		/*
>>>  		 * Remember that we saw a request from this process, but
>>> @@ -1940,7 +2013,7 @@ void elv_ioq_request_add(struct request_queue *q, struct request *rq)
>>>  		 * has other work pending, don't risk delaying until the
>>>  		 * idle timer unplug to continue working.
>>>  		 */
>>> -		if (elv_ioq_wait_request(ioq)) {
>>> +		if (group_wait || elv_ioq_wait_request(ioq)) {
>> Hi Vivek,
>>
>> I guess we need to clear ioq_wait_request flags if there are requests to 
>> go in this ioq. Otherwise, once waitting request on ioq, it will go into
>> this path every time when a request is enqueued.
>>
>> Signed-off-by: Gui Jianfeng <guijianfeng@cn.fujitsu.com>
> 
> Hi Gui,
> 
> It does sound like that we need to clear ioq wait_request flag here. In
> fact upstream CFQ code is also not clearing this flag. Looking at the
> code, can't think why it should not be cleared here.
> 
> Can you please also generate a patch for CFQ and post to lkml.
> 

Sure, will do.
diff mbox

Patch

diff --git a/block/elevator-fq.c b/block/elevator-fq.c
index b3c387d..201543e 100644
--- a/block/elevator-fq.c
+++ b/block/elevator-fq.c
@@ -2708,6 +2708,9 @@  void elv_ioq_request_add(struct request_queue *q, struct request *rq)
 				__blk_run_queue(q);
 			else
 				elv_mark_ioq_must_dispatch(ioq);
+
+			if (elv_ioq_wait_request(ioq))
+				elv_clear_ioq_wait_request(ioq);
 		}
 	} else if (elv_should_preempt(q, ioq, rq)) {
 		/*