Re: [PATCH 15/20] blkio: Take care of preemptions across groups

[Vivek Goyal <vgoyal@redhat.com>]

On Wed, Nov 04, 2009 at 02:00:33PM -0500, Jeff Moyer wrote:
> Vivek Goyal <vgoyal@redhat.com> writes:
> 
> > +static bool cfq_should_preempt_group(struct cfq_data *cfqd,
> > +		struct cfq_queue *cfqq, struct cfq_queue *new_cfqq)
> > +{
> > +	struct cfq_entity *cfqe = &cfqq->entity;
> > +	struct cfq_entity *new_cfqe = &new_cfqq->entity;
> > +
> > +	if (cfqq_to_cfqg(cfqq) != &cfqd->root_group)
> > +		cfqe = parent_entity(&cfqq->entity);
> > +
> > +	if (cfqq_to_cfqg(new_cfqq) != &cfqd->root_group)
> > +		new_cfqe = parent_entity(&new_cfqq->entity);
> > +
> > +	/*
> > +	 * Allow an RT request to pre-empt an ongoing non-RT cfqq timeslice.
> > +	 */
> > +
> > +	if (new_cfqe->ioprio_class == IOPRIO_CLASS_RT
> > +	    && cfqe->ioprio_class != IOPRIO_CLASS_RT)
> > +		return true;
> > +	/*
> > +	 * Allow an BE request to pre-empt an ongoing IDLE clas timeslice.
> > +	 */
> > +
> > +	if (new_cfqe->ioprio_class == IOPRIO_CLASS_BE
> > +	    && cfqe->ioprio_class == IOPRIO_CLASS_IDLE)
> > +		return true;
> > +
> > +	return false;
> > +}
> 
> What was the motivation for this?  It seems like this would really break
> isolation.  What if one group has all RT priority tasks, will it starve
> out the other groups?
> 

It will not as we traverse up the hierarchy and look for the ioprio class
of the group entity.

So if you got following configuration where G1 and G2 are two groups. G1
is prio class RT and G2 is prio class BE, then any queue in G1 will
preempt any queue in G2 as at highest level, G1 and G2 are different class
altogether.

			root
			/  \
		       G1  G2

Normal cfqq preemption checks will not catch this. So if G2 has some BE
cfqq running, and some BE queue gets backlogged in G1, this new queue wil
not preempt the queue in G2 and it should have.

That's why preemption checks at group level.

Secondly if G1 and G2 are of ioprioclass BE and all the jobs in G1 are of
RT nature, they will not preempt the queues in G2, hence providing
isolation.

Thanks
Vivek