Re: [PATCH 1/12] cpufreq: governor: Close dbs_data update race condition

From	"Rafael J. Wysocki" <rafael@kernel.org>
Newsgroups	linux.kernel
Subject	Re: [PATCH 1/12] cpufreq: governor: Close dbs_data update race condition
Date	2016-02-19 03:40 +0100
Message-ID	<r3J97-6Cc-9@gated-at.bofh.it> (permalink)
References	<r3lJv-6xI-3@gated-at.bofh.it> <r3lJv-6xI-1@gated-at.bofh.it> <r3pk6-MU-17@gated-at.bofh.it> <r3zCP-8go-17@gated-at.bofh.it> <r3IZs-6yl-5@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

On Fri, Feb 19, 2016 at 3:27 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 18-02-16, 17:20, Rafael J. Wysocki wrote:
>> On Thu, Feb 18, 2016 at 6:24 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>> > On 18-02-16, 02:19, Rafael J. Wysocki wrote:
>
>> >> @@ -112,7 +112,7 @@ static ssize_t governor_store(struct kob
>> >>
>> >>       mutex_lock(&dbs_data->mutex);
>> >>
>> >> -     if (gattr->store)
>> >> +     if (dbs_data->usage_count && gattr->store)
>> >
>> > That's not gonna be enough. The above lock doesn't guarantee
>> > protection with any such races.
>
> Oops, I completely misread it. Really sorry about that.
>
> But now that I have read the code again, I wonder why we need this protection at
> all. The first thing we do after decrementing the usage_count counter, is we put
> the kobject. Which will ensure that the sysfs files are all gone. So, what is
> the race we are trying to fix then?

The ->store() callbacks for different attributes may do silly stuff
like walking all CPUs in the system and updating per-CPU data for
them.  If the dbs_data the callback has been called for is going away,
this is pointless at best and may be actually harmful depending on
what the callback is really doing.

Thanks,
Rafael

Thread

[PATCH 1/12] cpufreq: governor: Close dbs_data update race condition "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-02-18 02:40 +0100
  Re: [PATCH 1/12] cpufreq: governor: Close dbs_data update race  condition Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-18 06:30 +0100
    Re: [PATCH 1/12] cpufreq: governor: Close dbs_data update race condition "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-18 17:30 +0100
      Re: [PATCH 1/12] cpufreq: governor: Close dbs_data update race  condition Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-19 03:30 +0100
        Re: [PATCH 1/12] cpufreq: governor: Close dbs_data update race condition "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-19 03:40 +0100
  Re: [PATCH 1/12] cpufreq: governor: Close dbs_data update race  condition Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-19 04:20 +0100

csiph-web