Re: [PATCH] IO Controller: No need to stop idling in as

[Vivek Goyal <vgoyal@redhat.com>]

On Fri, Mar 27, 2009 at 02:58:34PM +0800, Gui Jianfeng wrote:
> Vivek Goyal wrote:
> 
> >  		.elevator_alloc_sched_queue_fn = as_alloc_as_queue,
> >  		.elevator_free_sched_queue_fn = as_free_as_queue,
> > +#ifdef CONFIG_IOSCHED_AS_HIER
> > +		.elevator_expire_ioq_fn =       as_expire_ioq,
> > +		.elevator_active_ioq_set_fn =   as_active_ioq_set,
> >  	},
> > -
> > +	.elevator_features = ELV_IOSCHED_NEED_FQ | ELV_IOSCHED_SINGLE_IOQ | ELV_IOSCHED_DONT_IDLE,
> 
>   Hi Vivek,
> 
>   I found the IO Controller doesn't work in as.
>   I dig into this issue, and notice that you stop idling in as. IMHO, this will cause
>   active ioq is always expired when tring to choose a new ioq to serve(elv_fq_select_ioq). 
>   Because idling is disabled, active ioq can't be kept anymore.
>   So i just get rid of ELV_IOSCHED_DONT_IDLE, and it works fine this time.
> 

Hi Gui,

Thanks for the testing. I have not enabled idling for AS in common layer
becuase AS has its own idling/anticipation logic. I think we should not have
anticipation going on at two places, common layer as well as individual
io scheduler. That's the reason I have implemented a function
elv_iosched_expire_ioq(), which calls io scheduler to find out if an ioq
can be expired now. 

So in elv_fq_select_ioq(), we call elv_iosched_expire_ioq() and if
ioschduler denies expiration, then we don't expire the queue. In this case
AS can deny the expiration if it is anticipating for next request.

Actually AS is very different and its little tricky to make it work with
common layer, especially in terms of anticipation. This is on my TODO list
but before I fix AS, I wanted to get other things right with common layer,
cfq, noop and deadline.

Thanks
Vivek


> Signed-off-by: Gui Jianfeng <guijianfeng@cn.fujitsu.com>
> ---
>  block/as-iosched.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/block/as-iosched.c b/block/as-iosched.c
> index 27c14a7..499c521 100644
> --- a/block/as-iosched.c
> +++ b/block/as-iosched.c
> @@ -1689,7 +1689,7 @@ static struct elevator_type iosched_as = {
>  		.elevator_expire_ioq_fn =       as_expire_ioq,
>  		.elevator_active_ioq_set_fn =   as_active_ioq_set,
>  	},
> -	.elevator_features = ELV_IOSCHED_NEED_FQ | ELV_IOSCHED_SINGLE_IOQ | ELV_IOSCHED_DONT_IDLE,
> +	.elevator_features = ELV_IOSCHED_NEED_FQ | ELV_IOSCHED_SINGLE_IOQ,
>  #else
>  	},
>  #endif
> -- 
> 1.5.4.rc3
>