lkml.org 
[lkml]   [2010]   [Jan]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC]cfq-iosched: quantum check tweak
    On Tue, Jan 12, 2010 at 11:48:20PM +0800, Vivek Goyal wrote:
    > On Tue, Jan 12, 2010 at 11:07:56AM +0800, Shaohua Li wrote:
    >
    > [..]
    > > > > > > > I think this patch breaks the meaning of cfq_quantum? Now we can allow
    > > > > > > > dispatch of more requests from the same queue. I had kind of liked the
    > > > > > > > idea of respecting cfq_quantum. Especially it can help in testing. With
    > > > > > > > this patch cfq_quantum will more or less loose its meaning.
    > > > > > > cfq_quantum will still be enforced at the end of the slice, so its
    > > > > > > meaning of how many requests can be still pending when you finish your
    > > > > > > slice is preserved.
    > > > > >
    > > > > > Not always and it will depend how accurate your approximation of service
    > > > > > time is. If per request completion time is more than approximation (in
    > > > > > this case slice_idle), than you will end up with more requests in dispatch
    > > > > > queue from one cfqq at the time of slice expiry.
    > > > > we use slice_idle for a long time and no complain. So assume the approximation
    > > > > of service time is good.
    > > >
    > > > slice_idle is a variable and user can easily change it to 1ms and even 0.
    > > > In that case you will be theoritically be ready to dispatch 100/1 requests
    > > > from the cfqq?
    > > User changing it should know what he does. A less-experienced user can mess a lot
    > > of things, which we don't care.
    > >
    >
    > The point is that there is no obivious co-relation between slice_idle and
    > cfq_quantum. Even an experienced user would not expect that changing
    > slice_idle silently will enable dispatching more requests from each cfqq.
    Agree slice_idle hasn't relationship with cfq_quantum. Yes, there are more requests
    dispatched, but it shouldn't impact user experience. If it does, then the patch fails.

    > > 1 file changed, 21 insertions(+), 3 deletions(-)
    > >
    > > Index: linux-2.6/block/cfq-iosched.c
    > > ===================================================================
    > > --- linux-2.6.orig/block/cfq-iosched.c
    > > +++ linux-2.6/block/cfq-iosched.c
    > > @@ -19,7 +19,7 @@
    > > * tunables
    > > */
    > > /* max queue in one round of service */
    > > -static const int cfq_quantum = 4;
    > > +static const int cfq_quantum = 8;
    > > static const int cfq_fifo_expire[2] = { HZ / 4, HZ / 8 };
    > > /* maximum backwards seek, in KiB */
    > > static const int cfq_back_max = 16 * 1024;
    > > @@ -32,6 +32,8 @@ static int cfq_slice_idle = HZ / 125;
    > > static const int cfq_target_latency = HZ * 3/10; /* 300 ms */
    > > static const int cfq_hist_divisor = 4;
    > >
    > > +#define CFQ_SOFT_QUANTUM (4)
    > > +
    > > /*
    > > * offset from end of service tree
    > > */
    > > @@ -2242,6 +2244,19 @@ static int cfq_forced_dispatch(struct cf
    > > return dispatched;
    > > }
    > >
    > > +static inline bool cfq_slice_used_soon(struct cfq_data *cfqd,
    > > + struct cfq_queue *cfqq)
    > > +{
    > > + /* the queue hasn't finished any request, can't estimate */
    > > + if (cfq_cfqq_slice_new(cfqq) || cfqq->dispatched >= cfqd->cfq_quantum)
    > > + return 1;
    > > + if (time_after(jiffies + cfqd->cfq_slice_idle * cfqq->dispatched,
    > > + cfqq->slice_end))
    > > + return 1;
    > > +
    > > + return 0;
    > > +}
    > > +
    > > static bool cfq_may_dispatch(struct cfq_data *cfqd, struct cfq_queue *cfqq)
    > > {
    > > unsigned int max_dispatch;
    > > @@ -2258,7 +2273,10 @@ static bool cfq_may_dispatch(struct cfq_
    > > if (cfqd->sync_flight && !cfq_cfqq_sync(cfqq))
    > > return false;
    > >
    > > - max_dispatch = cfqd->cfq_quantum;
    > > + max_dispatch = cfqd->cfq_quantum / 2;
    > > + if (max_dispatch < CFQ_SOFT_QUANTUM)
    >
    > We don't have to hardcode CFQ_SOFT_QUANTUM or in fact we don't need it. We can
    > derive the soft limit from hard limit (cfq_quantum). Say soft limit will be
    > 50% of cfq_quantum value.
    I'm hoping this doesn't give user a surprise. Say cfq_quantum sets to 7, then we
    start doing throttling from 3 requests. Adding the CFQ_SOFT_QUANTUM gives a compatibility
    against old behavior at least. Am I over thinking?

    > > + max_dispatch = min_t(unsigned int, CFQ_SOFT_QUANTUM,
    > > + cfqd->cfq_quantum);
    > > if (cfq_class_idle(cfqq))
    > > max_dispatch = 1;
    > >
    > > @@ -2275,7 +2293,7 @@ static bool cfq_may_dispatch(struct cfq_
    > > /*
    > > * We have other queues, don't allow more IO from this one
    > > */
    > > - if (cfqd->busy_queues > 1)
    > > + if (cfqd->busy_queues > 1 && cfq_slice_used_soon(cfqd, cfqq))
    > > return false;
    >
    > So I guess here we can write something as follows.
    >
    > if (cfqd->busy_queues > 1 && cfq_slice_used_soon(cfqd, cfqq))
    > return false;
    >
    > if (cfqd->busy_queues == 1)
    > max_dispatch = -1;
    > else
    > /*
    > * Normally we start throttling cfqq when cfq_quantum/2
    > * requests have been dispatched. But we can drive
    > * deeper queue depths at the beginning of slice
    > * subjected to upper limit of cfq_quantum.
    > */
    > max_dispatch = cfqd->cfq_quantum;
    ok.

    Thanks,
    Shaohua


    \
     
     \ /
      Last update: 2010-01-13 09:19    [from the cache]
    ©2003-2014 Jasper Spaans. hosted at Digital Ocean