Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 19 Feb 2021 16:20:26 +0800
From:   Yue Hu <zbestahu@gmail.com>
To:     Viresh Kumar <viresh.kumar@linaro.org>
Cc:     rjw@rjwysocki.net, mingo@redhat.com, peterz@infradead.org,
        juri.lelli@redhat.com, vincent.guittot@linaro.org,
        dietmar.eggemann@arm.com, rostedt@goodmis.org, bsegall@google.com,
        linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
        huyue2@yulong.com, zbestahu@163.com
Subject: Re: [PATCH] cpufreq: schedutil: Don't consider freq reduction to
 busy CPU if need_freq_update is set
Message-ID: <20210219162026.00002e2b.zbestahu@gmail.com>
In-Reply-To: <20210219074249.2hcwcnakihor343h@vireshk-i7>
References: <20210218082514.1437-1-zbestahu@gmail.com>
        <20210218102029.syj6vkltlbtoxsig@vireshk-i7>
        <20210219113804.00004a7e.zbestahu@gmail.com>
        <20210219040933.2o5hhbjb6emf3xl4@vireshk-i7>
        <20210219144140.00004de9.zbestahu@gmail.com>
        <20210219074249.2hcwcnakihor343h@vireshk-i7>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Fri, 19 Feb 2021 13:12:49 +0530
Viresh Kumar <viresh.kumar@linaro.org> wrote:

> On 19-02-21, 14:41, Yue Hu wrote:
> > On Fri, 19 Feb 2021 09:39:33 +0530
> > Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >   
> > > On 19-02-21, 11:38, Yue Hu wrote:  
> > > > There's a possibility: we will use the previous freq to update
> > > > if next_f is reduced for busy CPU if need_freq_update is set in
> > > > sugov_update_next_freq().    
> > > 
> > > Right.
> > >   
> > > > This possibility would happen now? And this
> > > > update is what we want if it happens?    
> > > 
> > > This is exactly what we want here, don't reduce speed for busy
> > > CPU,  
> > 
> > I understand it should not skip this update but set the same freq as
> > previous one again for the special case if need_freq_update is set.
> > Am i rt?  
> 
> The special check, about not reducing freq if CPU had been busy
> recently, doesn't have anything to do with need_freq_update.

However, we will skip the update if need_freq_update is not set. And do
the update if need_freq_update is set.

Note that there are unnecessary fast switch check and spin lock/unlock
operations in freq skip path.

If we consider unnecessary behaviors above, then we should return right
away rather than continue to execute following code.

Consequently, we also need to consider need_update_freq flag since we
need to update freq currently if it is set.

> 
> Though previously we added the need_freq_update check there to make
> sure we account for any recent policy min/max change and don't skip
> freq update anymore. That won't happen anymore and so we don't need
> any check here related to need_freq_update.
> 
> If you still have doubt, please explain your concern in detail with an
> example as I am failing to understand it.
>