Message-ID: <f33bc346d8d4ae086ddf59db2670ce9a8f80a250.camel@linux.intel.com>
Subject: Re: [PATCH v2 0/2] intel_powerclamp: New module parameter
From:   srinivas pandruvada <srinivas.pandruvada@linux.intel.com>
To:     "Zhang, Rui" <rui.zhang@intel.com>,
        "rafael@kernel.org" <rafael@kernel.org>,
        "Neri, Ricardo" <ricardo.neri@intel.com>
Cc:     "linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "daniel.lezcano@linaro.org" <daniel.lezcano@linaro.org>
Date:   Mon, 06 Feb 2023 02:02:28 -0800
In-Reply-To: <468eedb9d8a839d22ffd18125d3104f8c014965c.camel@intel.com>
References: <20230205025902.2899734-1-srinivas.pandruvada@linux.intel.com>
         <a68a6f8c76cb719cd4865bd6aa726306772d4ee3.camel@intel.com>
         <60514763753f572f854f1bbf287c3c16fbbc12c3.camel@linux.intel.com>
         <468eedb9d8a839d22ffd18125d3104f8c014965c.camel@intel.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.44.4-0ubuntu1 
MIME-Version: 1.0
Precedence: bulk

On Mon, 2023-02-06 at 08:05 +0000, Zhang, Rui wrote:
> On Sun, 2023-02-05 at 18:45 -0800, srinivas pandruvada wrote:
> > Hi Rui,
> >=20
> > On Sun, 2023-02-05 at 15:57 +0000, Zhang, Rui wrote:
> > > Hi, Srinivas,
> > >=20
> > > First of all, the previous build error is gone.
> > >=20
> > > Second, I found something strange, which may be related with the
> > > scheduler asym-packing, so CC Ricardo.
> > >=20
> > I thought you disable ITMT before idle injection and reenebale
> > after
> > removal.
>=20
> No.
>=20
> I can reproduce this by playing with raw intel_powerclamp sysfs knobs
> and ITMT enabled.
>=20

This issue is happening even if ITMT disabled. If the module mask is
composed of P-cores it works or even on servers as expected.
Also if you offline all P-cores then select mask among E-cores, it is
working. Somehow P-core influences E-cores.

Since this patch is module mask related, that is functioning correctly.
We have to debug this interaction with P and E cores separately.

Thanks,
Srinivas


> >=20
> >=20
> >=20
> > > The test is done with pm linux-intel branch
>=20
> sorry, I mean linux-next branch.
>=20
> > > =C2=A0+ this patch series on an
> > > ADL system.
> > Can you do test on bleeding edge branch of Linux-pm?
> >=20
> > > =C2=A0cpu0~cpu7 are Pcore cpus, cpu8-cpu15 are Ecore cpus, and
> > > intel_powerclamp is register as cooling_device21.
> > >=20
> > > 1. run stress -c 16
> > > 2. update /sys/module/intel_powerclamp/parameters/cpumask
> > > =C2=A0=C2=A0 echo 90 > /sys/module/intel_powerclamp/parameters/max_id=
le
> > > 3. echo 90 > /sys/class/thermal/cooling_device21/cur_state
> > > 4. echo 0 > /sys/class/thermal/cooling_device21/cur_state
> > > I use turbostat to monitor the CPU Busy% in all 4 steps.
> > >=20
> > > If 'cpumask' does not include all the Ecore CPUs, all CPUs
> > > becomes
> > > 100%
> > > busy after idle injection removed in step 4.
> > >=20
> > that should be the case.
> >=20
> > > If 'cpumask' includes all the Ecore CPUs, i.e. cpumask =3D FFxy, in
> > > some
> > > cases, the Ecore CPUs will drop to an Busy% much lower than 10%,
> > > and
> > > then they don't come back to busy after idle injection removed in
> > > step
> > =C2=A0Do you see that idle injection is removed message in dmesg?
>=20
> yes.
>=20
> > We can also check powercap idle-inejct, if some CPUs still not wake
> > from play_idle.
>=20
> "ps" command shows the the idle_injection threads time is not
> increasing any more.
>=20
> >=20
> >=20
> > > 4, although we have 16 stress threads. And this also relates with
> > > how
> > > long we stay in idle injection.
> > >=20
> > > Say, when cpumask=3Dfff3, the problem can be triggered occasionally
> > > if
> > > there is a 10 second timeout between step 3 and step4, but it is
> > > much
> > > easier to reproducible if I increase the timeout to 20 seconds.
> > >=20
> > > It seems that Pcore can always pull tasks from Ecores, but Ecore
> > > can
> > > not pull tasks from Pcore HT siblings.
> > >=20
> > That will be regular load balance threads should do.
> > Better to try upsteam kernel first.
>=20
> I'm already running with linux-pm tree linux-next branch + this patch
> series.
>=20
> thanks,
> rui
>=20
> >=20
> > Thanks,
> > Srinivas
> >=20
> >=20
> > > thanks,
> > > rui
> > >=20
> > > On Sat, 2023-02-04 at 18:59 -0800, Srinivas Pandruvada wrote:
> > > > Split from the series for powerclamp user of powercap idle-
> > > > inject.
> > > >=20
> > > > v2
> > > > - Build warnings reported by Rui
> > > > - Moved the powerclamp documentation to admin guide folder
> > > > - Commit log updated as suggested by Rafael and other code
> > > > suggestion
> > > >=20
> > > > Srinivas Pandruvada (2):
> > > > =C2=A0 Documentation:admin-guide: Move intel_powerclamp
> > > > documentation
> > > > =C2=A0 thermal/drivers/intel_powerclamp: Add two module parameters
> > > >=20
> > > > =C2=A0Documentation/admin-guide/index.rst=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 |=C2=A0=C2=A0 1 +
> > > > =C2=A0.../thermal/intel_powerclamp.rst=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 |=C2=A0 22 +++
> > > > =C2=A0Documentation/driver-api/thermal/index.rst=C2=A0=C2=A0=C2=A0 =
|=C2=A0=C2=A0 1 -
> > > > =C2=A0MAINTAINERS=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0 |=C2=A0=C2=A0 1 +
> > > > =C2=A0drivers/thermal/intel/intel_powerclamp.c=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0 | 177
> > > > +++++++++++++++-
> > > > --
> > > > =C2=A05 files changed, 180 insertions(+), 22 deletions(-)
> > > > =C2=A0rename Documentation/{driver-api =3D> admin-
> > > > guide}/thermal/intel_powerclamp.rst (93%)
> > > >=20