lkml.org 
[lkml]   [2009]   [Nov]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH 03/21] blkio: Implement macro to traverse each idle tree in group
    On Tue, Dec 01, 2009 at 01:43:16AM +0530, Divyesh Shah wrote:
    > On Mon, Nov 30, 2009 at 8:29 AM, Vivek Goyal <vgoyal@redhat.com> wrote:
    > > o Implement a macro to traverse each service tree in the group. This avoids
    > >  usage of double for loop and special condition for idle tree 4 times.
    > >
    > > o Macro is little twisted because of special handling of idle class service
    > >  tree.
    > >
    > > Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
    > > ---
    > >  block/cfq-iosched.c |   35 +++++++++++++++++++++--------------
    > >  1 files changed, 21 insertions(+), 14 deletions(-)
    > >
    > > diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
    > > index 3baa3f4..c73ff44 100644
    > > --- a/block/cfq-iosched.c
    > > +++ b/block/cfq-iosched.c
    > > @@ -303,6 +303,15 @@ CFQ_CFQQ_FNS(deep);
    > >  #define cfq_log(cfqd, fmt, args...)    \
    > >        blk_add_trace_msg((cfqd)->queue, "cfq " fmt, ##args)
    > >
    > > +/* Traverses through cfq group service trees */
    > > +#define for_each_cfqg_st(cfqg, i, j, st) \
    > > +       for (i = 0; i < 3; i++) \
    > > +               for (j = 0, st = i < 2 ? &cfqg->service_trees[i][j] : \
    > > +                       &cfqg->service_tree_idle; \
    > > +                       (i < 2 && j < 3) || (i == 2 && j < 1); \
    > > +                       j++, st = i < 2 ? &cfqg->service_trees[i][j]: NULL) \
    >
    > Can this be simplified a bit by moving the service tree assignments
    > out of the for statement?

    Not sure how that can be done. One way is that st assignment happens in
    the body of the for loop. That means I shall have to open braces for for
    loop in the macro and user of the macro will close the braces. That will
    look really ugly.

    for_each_cfqg_st()
    }

    Do you have other ideas.

    > Also is it possible to use macros for the various service classes instead of
    > 1, 2, 3?
    >

    Please find attached the new version of patch. I have used macro names for
    various classses instead of numbers. Hopefully this one is little better
    to read.


    o Implement a macro to traverse each service tree in the group. This avoids
    usage of double for loop and special condition for idle tree 4 times.

    o Macro is little twisted because of special handling of idle class service
    tree.

    Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
    ---
    block/cfq-iosched.c | 41 +++++++++++++++++++++++++----------------
    1 file changed, 25 insertions(+), 16 deletions(-)

    Index: linux10/block/cfq-iosched.c
    ===================================================================
    --- linux10.orig/block/cfq-iosched.c 2009-11-30 17:20:42.000000000 -0500
    +++ linux10/block/cfq-iosched.c 2009-11-30 17:22:45.000000000 -0500
    @@ -140,9 +140,9 @@ struct cfq_queue {
    * IDLE is handled separately, so it has negative index
    */
    enum wl_prio_t {
    - IDLE_WORKLOAD = -1,
    BE_WORKLOAD = 0,
    - RT_WORKLOAD = 1
    + RT_WORKLOAD = 1,
    + IDLE_WORKLOAD = 2,
    };

    /*
    @@ -303,6 +303,17 @@ CFQ_CFQQ_FNS(deep);
    #define cfq_log(cfqd, fmt, args...) \
    blk_add_trace_msg((cfqd)->queue, "cfq " fmt, ##args)

    +/* Traverses through cfq group service trees */
    +#define for_each_cfqg_st(cfqg, i, j, st) \
    + for (i = 0; i <= IDLE_WORKLOAD; i++) \
    + for (j = 0, st = i < IDLE_WORKLOAD ? &cfqg->service_trees[i][j]\
    + : &cfqg->service_tree_idle; \
    + (i < IDLE_WORKLOAD && j <= SYNC_WORKLOAD) || \
    + (i == IDLE_WORKLOAD && j == 0); \
    + j++, st = i < IDLE_WORKLOAD ? \
    + &cfqg->service_trees[i][j]: NULL) \
    +
    +
    static inline enum wl_prio_t cfqq_prio(struct cfq_queue *cfqq)
    {
    if (cfq_class_idle(cfqq))
    @@ -565,6 +576,10 @@ cfq_choose_req(struct cfq_data *cfqd, st
    */
    static struct cfq_queue *cfq_rb_first(struct cfq_rb_root *root)
    {
    + /* Service tree is empty */
    + if (!root->count)
    + return NULL;
    +
    if (!root->left)
    root->left = rb_first(&root->rb);

    @@ -1592,18 +1607,14 @@ static int cfq_forced_dispatch(struct cf
    int dispatched = 0;
    int i, j;
    struct cfq_group *cfqg = &cfqd->root_group;
    + struct cfq_rb_root *st;

    - for (i = 0; i < 2; ++i)
    - for (j = 0; j < 3; ++j)
    - while ((cfqq = cfq_rb_first(&cfqg->service_trees[i][j]))
    - != NULL)
    - dispatched += __cfq_forced_dispatch_cfqq(cfqq);
    -
    - while ((cfqq = cfq_rb_first(&cfqg->service_tree_idle)) != NULL)
    - dispatched += __cfq_forced_dispatch_cfqq(cfqq);
    + for_each_cfqg_st(cfqg, i, j, st) {
    + while ((cfqq = cfq_rb_first(st)) != NULL)
    + dispatched += __cfq_forced_dispatch_cfqq(cfqq);
    + }

    cfq_slice_expired(cfqd, 0);
    -
    BUG_ON(cfqd->busy_queues);

    cfq_log(cfqd, "forced_dispatch=%d", dispatched);
    @@ -2974,6 +2985,7 @@ static void *cfq_init_queue(struct reque
    struct cfq_data *cfqd;
    int i, j;
    struct cfq_group *cfqg;
    + struct cfq_rb_root *st;

    cfqd = kmalloc_node(sizeof(*cfqd), GFP_KERNEL | __GFP_ZERO, q->node);
    if (!cfqd)
    @@ -2981,11 +2993,8 @@ static void *cfq_init_queue(struct reque

    /* Init root group */
    cfqg = &cfqd->root_group;
    -
    - for (i = 0; i < 2; ++i)
    - for (j = 0; j < 3; ++j)
    - cfqg->service_trees[i][j] = CFQ_RB_ROOT;
    - cfqg->service_tree_idle = CFQ_RB_ROOT;
    + for_each_cfqg_st(cfqg, i, j, st)
    + *st = CFQ_RB_ROOT;

    /*
    * Not strictly needed (since RB_ROOT just clears the node and we
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/
    \
     
     \ /
      Last update: 2009-11-30 23:29    [W:0.034 / U:0.048 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site