Messages in this thread | | | Subject | Re: [PATCH 02/16] blkio: Keep queue on service tree until we expire it | From | "Alan D. Brunelle" <> | Date | Tue, 17 Nov 2009 13:07:40 -0500 |
| |
Hi Vivek -
Some minor nit comments in this and the next three e-mails...
On Fri, 2009-11-13 at 12:40 -0500, Vivek Goyal wrote:
> @@ -1065,17 +1080,43 @@ static inline void cfq_slice_expired(struct cfq_data *cfqd, bool timed_out) > * Get next queue for service. Unless we have a queue preemption, > * we'll simply select the first cfqq in the service tree. > */ > -static struct cfq_queue *cfq_get_next_queue(struct cfq_data *cfqd) > +static struct cfq_queue *__cfq_get_next_queue(struct cfq_data *cfqd) > { > struct cfq_rb_root *service_tree = > service_tree_for(cfqd->serving_group, cfqd->serving_prio, > cfqd->serving_type, cfqd); > > + if (!cfqd->rq_queued) > + return NULL; > + > if (RB_EMPTY_ROOT(&service_tree->rb)) > return NULL; > return cfq_rb_first(service_tree); > } > > +static struct cfq_queue *cfq_get_next_queue(struct cfq_data *cfqd) > +{ > + struct cfq_group *cfqg = &cfqd->root_group; > + struct cfq_queue *cfqq; > + int i, j; > + > + if (!cfqd->rq_queued) > + return NULL; > + > + for (i = 0; i < 2; ++i) { > + for (j = 0; j < 3; ++j) {
Is this double for-loop a good candidate for an iterator macro construct? (I think this is used 6 times in your total patch set.)
> + cfqq = cfq_rb_first(&cfqg->service_trees[i][j]); > + if (cfqq) > + return cfqq; > + } > + } > +
Perhaps just change the 4 lines below with:
return cfq_rb_first(&cfgg->service_tree_idle);
to be consistent (e.g. right above in __cfq_get_next_queue) and for less code clutter?
> + cfqq = cfq_rb_first(&cfqg->service_tree_idle); > + if (cfqq) > + return cfqq; > + return NULL; > +} > +
| |