Messages in this thread | | | Date | Thu, 14 Jun 2007 17:36:05 +0530 | From | Srivatsa Vaddagiri <> | Subject | Re: [RFC][PATCH 5/6] core changes for group fairness |
| |
On Wed, Jun 13, 2007 at 10:56:06PM +0200, Dmitry Adamushko wrote: > >+static int balance_tasks(struct rq *this_rq, int this_cpu, struct rq > >*busiest, > >+ unsigned long max_nr_move, unsigned long > >max_load_move, > >+ struct sched_domain *sd, enum idle_type idle, > >+ int *all_pinned, unsigned long *load_moved, > >+ int this_best_prio, int best_prio, int > >best_prio_seen, > >+ void *iterator_arg, > >+ struct task_struct *(*iterator_start)(void *arg), > >+ struct task_struct *(*iterator_next)(void *arg)); > > IMHO, it looks a bit frightening :)
I agree :) It is taking (ooops) 15 args (8 perhaps was the previous record in sched.c (move_tasks)!
> maybe it would be possible to > create a structure that combines some relevant argumens .. at least, > the last 3 ones.
How does this look?
struct balance_tasks_args { struct rq *this_rq, struct rq *busiest; unsigned long max_nr_move, unsigned long max_load_move; struct sched_domain *sd, enum idle_type idle; int this_best_prio, best_prio, best_prio_seen; int *all_pinned; unsigned long *load_moved; void *iterator_arg; struct task_struct *(*iterator_start)(void *arg); struct task_struct *(*iterator_next)(void *arg)); };
static int balance_tasks(struct balance_tasks_args *arg);
[ down to one argument now! ]
?
I will try this in my next iteration ..
> >-static int move_tasks(struct rq *this_rq, int this_cpu, struct rq > >*busiest, > >+static int balance_tasks(struct rq *this_rq, int this_cpu, struct rq > >*busiest, > > unsigned long max_nr_move, unsigned long > > max_load_move, > > struct sched_domain *sd, enum idle_type idle, > >- int *all_pinned) > >+ int *all_pinned, unsigned long *load_moved, > >+ int this_best_prio, int best_prio, int > >best_prio_seen, > >+ void *iterator_arg, > >+ struct task_struct *(*iterator_start)(void *arg), > >+ struct task_struct *(*iterator_next)(void *arg)) > > I think, there is a possible problem here. If I'm not complete wrong, > this function (move_tasks() in the current mainline) can move more > 'load' than specified by the 'max_load_move'..
Yes I think you are right. I will tackle this in next iteration.
Thanks for all your review so far!
-- Regards, vatsa - 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/
| |