Messages in this thread | | | Date | Mon, 14 Feb 2011 11:41:39 -0500 | From | Vivek Goyal <> | Subject | Re: [PATCH 2] Avoid preferential treatment of groups that aren't backlogged |
| |
On Fri, Feb 11, 2011 at 11:39:30AM -0800, Chad Talbott wrote:
[..] > static struct cfq_group *cfq_get_next_cfqg(struct cfq_data *cfqd); > @@ -873,18 +877,13 @@ cfq_group_service_tree_add(struct cfq_data *cfqd, struct cfq_group *cfqg) > if (!RB_EMPTY_NODE(&cfqg->rb_node)) > return; > > - /* > - * Currently put the group at the end. Later implement something > - * so that groups get lesser vtime based on their weights, so that > - * if group does not loose all if it was not continously backlogged. > - */ > - n = rb_last(&st->rb); > - if (n) { > - __cfqg = rb_entry_cfqg(n); > - cfqg->vdisktime = __cfqg->vdisktime + CFQ_IDLE_DELAY; > - } else > + if (cfqd->active_generation > cfqg->generation_num) > cfqg->vdisktime = st->min_vdisktime; > - > + else > + /* We assume that vdisktime was not modified when the task > + was off the service tree. > + */ > + cfqg->vdisktime = max(st->min_vdisktime, cfqg->vdisktime);
I think above usage of max() also will give wrong results upon wrapping. So how about using max_vdisktime() instead? I know that results are not catastrophic when it happens, still lets use the right thing.
Thanks Vivek
| |