linux-kernel - Re: [PATCH] sched: fix autogroup nice tune on UP

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <AANLkTinShA64ZVH91Rt-jQuX86gKCaezFn_D_m+820We@mail.gmail.com>
Date:	Sun, 23 Jan 2011 17:16:09 +0200
From:	Pekka Enberg <penberg@...nel.org>
To:	Yong Zhang <yong.zhang0@...il.com>
Cc:	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Michael Witten <mfwitten@...il.com>,
	linux-kernel@...r.kernel.org,
	Christian Kujau <christian@...dbynature.de>,
	Mike Galbraith <efault@....de>, Ingo Molnar <mingo@...e.hu>
Subject: Re: [PATCH] sched: fix autogroup nice tune on UP

On Sun, Jan 23, 2011 at 5:03 PM, Yong Zhang <yong.zhang0@...il.com> wrote:
> And there is another issue on UP when FAIR_GROUP_SCHED
> enabled.
>
> ---
> From: Yong Zhang <yong.zhang0@...il.com>
> Subject: [PATCH] sched: fix autogroup nice tune on UP
>
> When on UP with FAIR_GROUP_SCHED enabled, tune shares
> only affect tg->shares, but is not reflected on
> tg->se->load, the reason is update_cfs_shares()
> do nothing on SMP.

Typo here? You mean UP, right?

> So let sched_group_set_shares() update tg->se->load
> directly for UP && FAIR_GROUP_SCHED.
>
> This issue is found when enable autogroup, but also
> exists on cgroup.cpu on UP.
>
> Signed-off-by: Yong Zhang <yong.zhang0@...il.com>
> Cc: Ingo Molnar <mingo@...e.hu>
> Cc: Peter Zijlstra <peterz@...radead.org>
> Cc: Mike Galbraith <efault@....de>
> ---
>  kernel/sched.c |    8 ++++++--
>  1 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/sched.c b/kernel/sched.c
> index 3ec2c6c..b8ed459 100644
> --- a/kernel/sched.c
> +++ b/kernel/sched.c
> @@ -8483,8 +8483,8 @@ static DEFINE_MUTEX(shares_mutex);
>
>  int sched_group_set_shares(struct task_group *tg, unsigned long shares)
>  {
> -       int i;
> -       unsigned long flags;
> +       int uninitialized_var(i);
> +       unsigned long uninitialized_var(flags);
>
>        /*
>         * We can't change the weight of the root cgroup.
> @@ -8502,6 +8502,7 @@ int sched_group_set_shares(struct task_group *tg, unsigned long shares)
>                goto done;
>
>        tg->shares = shares;
> +#ifdef CONFIG_SMP
>        for_each_possible_cpu(i) {
>                struct rq *rq = cpu_rq(i);
>                struct sched_entity *se;
> @@ -8513,6 +8514,9 @@ int sched_group_set_shares(struct task_group *tg, unsigned long shares)
>                        update_cfs_shares(group_cfs_rq(se), 0);
>                raw_spin_unlock_irqrestore(&rq->lock, flags);
>        }
> +#else
> +       update_load_set(&tg->se[0]->load, shares);
> +#endif

I'm not an expert on the scheduler but this looks like the wrong thing
to do. Shouldn't you fix update_cfs_shares() to work on UP properly?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/