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

["Srivatsa S. Bhat" <srivatsa.bhat@linux.vnet.ibm.com>]

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?

Thanks,
Srivatsa S. Bhat