[dm-devel] Re: [PATCH] io-controller: Preempt a non-rt queue if a rt ioq is present in ancestor or sibling groups

[Vivek Goyal <vgoyal redhat com>]

On Mon, Jun 22, 2009 at 03:44:08PM +0800, Gui Jianfeng wrote:
> Preempt the ongoing non-rt ioq if there are rt ioqs waiting for dispatching
> in ancestor or sibling groups. It will give other group's rt ioq an chance 
> to dispatch ASAP.
> 

Hi Gui,

Will new preempton logic of traversing up the hiearchy so that both new
queue and old queue are at same level to take a preemption decision not
take care of above scenario?

Please have a look at bfq_find_matching_entity().

At the same time we probably don't want to preempt the non-rt queue
with an RT queue in sibling group until and unless sibling group is an
RT group.

		root
		/  \
	   BEgrpA  BEgrpB
	      |     |	
	  BEioq1   RTioq2

Above we got two BE group A and B and assume ioq in group A is being
served and then an RT request in group B comes. Because group B is an
BE class group, we should not preempt the queue in group A.

Thanks
Vivek


> Signed-off-by: Gui Jianfeng <guijianfeng cn fujitsu com>
> ---
>  block/elevator-fq.c |   44 +++++++++++++++++++++++++++++++++++++++-----
>  block/elevator-fq.h |    1 +
>  2 files changed, 40 insertions(+), 5 deletions(-)
> 
> diff --git a/block/elevator-fq.c b/block/elevator-fq.c
> index 2ad40eb..80526fd 100644
> --- a/block/elevator-fq.c
> +++ b/block/elevator-fq.c
> @@ -3245,8 +3245,16 @@ void elv_add_ioq_busy(struct elv_fq_data *efqd, struct io_queue *ioq)
>  	elv_mark_ioq_busy(ioq);
>  	efqd->busy_queues++;
>  	if (elv_ioq_class_rt(ioq)) {
> +		struct io_entity *entity;
>  		struct io_group *iog = ioq_to_io_group(ioq);
> +
>  		iog->busy_rt_queues++;
> +		entity = iog->entity.parent;
> +
> +		for_each_entity(entity) {
> +			iog = io_entity_to_iog(entity);
> +			iog->sub_busy_rt_queues++;
> +		}
>  	}
>  
>  #ifdef CONFIG_DEBUG_GROUP_IOSCHED
> @@ -3290,9 +3298,18 @@ void elv_del_ioq_busy(struct elevator_queue *e, struct io_queue *ioq,
>  	elv_clear_ioq_busy(ioq);
>  	BUG_ON(efqd->busy_queues == 0);
>  	efqd->busy_queues--;
> +
>  	if (elv_ioq_class_rt(ioq)) {
> +		struct io_entity *entity;
>  		struct io_group *iog = ioq_to_io_group(ioq);
> +
>  		iog->busy_rt_queues--;
> +		entity = iog->entity.parent;
> +
> +		for_each_entity(entity) {
> +			iog = io_entity_to_iog(entity);
> +			iog->sub_busy_rt_queues--;
> +		}
>  	}
>  
>  	elv_deactivate_ioq(efqd, ioq, requeue);
> @@ -3735,12 +3752,32 @@ int elv_iosched_expire_ioq(struct request_queue *q, int slice_expired,
>  	return ret;
>  }
>  
> +static int check_rt_queue(struct io_queue *ioq)
> +{
> +	struct io_group *iog;
> +	struct io_entity *entity;
> +
> +	iog = ioq_to_io_group(ioq);
> +
> +	if (iog->busy_rt_queues)
> +		return 1;
> +
> +	entity = iog->entity.parent;
> +
> +	for_each_entity(entity) {
> +		iog = io_entity_to_iog(entity);
> +		if (iog->sub_busy_rt_queues)
> +			return 1;
> +	}
> +
> +	return 0;
> +}
> +
>  /* Common layer function to select the next queue to dispatch from */
>  void *elv_fq_select_ioq(struct request_queue *q, int force)
>  {
>  	struct elv_fq_data *efqd = &q->elevator->efqd;
>  	struct io_queue *new_ioq = NULL, *ioq = elv_active_ioq(q->elevator);
> -	struct io_group *iog;
>  	int slice_expired = 1;
>  
>  	if (!elv_nr_busy_ioq(q->elevator))
> @@ -3811,12 +3848,9 @@ void *elv_fq_select_ioq(struct request_queue *q, int force)
>  	/*
>  	 * If we have a RT cfqq waiting, then we pre-empt the current non-rt
>  	 * cfqq.
> -	 *
> -	 * TODO: This does not seem right across the io groups. Fix it.
>  	 */
> -	iog = ioq_to_io_group(ioq);
>  
> -	if (!elv_ioq_class_rt(ioq) && iog->busy_rt_queues) {
> +	if (!elv_ioq_class_rt(ioq) && check_rt_queue(ioq)) {
>  		/*
>  		 * We simulate this as cfqq timed out so that it gets to bank
>  		 * the remaining of its time slice.
> diff --git a/block/elevator-fq.h b/block/elevator-fq.h
> index b3193f8..be6c1af 100644
> --- a/block/elevator-fq.h
> +++ b/block/elevator-fq.h
> @@ -248,6 +248,7 @@ struct io_group {
>  	 * non-RT cfqq in service when this value is non-zero.
>  	 */
>  	unsigned int busy_rt_queues;
> +	unsigned int sub_busy_rt_queues;
>  
>  	int deleting;
>  	unsigned short iocg_id;
> -- 
> 1.5.4.rc3
>