lkml.org 
[lkml]   [2007]   [Jun]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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/

\
 
 \ /
  Last update: 2007-06-14 14:03    [W:2.064 / U:1.892 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site