Messages in this thread | | | Subject | Re: [patch 2/3]cfq-iosched: schedule dispatch for noidle queue | From | Shaohua Li <> | Date | Tue, 09 Nov 2010 09:31:07 +0800 |
| |
On Mon, 2010-11-08 at 22:28 +0800, Vivek Goyal wrote: > On Mon, Nov 08, 2010 at 10:07:18AM +0800, Shaohua Li wrote: > > A queue is idle at cfq_dispatch_requests(), but it gets noidle later. Unless > > other task explictly does unplug or all requests are drained, we will not > > deliever requests to the disk even cfq_arm_slice_timer doesn't make the > > queue idle. For example, cfq_should_idle() returns true because of > > service_tree->count == 1, and then other queues are added. Note, I didn't > > see obvious performance impacts so far with the patch, but just thought > > this could be a problem. > > > > Signed-off-by: Shaohua Li <shaohua.li@intel.com> > > > > --- > > block/cfq-iosched.c | 23 ++++++++++++++++++++++- > > 1 file changed, 22 insertions(+), 1 deletion(-) > > > > Index: linux/block/cfq-iosched.c > > =================================================================== > > --- linux.orig/block/cfq-iosched.c 2010-11-08 08:41:20.000000000 +0800 > > +++ linux/block/cfq-iosched.c 2010-11-08 08:43:51.000000000 +0800 > > @@ -3265,6 +3265,10 @@ cfq_should_preempt(struct cfq_data *cfqd > > if (cfq_class_rt(new_cfqq) && !cfq_class_rt(cfqq)) > > return true; > > > > + /* An idle queue should not be idle now for some reason */ > > + if (RB_EMPTY_ROOT(&cfqq->sort_list) && !cfq_should_idle(cfqd, cfqq)) > > + return true; > > + > > if (!cfqd->active_cic || !cfq_cfqq_wait_request(cfqq)) > > return false; > > > > @@ -3508,8 +3512,25 @@ static void cfq_completed_request(struct > > } > > } > > > > - if (!cfqd->rq_in_driver) > > + if (!cfqd->rq_in_driver) { > > cfq_schedule_dispatch(cfqd); > > + return; > > + } > > + /* > > + * A queue is idle at cfq_dispatch_requests(), but it gets noidle > > + * later. We schedule a dispatch if the queue has no requests, > > + * otherwise the disk is actually in idle till all requests > > + * are finished even cfq_arm_slice_timer doesn't make the queue idle > > + * */ > > Why do we have to wait for all requests to finish in device? Will driver > most likely not ask for next request when 1-2 requests have completed > and at that time we should expire the queue if queue is no more marked > as "noidle"? The issue is a queue is idle just because it's the last queue of the service tree. Then a new queue is added and the idled queue should not idle now. we should preempt the idled queue soon. does this make sense to you?
Thanks, Shaohua
| |