Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <3e6c2e1d4008e70b14abc087c87bb80c78769011.camel@intel.com>
Subject: Re: [RFC PATCH 2/3] cpuidle: ladder: Tune promotion/demotion
 threshold
From:   Zhang Rui <rui.zhang@intel.com>
To:     "Rafael J. Wysocki" <rafael@kernel.org>
Cc:     rjw@rjwysocki.net, daniel.lezcano@linaro.org,
        linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org
Date:   Sun, 27 Nov 2022 11:18:48 +0800
In-Reply-To: <CAJZ5v0gWwqtqezkBapqK4RbefOT2q7R7pWiTb8E4AbptFu7tAg@mail.gmail.com>
References: <20221105174225.28673-1-rui.zhang@intel.com>
         <20221105174225.28673-2-rui.zhang@intel.com>
         <CAJZ5v0gPOUQDb8c_pVYjzBvU3e3U9JoLhJy5vRBF4h2=zvaHHw@mail.gmail.com>
         <5ed329f894bc81f5375303a69c07dee16630503e.camel@intel.com>
         <CAJZ5v0gWwqtqezkBapqK4RbefOT2q7R7pWiTb8E4AbptFu7tAg@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.36.5-0ubuntu1 
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

> 
> > I don't have a solid proof for this. But at least for the pure idle
> > scenario, I don't think 30% deep idle residency is the right
> > behavior,
> > and it needs to be tuned anyway.
> 
> Well, have you checked what happens if the counts are set to the same
> value, e.g. 2?

Well, this is embarrassing. I found a problem with my previous data
when I re-evaluate following your suggestion.

In short,
1. the 30% deep idle residency problem was got when I added some
trace_printk() in the ladder_select_state()
2, without those trace_printk(), after patch 1, the ladder governor can
still get 98% CPU%c7 in pure idle scenario.

Currently, my understanding is that trace_printk() can call
__schedule() and this increased the chance that call_cpuidle() returns
immediately. When this happens, dev->last_residency_ns is set to 0 and
results in a real demotion next time.

Anyway, you are right on questioning this approach, because this seems
to be a different problem or even a false alarm.

So, I think I will submit patch 1/3 and 3/3 as they are bug fixes, and
drop this patch for now, and leave the tuning work, if there is any,
for the real ladder governor users. What do you think?

thanks,
rui