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;
Date:   Wed, 25 May 2022 11:02:16 +0530
From:   Viresh Kumar <viresh.kumar@linaro.org>
To:     "Rafael J. Wysocki" <rafael@kernel.org>
Cc:     Schspa Shi <schspa@gmail.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Linux PM <linux-pm@vger.kernel.org>
Subject: Re: [PATCH v3] cpufreq: fix race on cpufreq online
Message-ID: <20220525053216.yw4asdj3ynf7fzwo@vireshk-i7>
References: <CAJZ5v0gN_yDFpvCXRXv8rN-i3TugCi-HKpBKK2z4eWU0Zm1GUg@mail.gmail.com>
 <CAJZ5v0id+7vkqMQEyVRe29oF_dRtzZ0EhoYUn8=yzeENDeABJw@mail.gmail.com>
 <20220512065623.q4aa6y52pst3zpxu@vireshk-i7>
 <CAJZ5v0jeYiZ6esdxnJbOyDztNqOAbjcjxmpca3JTFWRh+cwdBw@mail.gmail.com>
 <20220513042705.nbnd6vccuiu6lb7a@vireshk-i7>
 <20220524111456.hw4qugsvt4bm7reh@vireshk-i7>
 <CAJZ5v0i_qpcaUwdZY=KR_e=HC6MZUw2aDGL=hxgo866dF1zwOA@mail.gmail.com>
 <20220524112917.apcvvvblksg7jdu4@vireshk-i7>
 <CAJZ5v0jRtYcscWjUras9RC9LOTHf=qu1SPBhnC=52Gb3KKAQNw@mail.gmail.com>
 <CAJZ5v0hHcW-JkXyOZ7ftR+sUQxysEV2--j29owdojYbg_dR_iw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAJZ5v0hHcW-JkXyOZ7ftR+sUQxysEV2--j29owdojYbg_dR_iw@mail.gmail.com>
Precedence: bulk

On 24-05-22, 13:53, Rafael J. Wysocki wrote:
> On Tue, May 24, 2022 at 1:48 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
> > So this was done before the entire CPU hotplug rework and it was
> > useful at that time.
> >
> > The current code always runs cpufreq_set_policy() under policy->rwsem
> > and governors are stopped under policy->rwsem, so this particular race
> > cannot happen AFAICS.
> >
> > Locking CPU hotplug prevents CPUs from going away while store() is
> > running, but in order to run store(), the caller must hold an active
> > reference to the policy kobject.  That prevents the policy from being
> > freed and so policy->rwsem can be acquired.  After policy->rwsem has
> > been acquired, policy->cpus can be checked to determine whether or not
> > there are any online CPUs for the given policy (there may be none),
> > because policy->cpus is only manipulated under policy->rwsem.
> >
> > If a CPU that belongs to the given policy is going away,
> > cpufreq_offline() has to remove it from policy->cpus under
> > policy->rwsem, so either it has to wait for store() to release
> > policy->rwsem, or store() will acquire policy->rwsem after it and will
> > find that policy->cpus is empty.
> 
> Moreover, locking CPU hotplug doesn't actually prevent
> cpufreq_remove_dev() from running which can happen when the cpufreq
> driver is unregistered, for example.

Right, we can get rid of this now I believe.

-- 
viresh