Re: [PATCH][RFC v2] ACPI: acpi_pad: Do not launch acpi_pad threads on idle cpus

From: Yu Chen
Date: Mon Dec 10 2018 - 22:04:52 EST


Hi Joey,
On Mon, Dec 10, 2018 at 02:31:53PM +0800, joeyli wrote:
> Hi Chen Yu and ACPI experts,
>
> On Sat, May 05, 2018 at 07:53:22PM +0800, Chen Yu wrote:
> > According to current implementation of acpi_pad driver,
> > it does not make sense to spawn any power saving threads
> > on the cpus which are already idle - it might bring
> > unnecessary overhead on these idle cpus and causes power
> > waste. So verify the condition that if the number of 'busy'
> > cpus exceeds the amount of the 'forced idle' cpus is met.
> > This is applicable due to round-robin attribute of the
> > power saving threads, otherwise ignore the setting/ACPI
> > notification.
> >
> > Suggested-by: Lenny Szubowicz <lszubowi@xxxxxxxxxx>
> > Suggested-by: Len Brown <lenb@xxxxxxxxxx>
> > Cc: "Rafael J. Wysocki" <rjw@xxxxxxxxxxxxx>
> > Cc: Lenny Szubowicz <lszubowi@xxxxxxxxxx>
> > Cc: Len Brown <lenb@xxxxxxxxxx>
> > Cc: Jacob Pan <jacob.jun.pan@xxxxxxxxxxxxxxx>
> > Cc: Rui Zhang <rui.zhang@xxxxxxxxx>
> > Cc: linux-acpi@xxxxxxxxxxxxxxx
> > Signed-off-by: Chen Yu <yu.c.chen@xxxxxxxxx>
>
> Do you have any news for this patch? Why it did not merged by kernel
> maineline?
>
We are evaluating if this could be integrated into idle injection framework.
May I know if there's any requirement/background from SUSE on this?

Best,
Ryan(Yu)
> Thanks a lot!
> Joey Lee
>
> > ---
> > drivers/acpi/acpi_pad.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 51 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/acpi/acpi_pad.c b/drivers/acpi/acpi_pad.c
> > index 552c1f7..515e60e 100644
> > --- a/drivers/acpi/acpi_pad.c
> > +++ b/drivers/acpi/acpi_pad.c
> > @@ -254,12 +254,62 @@ static void set_power_saving_task_num(unsigned int num)
> > }
> > }
> >
> > +/*
> > + * Extra acpi_pad threads should not be created until
> > + * the requested idle count is less than/equals to the
> > + * number of the busy cpus - it does not make sense to
> > + * throttle the idle cpus.
> > + */
> > +#define SAMPLE_INTERVAL_JIF 20
> > +
> > +static u64 get_idle_time(int cpu)
> > +{
> > + u64 idle, idle_usecs = -1ULL;
> > +
> > + idle_usecs = get_cpu_idle_time_us(cpu, NULL);
> > +
> > + if (idle_usecs == -1ULL)
> > + idle = kcpustat_cpu(cpu).cpustat[CPUTIME_IDLE];
> > + else
> > + idle = idle_usecs * NSEC_PER_USEC;
> > +
> > + return idle;
> > +}
> > +
> > +static bool idle_nr_valid(unsigned int num_cpus)
> > +{
> > + int busy_nr = 0, i = 0, load_thresh = 100 - idle_pct;
> > +
> > + if (!num_cpus)
> > + return true;
> > +
> > + for_each_online_cpu(i) {
> > + u64 wall_time, idle_time;
> > + unsigned int elapsed_delta, idle_delta, load;
> > +
> > + wall_time = jiffies64_to_nsecs(get_jiffies_64());
> > + idle_time = get_idle_time(i);
> > + /* Wait and see... */
> > + schedule_timeout_uninterruptible(SAMPLE_INTERVAL_JIF);
> > +
> > + idle_delta = get_idle_time(i) - idle_time;
> > + elapsed_delta = jiffies64_to_nsecs(get_jiffies_64()) - wall_time;
> > + idle_delta = (idle_delta > elapsed_delta) ? elapsed_delta : idle_delta;
> > + load = 100 * (elapsed_delta - idle_delta) / elapsed_delta;
> > + if (load >= load_thresh)
> > + busy_nr++;
> > + }
> > +
> > + return (busy_nr >= num_cpus) ? true : false;
> > +}
> > +
> > static void acpi_pad_idle_cpus(unsigned int num_cpus)
> > {
> > get_online_cpus();
> >
> > num_cpus = min_t(unsigned int, num_cpus, num_online_cpus());
> > - set_power_saving_task_num(num_cpus);
> > + if (idle_nr_valid(num_cpus))
> > + set_power_saving_task_num(num_cpus);
> >
> > put_online_cpus();
> > }
> > --
> > 2.7.4
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > More majordomo info at http://vger.kernel.org/majordomo-info.html