All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vivek Goyal <vgoyal@redhat.com>
To: Gui Jianfeng <guijianfeng@cn.fujitsu.com>
Cc: linux-kernel@vger.kernel.org,
	containers@lists.linux-foundation.org, dm-devel@redhat.com,
	jens.axboe@oracle.com, nauman@google.com, dpshah@google.com,
	ryov@valinux.co.jp, balbir@linux.vnet.ibm.com,
	righi.andrea@gmail.com, lizf@cn.fujitsu.com, mikew@google.com,
	fchecconi@gmail.com, paolo.valente@unimore.it,
	fernando@oss.ntt.co.jp, s-uchida@ap.jp.nec.com,
	taka@valinux.co.jp, jmoyer@redhat.com, dhaval@linux.vnet.ibm.com,
	m-ikeda@ds.jp.nec.com, agk@redhat.com, akpm@linux-foundation.org,
	peterz@infradead.org
Subject: Re: [PATCH 05/24] io-controller: Modify cfq to make use of flat elevator fair queuing
Date: Fri, 31 Jul 2009 09:18:13 -0400	[thread overview]
Message-ID: <20090731131813.GB3668@redhat.com> (raw)
In-Reply-To: <4A713E10.2030204@cn.fujitsu.com>

On Thu, Jul 30, 2009 at 02:30:40PM +0800, Gui Jianfeng wrote:
> Vivek Goyal wrote:
> ...
> >  /*
> >   * Check if new_cfqq should preempt the currently active queue. Return 0 for
> > - * no or if we aren't sure, a 1 will cause a preempt.
> > + * no or if we aren't sure, a 1 will cause a preemption attempt.
> > + * Some of the preemption logic has been moved to common layer. Only cfq
> > + * specific parts are left here.
> >   */
> >  static int
> > -cfq_should_preempt(struct cfq_data *cfqd, struct cfq_queue *new_cfqq,
> > -		   struct request *rq)
> > +cfq_should_preempt(struct request_queue *q, void *new_cfqq, struct request *rq)
> >  {
> > -	struct cfq_queue *cfqq;
> > +	struct cfq_data *cfqd = q->elevator->elevator_data;
> > +	struct cfq_queue *cfqq = elv_active_sched_queue(q->elevator);
> >  
> > -	cfqq = cfqd->active_queue;
> >  	if (!cfqq)
> >  		return 0;
> >  
> > -	if (cfq_slice_used(cfqq))
> > +	if (elv_ioq_slice_used(cfqq->ioq))
> >  		return 1;
> >  
> >  	if (cfq_class_idle(new_cfqq))
> > @@ -2018,13 +1661,7 @@ cfq_should_preempt(struct cfq_data *cfqd, struct cfq_queue *new_cfqq,
> >  	if (rq_is_meta(rq) && !cfqq->meta_pending)
> >  		return 1;
> >  
> > -	/*
> > -	 * Allow an RT request to pre-empt an ongoing non-RT cfqq timeslice.
> > -	 */
> > -	if (cfq_class_rt(new_cfqq) && !cfq_class_rt(cfqq))
> > -		return 1;
> > -
> > -	if (!cfqd->active_cic || !cfq_cfqq_wait_request(cfqq))
> > +	if (!cfqd->active_cic || !elv_ioq_wait_request(cfqq->ioq))
> >  		return 0;
> >  
> >  	/*
> 
> Hi Vivek,
> 
> cfq_should_preempt() will do the check "if (cfq_rq_close(cfqd, rq)) to see whether
> it should preempt the current cfqq. From fairness point of view,  should we also 
> check "fairness" value, if it's set fairness == 1, don't allow to preempt the current
> cfqq?

Hi Gui,

In V7, fairness=1 means that we try to dispatch request only from one
queue at a time and wait for requests to finish from that queue before
next queue is scheduled in. This helps in better disk time accounting for the
queue.

But currently this is not true for preemption path. So if we decide to
preempt the current queue (either by elevator layer or by cfq), we expire
the queue immediately and bring in the new one. So this is just not
cfq_rq_close() but the whole preemption path.

Currently I will leave it as it is but if we run into significant issues, 
then we can fix it. It will require extra logic of keeping track that
current queue has been preempted. Also keep track who preempted etc and
as soon as last request from queue completes, expire it.

Thanks
Vivek

WARNING: multiple messages have this Message-ID (diff)
From: Vivek Goyal <vgoyal@redhat.com>
To: Gui Jianfeng <guijianfeng@cn.fujitsu.com>
Cc: dhaval@linux.vnet.ibm.com, peterz@infradead.org,
	dm-devel@redhat.com, dpshah@google.com, jens.axboe@oracle.com,
	agk@redhat.com, balbir@linux.vnet.ibm.com,
	paolo.valente@unimore.it, fernando@oss.ntt.co.jp,
	mikew@google.com, jmoyer@redhat.com, nauman@google.com,
	m-ikeda@ds.jp.nec.com, lizf@cn.fujitsu.com, fchecconi@gmail.com,
	s-uchida@ap.jp.nec.com, containers@lists.linux-foundation.org,
	linux-kernel@vger.kernel.org, akpm@linux-foundation.org,
	righi.andrea@gmail.com
Subject: Re: [PATCH 05/24] io-controller: Modify cfq to make use of flat elevator fair queuing
Date: Fri, 31 Jul 2009 09:18:13 -0400	[thread overview]
Message-ID: <20090731131813.GB3668@redhat.com> (raw)
In-Reply-To: <4A713E10.2030204@cn.fujitsu.com>

On Thu, Jul 30, 2009 at 02:30:40PM +0800, Gui Jianfeng wrote:
> Vivek Goyal wrote:
> ...
> >  /*
> >   * Check if new_cfqq should preempt the currently active queue. Return 0 for
> > - * no or if we aren't sure, a 1 will cause a preempt.
> > + * no or if we aren't sure, a 1 will cause a preemption attempt.
> > + * Some of the preemption logic has been moved to common layer. Only cfq
> > + * specific parts are left here.
> >   */
> >  static int
> > -cfq_should_preempt(struct cfq_data *cfqd, struct cfq_queue *new_cfqq,
> > -		   struct request *rq)
> > +cfq_should_preempt(struct request_queue *q, void *new_cfqq, struct request *rq)
> >  {
> > -	struct cfq_queue *cfqq;
> > +	struct cfq_data *cfqd = q->elevator->elevator_data;
> > +	struct cfq_queue *cfqq = elv_active_sched_queue(q->elevator);
> >  
> > -	cfqq = cfqd->active_queue;
> >  	if (!cfqq)
> >  		return 0;
> >  
> > -	if (cfq_slice_used(cfqq))
> > +	if (elv_ioq_slice_used(cfqq->ioq))
> >  		return 1;
> >  
> >  	if (cfq_class_idle(new_cfqq))
> > @@ -2018,13 +1661,7 @@ cfq_should_preempt(struct cfq_data *cfqd, struct cfq_queue *new_cfqq,
> >  	if (rq_is_meta(rq) && !cfqq->meta_pending)
> >  		return 1;
> >  
> > -	/*
> > -	 * Allow an RT request to pre-empt an ongoing non-RT cfqq timeslice.
> > -	 */
> > -	if (cfq_class_rt(new_cfqq) && !cfq_class_rt(cfqq))
> > -		return 1;
> > -
> > -	if (!cfqd->active_cic || !cfq_cfqq_wait_request(cfqq))
> > +	if (!cfqd->active_cic || !elv_ioq_wait_request(cfqq->ioq))
> >  		return 0;
> >  
> >  	/*
> 
> Hi Vivek,
> 
> cfq_should_preempt() will do the check "if (cfq_rq_close(cfqd, rq)) to see whether
> it should preempt the current cfqq. From fairness point of view,  should we also 
> check "fairness" value, if it's set fairness == 1, don't allow to preempt the current
> cfqq?

Hi Gui,

In V7, fairness=1 means that we try to dispatch request only from one
queue at a time and wait for requests to finish from that queue before
next queue is scheduled in. This helps in better disk time accounting for the
queue.

But currently this is not true for preemption path. So if we decide to
preempt the current queue (either by elevator layer or by cfq), we expire
the queue immediately and bring in the new one. So this is just not
cfq_rq_close() but the whole preemption path.

Currently I will leave it as it is but if we run into significant issues, 
then we can fix it. It will require extra logic of keeping track that
current queue has been preempted. Also keep track who preempted etc and
as soon as last request from queue completes, expire it.

Thanks
Vivek

  parent reply	other threads:[~2009-07-31 13:19 UTC|newest]

Thread overview: 120+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-07-24 20:27 [RFC] IO scheduler based IO controller V7 Vivek Goyal
2009-07-24 20:27 ` Vivek Goyal
2009-07-24 20:27 ` [PATCH 01/24] io-controller: Documentation Vivek Goyal
2009-07-24 20:27   ` Vivek Goyal
2009-07-24 20:27 ` [PATCH 02/24] io-controller: Core of the B-WF2Q+ scheduler Vivek Goyal
2009-07-24 20:27   ` Vivek Goyal
2009-07-24 20:27 ` [PATCH 03/24] io-controller: bfq support of in-class preemption Vivek Goyal
2009-07-24 20:27   ` Vivek Goyal
2009-07-27 16:54   ` Jerome Marchand
     [not found]     ` <4A6DDBDE.8020608-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2009-07-27 22:41       ` Vivek Goyal
2009-07-27 22:41     ` Vivek Goyal
2009-07-27 22:41       ` Vivek Goyal
     [not found]       ` <20090727224138.GA3702-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2009-07-28 11:44         ` Jerome Marchand
2009-07-28 11:44           ` Jerome Marchand
2009-07-28 13:52           ` Vivek Goyal
2009-07-28 13:52             ` Vivek Goyal
2009-07-28 14:29             ` Jerome Marchand
2009-07-28 15:03               ` Vivek Goyal
2009-07-28 15:03                 ` Vivek Goyal
2009-07-28 15:37                 ` Jerome Marchand
     [not found]                   ` <4A6F1B4F.6080709-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2009-07-28 18:45                     ` Vivek Goyal
2009-07-28 18:45                   ` Vivek Goyal
2009-07-28 18:45                     ` Vivek Goyal
     [not found]                 ` <20090728150310.GA3870-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2009-07-28 15:37                   ` Jerome Marchand
     [not found]               ` <4A6F0B32.7060801-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2009-07-28 15:03                 ` Vivek Goyal
     [not found]             ` <20090728135212.GC6133-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2009-07-28 14:29               ` Jerome Marchand
     [not found]           ` <4A6EE4A0.6080700-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2009-07-28 13:52             ` Vivek Goyal
     [not found]   ` <1248467274-32073-4-git-send-email-vgoyal-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2009-07-27 16:54     ` Jerome Marchand
2009-07-24 20:27 ` [PATCH 04/24] io-controller: Common flat fair queuing code in elevaotor layer Vivek Goyal
2009-07-24 20:27   ` Vivek Goyal
2009-07-24 20:27 ` [PATCH 05/24] io-controller: Modify cfq to make use of flat elevator fair queuing Vivek Goyal
2009-07-24 20:27   ` Vivek Goyal
     [not found]   ` <1248467274-32073-6-git-send-email-vgoyal-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2009-07-30  6:30     ` Gui Jianfeng
2009-07-30 15:42     ` Jerome Marchand
2009-07-30  6:30   ` Gui Jianfeng
     [not found]     ` <4A713E10.2030204-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
2009-07-31 13:18       ` Vivek Goyal
2009-07-31 13:18     ` Vivek Goyal [this message]
2009-07-31 13:18       ` Vivek Goyal
2009-07-30 15:42   ` Jerome Marchand
     [not found]     ` <4A71BF76.6040709-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2009-07-30 18:30       ` Vivek Goyal
2009-07-30 18:30     ` Vivek Goyal
2009-07-30 18:30       ` Vivek Goyal
2009-07-24 20:27 ` [PATCH 06/24] io-controller: core bfq scheduler changes for hierarchical setup Vivek Goyal
2009-07-24 20:27   ` Vivek Goyal
2009-07-24 20:27 ` [PATCH 07/24] io-controller: cgroup related changes for hierarchical group support Vivek Goyal
2009-07-24 20:27   ` Vivek Goyal
2009-07-24 20:27 ` [PATCH 08/24] io-controller: Common hierarchical fair queuing code in elevaotor layer Vivek Goyal
2009-07-24 20:27   ` Vivek Goyal
2009-07-24 20:27 ` [PATCH 09/24] io-controller: cfq changes to use " Vivek Goyal
2009-07-24 20:27   ` Vivek Goyal
2009-07-24 20:27 ` [PATCH 11/24] io-controller: Debug hierarchical IO scheduling Vivek Goyal
2009-07-24 20:27   ` Vivek Goyal
2009-07-24 20:27 ` [PATCH 12/24] io-controller: Introduce group idling Vivek Goyal
2009-07-24 20:27   ` Vivek Goyal
2009-07-24 20:27 ` [PATCH 13/24] io-controller: Wait for requests to complete from last queue before new queue is scheduled Vivek Goyal
2009-07-24 20:27   ` Vivek Goyal
2009-07-24 20:27 ` [PATCH 14/24] io-controller: Separate out queue and data Vivek Goyal
2009-07-24 20:27   ` Vivek Goyal
2009-07-24 20:27 ` [PATCH 15/24] io-conroller: Prepare elevator layer for single queue schedulers Vivek Goyal
2009-07-24 20:27   ` Vivek Goyal
2009-07-24 20:27 ` [PATCH 16/24] io-controller: noop changes for hierarchical fair queuing Vivek Goyal
2009-07-24 20:27   ` Vivek Goyal
2009-07-24 20:27 ` [PATCH 17/24] io-controller: deadline " Vivek Goyal
2009-07-24 20:27   ` Vivek Goyal
2009-07-24 20:27 ` [PATCH 18/24] io-controller: anticipatory " Vivek Goyal
2009-07-24 20:27   ` Vivek Goyal
     [not found] ` <1248467274-32073-1-git-send-email-vgoyal-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2009-07-24 20:27   ` [PATCH 01/24] io-controller: Documentation Vivek Goyal
2009-07-24 20:27   ` [PATCH 02/24] io-controller: Core of the B-WF2Q+ scheduler Vivek Goyal
2009-07-24 20:27   ` [PATCH 03/24] io-controller: bfq support of in-class preemption Vivek Goyal
2009-07-24 20:27   ` [PATCH 04/24] io-controller: Common flat fair queuing code in elevaotor layer Vivek Goyal
2009-07-24 20:27   ` [PATCH 05/24] io-controller: Modify cfq to make use of flat elevator fair queuing Vivek Goyal
2009-07-24 20:27   ` [PATCH 06/24] io-controller: core bfq scheduler changes for hierarchical setup Vivek Goyal
2009-07-24 20:27   ` [PATCH 07/24] io-controller: cgroup related changes for hierarchical group support Vivek Goyal
2009-07-24 20:27   ` [PATCH 08/24] io-controller: Common hierarchical fair queuing code in elevaotor layer Vivek Goyal
2009-07-24 20:27   ` [PATCH 09/24] io-controller: cfq changes to use " Vivek Goyal
2009-07-24 20:27   ` [PATCH 10/24] io-controller: Export disk time used and nr sectors dipatched through cgroups Vivek Goyal
2009-07-24 20:27     ` Vivek Goyal
2009-07-24 20:27   ` [PATCH 11/24] io-controller: Debug hierarchical IO scheduling Vivek Goyal
2009-07-24 20:27   ` [PATCH 12/24] io-controller: Introduce group idling Vivek Goyal
2009-07-24 20:27   ` [PATCH 13/24] io-controller: Wait for requests to complete from last queue before new queue is scheduled Vivek Goyal
2009-07-24 20:27   ` [PATCH 14/24] io-controller: Separate out queue and data Vivek Goyal
2009-07-24 20:27   ` [PATCH 15/24] io-conroller: Prepare elevator layer for single queue schedulers Vivek Goyal
2009-07-24 20:27   ` [PATCH 16/24] io-controller: noop changes for hierarchical fair queuing Vivek Goyal
2009-07-24 20:27   ` [PATCH 17/24] io-controller: deadline " Vivek Goyal
2009-07-24 20:27   ` [PATCH 18/24] io-controller: anticipatory " Vivek Goyal
2009-07-24 20:27   ` [PATCH 19/24] blkio_cgroup patches from Ryo to track async bios Vivek Goyal
2009-07-24 20:27   ` [PATCH 20/24] io-controller: map async requests to appropriate cgroup Vivek Goyal
2009-07-24 20:27   ` [PATCH 21/24] io-controller: Per cgroup request descriptor support Vivek Goyal
2009-07-24 20:27   ` [PATCH 22/24] io-controller: Per io group bdi congestion interface Vivek Goyal
2009-07-24 20:27   ` [PATCH 23/24] io-controller: Support per cgroup per device weights and io class Vivek Goyal
2009-07-24 20:27   ` [PATCH 24/24] map sync requests to group using bio tracking info Vivek Goyal
2009-07-31  5:21   ` [RFC] IO scheduler based IO controller V7 Gui Jianfeng
2009-07-24 20:27 ` [PATCH 19/24] blkio_cgroup patches from Ryo to track async bios Vivek Goyal
2009-07-24 20:27   ` Vivek Goyal
2009-07-24 20:27 ` [PATCH 20/24] io-controller: map async requests to appropriate cgroup Vivek Goyal
2009-07-24 20:27   ` Vivek Goyal
2009-07-24 20:27 ` [PATCH 21/24] io-controller: Per cgroup request descriptor support Vivek Goyal
2009-07-24 20:27   ` Vivek Goyal
2009-07-24 20:27 ` [PATCH 22/24] io-controller: Per io group bdi congestion interface Vivek Goyal
2009-07-24 20:27   ` Vivek Goyal
2009-08-08  8:14   ` Gui Jianfeng
     [not found]   ` <1248467274-32073-23-git-send-email-vgoyal-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2009-08-08  8:14     ` Gui Jianfeng
2009-07-24 20:27 ` [PATCH 23/24] io-controller: Support per cgroup per device weights and io class Vivek Goyal
2009-07-24 20:27   ` Vivek Goyal
2009-07-24 20:27 ` [PATCH 24/24] map sync requests to group using bio tracking info Vivek Goyal
2009-07-24 20:27   ` Vivek Goyal
2009-07-31  5:21 ` [RFC] IO scheduler based IO controller V7 Gui Jianfeng
2009-07-31  5:21   ` Gui Jianfeng
     [not found]   ` <4A727F6F.9010005-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
2009-07-31 13:13     ` Vivek Goyal
2009-07-31 13:13   ` Vivek Goyal
2009-07-31 13:13     ` Vivek Goyal
2009-08-03  0:40     ` Gui Jianfeng
2009-08-04  0:48     ` Gui Jianfeng
     [not found]       ` <4A778540.5050502-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
2009-08-04  1:30         ` Vivek Goyal
2009-08-04  1:30       ` Vivek Goyal
2009-08-04  1:30         ` Vivek Goyal
2009-08-18  0:42         ` Gui Jianfeng
     [not found]         ` <20090804013001.GB2282-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2009-08-18  0:42           ` Gui Jianfeng
     [not found]     ` <20090731131359.GA3668-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2009-08-03  0:40       ` Gui Jianfeng
2009-08-04  0:48       ` Gui Jianfeng

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090731131813.GB3668@redhat.com \
    --to=vgoyal@redhat.com \
    --cc=agk@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=balbir@linux.vnet.ibm.com \
    --cc=containers@lists.linux-foundation.org \
    --cc=dhaval@linux.vnet.ibm.com \
    --cc=dm-devel@redhat.com \
    --cc=dpshah@google.com \
    --cc=fchecconi@gmail.com \
    --cc=fernando@oss.ntt.co.jp \
    --cc=guijianfeng@cn.fujitsu.com \
    --cc=jens.axboe@oracle.com \
    --cc=jmoyer@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lizf@cn.fujitsu.com \
    --cc=m-ikeda@ds.jp.nec.com \
    --cc=mikew@google.com \
    --cc=nauman@google.com \
    --cc=paolo.valente@unimore.it \
    --cc=peterz@infradead.org \
    --cc=righi.andrea@gmail.com \
    --cc=ryov@valinux.co.jp \
    --cc=s-uchida@ap.jp.nec.com \
    --cc=taka@valinux.co.jp \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.