Messages in this thread | | | From | Jeff Moyer <> | Subject | Re: [PATCH] cfq-iosched: Wait for next request if we are about to expire the queue | Date | Tue, 08 Dec 2009 12:52:48 -0500 |
| |
Vivek Goyal <vgoyal@redhat.com> writes:
> If there is a sequential reader running in a group, we wait for next request > to come in that group after slice expiry and once new request is in, we expire > the queue. Otherwise we delete the group from service tree and group looses > its fair share. > > So far I was marking a queue as wait_busy if it had consumed its slice and > it was last queue in the group. But this condition did not cover following > two cases. > > - If think time of process is more than slice left, we will not arm the timer > and queue will be expired. > > - If we hit the boundary condition where slice has not expired after request > completion but almost immediately after (4-5 ns), select_queue() hits and > by that time slice has expired because jiffies has incremented by one. > > Gui was hitting the boundary condition and not getting fairness numbers > proportional to weight. > > This patch puts the checks for above two conditions and improves the fairness > numbers for sequential workload on rotational media. > > Reported-by: Gui Jianfeng <guijianfeng@cn.fujitsu.com> > Signed-off-by: Vivek Goyal <vgoyal@redhat.com> > Acked-by: Gui Jianfeng <guijiafneng@cn.fujitsu.com> > --- > block/cfq-iosched.c | 30 ++++++++++++++++++++++++++---- > 1 file changed, 26 insertions(+), 4 deletions(-) > > Index: linux1/block/cfq-iosched.c > =================================================================== > --- linux1.orig/block/cfq-iosched.c 2009-12-07 14:40:03.000000000 -0500 > +++ linux1/block/cfq-iosched.c 2009-12-07 14:40:04.000000000 -0500 > @@ -3251,6 +3251,29 @@ static void cfq_update_hw_tag(struct cfq > cfqd->hw_tag = 0; > } > > +static inline bool > +cfq_should_wait_busy(struct cfq_data *cfqd, struct cfq_queue *cfqq) > +{ > + struct cfq_io_context *cic = cfqd->active_cic; > + > + /* If there are other queues in the group, don't wait */ > + if (cfqq->cfqg->nr_cfqq > 1) > + return false; > + > + if (cfq_slice_used(cfqq)) > + return true; > + > + if (cfqq->slice_end - jiffies == 1) > + return true;
This really looks like a hack. At the very least, it requires a big comment above it explaining why it's there. Would it be possible to detect such a condition in select_queue and handle it there?
Cheers, Jeff
| |