| 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
| ||
|
Message-ID: <CAKfTPtBZVKXoJPz0w2-WxOddfq5k+mNCB3H_Kay6Y9OgB9Y-GQ@mail.gmail.com>
Date: Fri, 15 Nov 2024 16:30:01 +0100
From: Vincent Guittot <vincent.guittot@...aro.org>
To: Saravana Kannan <saravanak@...gle.com>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>, Pavel Machek <pavel@....cz>, Len Brown <len.brown@...el.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Ingo Molnar <mingo@...hat.com>,
Peter Zijlstra <peterz@...radead.org>, Juri Lelli <juri.lelli@...hat.com>,
Dietmar Eggemann <dietmar.eggemann@....com>, Steven Rostedt <rostedt@...dmis.org>,
Ben Segall <bsegall@...gle.com>, Mel Gorman <mgorman@...e.de>,
Valentin Schneider <vschneid@...hat.com>, Geert Uytterhoeven <geert@...ux-m68k.org>, Marek Vasut <marex@...x.de>,
Bird@...gle.com, Tim <Tim.Bird@...y.com>, kernel-team@...roid.com,
linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 5/5] PM: sleep: Spread out async kworker threads during
dpm_resume*() phases
On Fri, 15 Nov 2024 at 06:25, Saravana Kannan <saravanak@...gle.com> wrote:
>
> On Thu, Nov 14, 2024 at 2:09 PM Saravana Kannan <saravanak@...gle.com> wrote:
> >
> > As of today, the scheduler doesn't spread out all the kworker threads
> > across all the available CPUs during suspend/resume. This causes
> > significant resume latency during the dpm_resume*() phases.
> >
> > System resume latency is a very user-visible event. Reducing the
> > latency is more important than trying to be energy aware during that
> > period.
> >
> > Since there are no userspace processes running during this time and
> > this is a very short time window, we can simply disable EAS during
> > resume so that the parallel resume of the devices is spread across all
> > the CPUs.
> >
> > On a Pixel 6, averaging over 100 suspend/resume cycles, the new logic
> > plus disabling EAS for resume yields significant improvements:
> > +---------------------------+-----------+------------+------------------+
> > | Phase | Old full sync | New full async | % change |
> > | | | + EAS disabled | |
> > +---------------------------+-----------+------------+------------------+
> > | Total dpm_suspend*() time | 107 ms | 62 ms | -42% |
> > +---------------------------+-----------+------------+------------------+
> > | Total dpm_resume*() time | 75 ms | 61 ms | -19% |
> > +---------------------------+-----------+------------+------------------+
> > | Sum | 182 ms | 123 ms | -32% |
> > +---------------------------+-----------+------------+------------------+
> >
> > Signed-off-by: Saravana Kannan <saravanak@...gle.com>
> > ---
> > kernel/power/suspend.c | 16 ++++++++++++++++
> > kernel/sched/topology.c | 13 +++++++++++++
> > 2 files changed, 29 insertions(+)
> >
> > diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
> > index 09f8397bae15..7304dc39958f 100644
> > --- a/kernel/power/suspend.c
> > +++ b/kernel/power/suspend.c
> > @@ -393,6 +393,12 @@ void __weak arch_suspend_enable_irqs(void)
> > local_irq_enable();
> > }
> >
> > +/*
> > + * Intentionally not part of a header file to avoid risk of abuse by other
> > + * drivers.
> > + */
> > +void sched_set_energy_aware(unsigned int enable);
extern void sched_set_energy_aware(unsigned int enable);
clear the warning
> > +
> > /**
> > * suspend_enter - Make the system enter the given sleep state.
> > * @state: System sleep state to enter.
> > @@ -468,6 +474,15 @@ static int suspend_enter(suspend_state_t state, bool *wakeup)
> >
> > Platform_wake:
> > platform_resume_noirq(state);
> > + /*
> > + * We do this only for resume instead of suspend and resume for these
> > + * reasons:
> > + * - Performance is more important than power for resume.
> > + * - Power spent entering suspend is more important for suspend. Also,
> > + * stangely, disabling EAS was making suspent a few milliseconds
> > + * slower in my testing.
> > + */
> > + sched_set_energy_aware(0);
> > dpm_resume_noirq(PMSG_RESUME);
> >
> > Platform_early_resume:
> > @@ -520,6 +535,7 @@ int suspend_devices_and_enter(suspend_state_t state)
> > Resume_devices:
> > suspend_test_start();
> > dpm_resume_end(PMSG_RESUME);
> > + sched_set_energy_aware(1);
> > suspend_test_finish("resume devices");
> > trace_suspend_resume(TPS("resume_console"), state, true);
> > resume_console();
> > diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
> > index 9748a4c8d668..c069c0b17cbf 100644
> > --- a/kernel/sched/topology.c
> > +++ b/kernel/sched/topology.c
> > @@ -284,6 +284,19 @@ void rebuild_sched_domains_energy(void)
> > mutex_unlock(&sched_energy_mutex);
> > }
> >
> > +void sched_set_energy_aware(unsigned int enable)
>
> CC kernel/sched/build_utility.o
> In file included from kernel/sched/build_utility.c:88:
> kernel/sched/topology.c:287:6: warning: no previous prototype for
> ‘sched_set_energy_aware’ [-Wmissing-prototypes]
> 287 | void sched_set_energy_aware(unsigned int enable)
> | ^~~~~~~~~~~~~~~~~~~~~~
>
> Peter/Vincent,
>
> I noticed that I'm getting a warning for this line. But I'm not sure
> what to do about it. I intentionally didn't put this in a header file
> because I'm guessing we don't want to make this available to
> drivers/frameworks in general.
>
> Let me know how you want me to handle this.
>
> -Saravana
>
> > +{
> > + int state;
> > +
> > + if (!sched_is_eas_possible(cpu_active_mask))
> > + return;
> > +
> > + sysctl_sched_energy_aware = enable;
> > + state = static_branch_unlikely(&sched_energy_present);
> > + if (state != sysctl_sched_energy_aware)
> > + rebuild_sched_domains_energy();
> > +}
> > +
> > #ifdef CONFIG_PROC_SYSCTL
> > static int sched_energy_aware_handler(const struct ctl_table *table, int write,
> > void *buffer, size_t *lenp, loff_t *ppos)
> > --
> > 2.47.0.338.g60cca15819-goog
> >
Powered by blists - more mailing lists