Re: [PATCH 4/5] cpufreq: Don't drop rwsem before calling CPUFREQ_GOV_POLICY_EXIT

From: Rafael J. Wysocki
Date: Tue Feb 02 2016 - 16:54:13 EST


On Tue, Feb 2, 2016 at 11:57 AM, Viresh Kumar <viresh.kumar@xxxxxxxxxx> wrote:
> Commit 955ef4833574 ("cpufreq: Drop rwsem lock around
> CPUFREQ_GOV_POLICY_EXIT") dropped these because of some ABBA lockup
> issues.
>
> The previous commit has fixed them all and we don't need to drop these
> locks anymore.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@xxxxxxxxxx>

First of all, this is effectively reverting commit 955ef4833574, so
the subject should be "Revert commit 955ef4833574 (cpufreq: Drop rwsem
lock around CPUFREQ_GOV_POLICY_EXIT)".

There should be a Fixes: tag pointing to commit 955ef4833574 and a
Reported-by: for Juri.

If there is a link to a bug report related to this, it should be
pointed to by a Link: tag.

The changelog should say why the original commit was there and why the
way it attempted to solve the problem was incorrect. It also should
say that the original problem was addressed by a previous commit, so
this one can be reverted without consequences.

But I'm not going to write that changelog. I actually am not going to
write any changelogs for you any more, because I'm seriously tired of
doing that. Moreover, if I see a patch from you with a changelog
that's not acceptable to me, it will immediately go to the "not
applicable" trash bin no matter what the changes below look like. You
*have* *to* write useful changelogs. This isn't optional or best
effort. This is mandatory and important.

Now, I'm not really sure if the ordering of this patchset is right.
Maybe we should just revert upfront with the "we'll address the
original problem in the following commits" statement in the changelog
and fix it in a different way? It looks like patches [1-3/5] fix a
problem that isn't there even, but would appear after the [4/5] if
they were not applied previously, which doesn't sound really
straightforward to me.

Thanks,
Rafael