Re: Regression with suspend resume 5a87182aa21d6d5d306840feab9321818dd3e2a3

[Viresh Kumar]

On 16 December 2013 17:49, Bjørn Mork <bjorn@xxxxxxx> wrote:
> Viresh Kumar <viresh.kumar@xxxxxxxxxx> writes:

>> As a summary, after my patch to suspend/resume governors we can't
>> allow policies to be freed and allocated back.
>
> How do you deal with errors on suspend/resume then?  Are you always able
> to keep the policies, for all error cases?

We fixed that with http://www.spinics.net/lists/cpufreq/msg08720.html, isn't it?

> In any case: Splitting the suspend code between a cpu hotplug hook with
> special "frozen" logic and a cpufreq_suspend() called from
> dpm_suspend_noirq() confuses me, and I believe many others.  This is the
> reason such a bug could be caused by two "obviously fine" patches.  So
> please, at least keep the suspend logic in *one* place.

This fixes it I believe.

https://lkml.org/lkml/2013/12/2/26

Specially patch 3/3

>> Its not really a war between my patch versus yours :), but I believe the
>> right thing to do at this point is to get two patches in for 3.13 as well:
>>
>> 5a87182 cpufreq: suspend governors on system suspend/hibernate
>> and patch discussed here:
>> http://www.spinics.net/lists/cpufreq/msg08720.html
>
> Yes, that would probably work fine, at least as long as nothing goes
> wrong.  I must admit that I'm in no way able to play out all the
> different error scenarios in my head, but won't there still be cases
> where you end up freeing policies on suspend/resume?

No, we aren't supposed to free policies at all in suspend/resume..

>> To finish this problem as early as possible I tested above two
>> patches and didn't saw any regressions with suspend/resume or
>> Hibernation.. And obviously this fixes your issues as well :)
>>
>> @Rafael: I understand that it would be difficult for you to take these
>> now for 3.13 but they fix some serious problems reported earlier.
>> Specially the first patch which everybody thought is the culprit :)
>>
>> Please see if we can manage to get them in :)
>
> I think it needs serious testing with simulated errors first.  All error
> labels should be executed at least once for all combinations of inputs.

That can be best done by the people who reported these issues. And
you are one of those :)

I will ask others also to give these patches a try. .That would be
helpful.

> Simply trying it out and verifying that it works in the no-error case is
> not enough.  Which should be quite obvious now.

Sure.. But I already tested it earlier as well, when I sent that patch to
you. I tested it on top of my branch where the reverted patch was
present. So, for me they should work fine..

--
viresh
--
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/