Re: [PATCH 1/2] autonuma: fix typo in sched balance

From: Andrea Arcangeli
Date: Mon Mar 19 2012 - 13:09:34 EST


Hi Hillf,

Thanks a lot for starting looking into it despite I hadn't the time
yet to polish it in smaller commits.

On Mon, Mar 19, 2012 at 08:45:03PM +0800, Hillf Danton wrote:
> Hi Andrea
>
> It is redefined to be safely used by set/test_bit(cpu, mm_mask).
>
> Q-0: another case of mm_mask in struct rq?

You're right, we better move it to struct rq like I moved the other
arrays. I already moved those to rq, but one recent patch slipped off.

> Q-1: make sense to move NR_CPUS variable out of stack?

Absolutely. It should be ok in the latest autonuma branch (since Friday)

http://git.kernel.org/?p=linux/kernel/git/andrea/aa.git;a=commitdiff;h=c9effed1071202090cb9e09da814dabf1b9af409;hp=30ed50adf6cfe85f7feb12c4279359ec52f5f2cd

I committed it to aa.git last Friday, can you pull out again the
autonuma branch and verify?

> Q-2: for online mem node, cpumask_of_node(nid) implies cpu also online?
>
> for_each_online_node(nid) {
> for_each_cpu_and(cpu, cpumask_of_node(nid), allowed) {
> struct rq *rq;
> + if (!cpu_online(cpu))
> + continue;
> rq = cpu_rq(cpu);
> if (rq->curr->mm == mm)
> nr_mm++;
> }
> }
>
> where check needed for accessing rq->curr?

I need to check what is in rq->curr of a cpu that isn't online, idle
task maybe? At times cpu_online may be needed.


>
>
> Signed-off-by: Hillf Danton <dhillf@xxxxxxxxx>
> ---
>
> --- a/kernel/sched/numa.c Sat Mar 17 11:01:40 2012
> +++ b/kernel/sched/numa.c Mon Mar 19 20:01:34 2012
> @@ -95,7 +95,7 @@ void sched_autonuma_balance(void)
> struct migration_arg arg;
> struct task_struct *p = current;
> struct sched_autonuma *sched_autonuma = p->sched_autonuma;
> - DECLARE_BITMAP(mm_mask, MAX_NUMNODES);
> + DECLARE_BITMAP(mm_mask, NR_CPUS);
>
> if (!sched_autonuma || sched_autonuma->autonuma_stop_one_cpu || !p->mm)
> return;
> @@ -136,7 +136,7 @@ void sched_autonuma_balance(void)
> weight_current[nid] = p_w*AUTONUMA_BALANCE_SCALE/p_t;
> }
>
> - bitmap_zero(mm_mask, MAX_NUMNODES);
> + bitmap_zero(mm_mask, NR_CPUS);
> for_each_online_node(nid) {
> if (nid == cpu_nid)
> continue;

Ouch thanks a lot. I need to rediff against
c9effed1071202090cb9e09da814dabf1b9af409 , it's now in the rq.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/