Re: [PATCH v2] PM/Hibernation: Fix the early termination of testmodes

[Srivatsa S. Bhat]

On 11/19/2011 03:30 AM, Rafael J. Wysocki wrote:
> On Friday, November 18, 2011, Srivatsa S. Bhat wrote:
>> On 11/18/2011 02:57 PM, Srivatsa S. Bhat wrote:
>>> On 11/18/2011 02:50 PM, Rafael J. Wysocki wrote:
>>>>> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
>>>>> index b4511b6..257e8e7 100644
>>>>> --- a/kernel/power/hibernate.c
>>>>> +++ b/kernel/power/hibernate.c
>>>>> @@ -55,6 +55,8 @@ enum {
>>>>>  
>>>>>  static int hibernation_mode = HIBERNATION_SHUTDOWN;
>>>>>  
>>>>> +static bool freezer_test_done;
>>>>> +
>>>>>  static const struct platform_hibernation_ops *hibernation_ops;
>>>>>  
>>>>>  /**
>>>>> @@ -347,6 +349,17 @@ int hibernation_snapshot(int platform_mode)
>>>>>  	if (error)
>>>>>  		goto Close;
>>>>>  
>>>>> +	if (hibernation_test(TEST_FREEZER) ||
>>>>> +		hibernation_testmode(HIBERNATION_TESTPROC)) {
>>>>> +
>>>>> +		/*
>>>>> +		 * Indicate to the caller that we are returning due to a
>>>>> +		 * successful freezer test.
>>>>> +		 */
>>>>> +		freezer_test_done = true;
>>>>> +		goto Close;
>>>>> +	}
>>>>> +
>>>>>  	error = dpm_prepare(PMSG_FREEZE);
>>>>>  	if (error)
>>>>>  		goto Complete_devices;
>>>>> @@ -641,15 +654,13 @@ int hibernate(void)
>>>>>  	if (error)
>>>>>  		goto Finish;
>>>>>  
>>>>> -	if (hibernation_test(TEST_FREEZER))
>>>>> -		goto Thaw;
>>>>> -
>>>>> -	if (hibernation_testmode(HIBERNATION_TESTPROC))
>>>>> -		goto Thaw;
>>>>> -
>>>>>  	error = hibernation_snapshot(hibernation_mode == HIBERNATION_PLATFORM);
>>>>>  	if (error)
>>>>>  		goto Thaw;
>>>>> +	if (freezer_test_done) {
>>>>> +		freezer_test_done = false;
>>>>> +		goto Thaw;
>>>>> +	}
>>>>
>>>> What happens if hibernation_snapshot() and freezer_test_done is 'true'?
>>>> Won't that leek freezer_test_done to the next hibernation cycle?
>>>>
>>>
>>> Oh yes, thanks for catching that! I'll fix that and send an updated patch.
>>>
>>
>> Sorry, I think I hit reply too soon.
>> To restate your question, you are asking if freezer_test_done was set in
>> hibernation_snapshot() and then there was a genuine error in
>> hibernation_snapshot() which it returned, then won't freezer_test_done
>> leak to the next cycle.., right?
>>
>> If there is a genuine error before hibernation_test(TEST_FREEZER)... is
>> done, then we will never set freezer_test_done flag to true.
>>
>> And whenever freezer_test_done is set (inside hibernation_snapshot) there
>> is no way it will continue further executing normal code. It will jump to
>> the Close label and come out, so that here in hibernate(), the first
>> check for error will fail (because it returned 0) and the subsequent
>> check for freezer_test_done will succeed. So I don't see how it can leak.
>>
>> Or, am I missing something?
> 
> No, that's fine, I just wasn't sure. :-)
> 
> I'm going to apply your patch without the documentation changes.
> 

Great! Thanks a lot!

Thanks,
Srivatsa S. Bhat

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