linux-kernel - Re: [PATCH v6 2/4] sched/fair: Check a task has a fitting cpu when updating misfit

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: <CAKfTPtBLUkZ0hEd8K=e9wjg+zn9N5jgia-7wwLa3jaeYK+qkCw@mail.gmail.com>
Date: Thu, 7 Mar 2024 10:14:19 +0100
From: Vincent Guittot <vincent.guittot@...aro.org>
To: Qais Yousef <qyousef@...alina.io>
Cc: Dietmar Eggemann <dietmar.eggemann@....com>, Ingo Molnar <mingo@...nel.org>, 
	Peter Zijlstra <peterz@...radead.org>, linux-kernel@...r.kernel.org, 
	Pierre Gondois <Pierre.Gondois@....com>
Subject: Re: [PATCH v6 2/4] sched/fair: Check a task has a fitting cpu when
 updating misfit

On Wed, 6 Mar 2024 at 22:47, Qais Yousef <qyousef@...alina.io> wrote:
>
> On 03/03/24 17:44, Qais Yousef wrote:
>
> >       diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> >       index 174687252e1a..b0e60a565945 100644
> >       --- a/kernel/sched/fair.c
> >       +++ b/kernel/sched/fair.c
> >       @@ -8260,6 +8260,8 @@ static void set_task_max_allowed_capacity(struct task_struct *p)
> >                       cpumask_t *cpumask;
> >
> >                       cpumask = cpu_capacity_span(entry);
> >       +               if (!cpumask_intersects(cpu_active_mask, cpumask))
> >       +                       continue;
> >                       if (!cpumask_intersects(p->cpus_ptr, cpumask))
> >                               continue;
> >
> >       @@ -8269,6 +8271,53 @@ static void set_task_max_allowed_capacity(struct task_struct *p)
> >               rcu_read_unlock();
> >        }
> >
> >       +static void __update_tasks_max_allowed_capacity(unsigned long capacity)
> >       +{
> >       +       struct task_struct *g, *p;
> >       +
> >       +       for_each_process_thread(g, p) {
> >       +               if (fair_policy(p->policy) && p->max_allowed_capacity == capacity)
>
> This condition actually not good enough. We need to differentiate between going
> online/offline. I didn't want to call set_task_max_allowed_capacity()
> unconditionally and make hotplug even slower.

But should we even try to fix this ? hotplugging a cpu is a special
case and with patch 4 you will not increase lb_interval anymore

>
> I'm doing more testing and will post v8 once done. I need to cater for a new
> user when dynamic EM changes capacities too.. Small things can snow ball easily
> hehe.
>
> >       +                       set_task_max_allowed_capacity(p);
> >       +       }
> >       +}
> >       +
> >       +/*
> >       + * Handle a cpu going online/offline changing the available capacity levels.
> >       + */
> >       +static void update_tasks_max_allowed_capacity(int cpu, bool online)
> >       +{
> >       +       struct asym_cap_data *entry;
> >       +       bool do_update = false;
> >       +
> >       +       if (!sched_asym_cpucap_active())
> >       +               return;
> >       +
> >       +       if (cpuhp_tasks_frozen)
> >       +               return;
> >       +
> >       +       rcu_read_lock();
> >       +       /* Did a capacity level appear/disappear? */
> >       +       list_for_each_entry_rcu(entry, &asym_cap_list, link) {
> >       +               unsigned int nr_active;
> >       +               cpumask_t *cpumask;
> >       +
> >       +               cpumask = cpu_capacity_span(entry);
> >       +
> >       +               if (!cpumask_test_cpu(cpu, cpumask))
> >       +                       continue;
> >       +
> >       +               nr_active = cpumask_weight_and(cpu_active_mask, cpumask);
> >       +               if (online)
> >       +                       do_update = nr_active == 1;
> >       +               else
> >       +                       do_update = !nr_active;
> >       +               break;
> >       +       }
> >       +       if (do_update)
> >       +               __update_tasks_max_allowed_capacity(entry->capacity);
> >       +       rcu_read_unlock();
> >       +}
> >       +
> >        static void set_cpus_allowed_fair(struct task_struct *p, struct affinity_context *ctx)
> >        {
> >               set_cpus_allowed_common(p, ctx);
> >       @@ -12500,6 +12549,8 @@ static void rq_online_fair(struct rq *rq)
> >               update_sysctl();
> >
> >               update_runtime_enabled(rq);
> >       +
> >       +       update_tasks_max_allowed_capacity(cpu_of(rq), true);
> >        }
> >
> >        static void rq_offline_fair(struct rq *rq)
> >       @@ -12511,6 +12562,8 @@ static void rq_offline_fair(struct rq *rq)
> >
> >               /* Ensure that we remove rq contribution to group share: */
> >               clear_tg_offline_cfs_rqs(rq);
> >       +
> >       +       update_tasks_max_allowed_capacity(cpu_of(rq), false);
> >        }
> >
> >        #endif /* CONFIG_SMP */
> > --
> > 2.34.1