Re: [PATCH 1/2] cpufreq: unlock correct rwsem while updating policy->cpu

[Viresh Kumar]

On 17 September 2013 00:12, Jon Medhurst (Tixy) <tixy@xxxxxxxxxx> wrote:
> The commit log to that patch still mentions taking both locks.

Yeah, it was sent in hurry to just give you a working solution and I forgot
to see the log if it is still valid.

> The code itself fixes the lockdep errors I had, so
>
> Tested-by: Jon Medhurst <tixy@xxxxxxxxxx>

Great!!

> however, I still have concerns about the locking (below)...

:(

> But what about reads, like in cpufreq_frequency_table_update_policy_cpu
> which is called immediately after the unlocking writes? Should that not
> be covered by a reader lock?
>
> And after that call, policy is passed into blocking_notifier_call_chain,
> do those callbacks not want to look at policy fields? Or are they going
> to do there own locking?

policy->cpu is used at multiple places outside of cpufreq.c and cpufreq
core can't really put read locks for those accesses. Things will turn bad
only if cpufreq core has got these races where we are trying to access
a struct or pointer that belonged to the last policy->cpu, which is updated
now.. For example the case of lock you reported.. And so lock is required
only for those places..

> Or is update_policy_cpu itself meant to be called with a read lock held?

No.

> This is the first time I've looked at this code, so feel free just to
> say 'it's complicated, just trust me, I'm the expert, I know what I'm
> doing'...

We aren't that rude :)

Now, that you have tested this patch let me resent it...
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/