lkml.org 
[lkml]   [2009]   [Nov]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH 02/20] blkio: Change CFQ to use CFS like queue time stamps
Date
Vivek Goyal <vgoyal@redhat.com> writes:

> o Currently CFQ provides priority scaled time slices to processes. If a process
> does not use the time slice, either because process did not have sufficient
> IO to do or because think time of process is large and CFQ decided to disable
> idling, then processes looses it time slice share.
^^^^^^
loses

> o One possible way to handle this is implement CFS like time stamping of the
> cfq queues and keep track of vtime. Next queue for execution will be selected
> based on the one who got lowest vtime. This patch implemented time stamping
> mechanism of cfq queues based on disk time used.
>
> o min_vdisktime represents the minimum vdisktime of the queue, either being
^^^^^
> serviced or leftmost element on the serviec tree.

queue or service tree? The latter seems to make more sense to me.

> +static inline u64
> +cfq_delta_fair(unsigned long delta, struct cfq_queue *cfqq)
> +{
> + const int base_slice = cfqq->cfqd->cfq_slice[cfq_cfqq_sync(cfqq)];
> +
> + return delta + (base_slice/CFQ_SLICE_SCALE * (cfqq->ioprio - 4));
> +}

cfq_scale_delta might be a better name.


> +static inline u64 max_vdisktime(u64 min_vdisktime, u64 vdisktime)
> +{
> + s64 delta = (s64)(vdisktime - min_vdisktime);
> + if (delta > 0)
> + min_vdisktime = vdisktime;
> +
> + return min_vdisktime;
> +}
> +
> +static inline u64 min_vdisktime(u64 min_vdisktime, u64 vdisktime)
> +{
> + s64 delta = (s64)(vdisktime - min_vdisktime);
> + if (delta < 0)
> + min_vdisktime = vdisktime;
> +
> + return min_vdisktime;
> +}

Is there a reason you've reimplemented min and max?

> + /*
> + * Maintain a cache of leftmost tree entries (it is frequently
> + * used)
> + */

You make it sound like there is a cache of more than one entry. Please
fix the comment.

> +static void cfqq_served(struct cfq_queue *cfqq, unsigned long served)
> +{
> + /*
> + * We don't want to charge more than allocated slice otherwise this
> + * queue can miss one dispatch round doubling max latencies. On the
> + * other hand we don't want to charge less than allocated slice as
> + * we stick to CFQ theme of queue loosing its share if it does not
^^^^^^^
losing


> +/*
> + * Handles three operations.
> + * Addition of a new queue to service tree, when a new request comes in.
> + * Resorting of an expiring queue (used after slice expired)
> + * Requeuing a queue at the front (used during preemption).
> + */
> +static void cfq_service_tree_add(struct cfq_data *cfqd, struct cfq_queue *cfqq,
> + bool add_front, unsigned long service)

service? Can we come up with a better name that actually hints at what
this is? service_time, maybe?


Mostly this looks pretty good and is fairly easy to read.

Cheers,
Jeff


\
 
 \ /
  Last update: 2009-11-04 15:33    [W:0.276 / U:0.056 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site