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

[Srivatsa S. Bhat]

On 11/18/2011 02:50 PM, Rafael J. Wysocki wrote:
> On Friday, November 18, 2011, Srivatsa S. Bhat wrote:
>> Commit 2aede851ddf08666f68ffc17be446420e9d2a056
>> (PM / Hibernate: Freeze kernel threads after preallocating memory)
>> postponed the freezing of kernel threads to after preallocating memory
>> for hibernation. But while doing that, the hibernation test TEST_FREEZER
>> and the test mode HIBERNATION_TESTPROC were not moved accordingly.
>>
>> As a result, when using these test modes, it only goes upto the freezing of
>> userspace and exits, when in fact it should go till the complete end of task
>> freezing stage, namely the freezing of kernel threads as well.
>>
>> So, move these points of exit to appropriate places so that freezing of
>> kernel threads is also tested while using these test harnesses. And while at
>> it, add some documentation about these test modes.
>>
>> v2: Changed 'freezer_test_done' flag from int to bool.
>>
>> Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@xxxxxxxxxxxxxxxxxx>
> 
> On a second thought I'm not sure about the documentation changes.
> In fact, the "test" and "testproc" modes are deprecated and I was going to
> remove them some time this year.  That's why they weren't documented
> (i.e. intentionally) and it's better to keep it that way IMO.
> 

I was actually wondering about the same thing (why are test and testproc
both still there, when pm_test is also there)..

So I'll drop the documentation changes in this patch, and maybe send a
separate patch that will remove the 2? Will that be good?

>> ---
>>
>>  Documentation/power/basic-pm-debugging.txt |   16 ++++++++++++----
>>  kernel/power/hibernate.c                   |   28 ++++++++++++++++++++++------
>>  2 files changed, 34 insertions(+), 10 deletions(-)
>>
>> diff --git a/Documentation/power/basic-pm-debugging.txt b/Documentation/power/basic-pm-debugging.txt
>> index 40a4c65..ba6c879 100644
>> --- a/Documentation/power/basic-pm-debugging.txt
>> +++ b/Documentation/power/basic-pm-debugging.txt
>> @@ -38,10 +38,18 @@ identify what goes wrong.
>>  
>>  a) Test modes of hibernation
>>  
>> -To find out why hibernation fails on your system, you can use a special testing
>> -facility available if the kernel is compiled with CONFIG_PM_DEBUG set.  Then,
>> -there is the file /sys/power/pm_test that can be used to make the hibernation
>> -core run in a test mode.  There are 5 test modes available:
>> +To find out why hibernation fails on your system, you can use some special
>> +testing facilities available if the kernel is compiled with CONFIG_PM_DEBUG set.
>> +Hibernation will then have 2 more modes that can be used, namely "testproc" and
>> +"test". When the "testproc" mode is used, it executes everything upto the task
>> +freezing phase and reverts back. When the "test" mode is used, it goes much
>> +deeper down, stopping just short of actually hibernating the machine, and then
>> +reverts back to normal state.
>> +
>> +However, there is an even more fine-grained debugging tool, also enabled by
>> +setting CONFIG_PM_DEBUG: there appears a file /sys/power/pm_test that can be
>> +used to make the hibernation core run in a test mode.
>> +There are 5 test modes available:
>>  
>>  freezer
>>  - test the freezing of processes
>> 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.

>>  
>>  	if (in_suspend) {
>>  		unsigned int flags = 0;
>> @@ -864,6 +875,11 @@ static const char * const hibernation_modes[] = {
>>   *
>>   * If a platform hibernation driver is in use, 'platform' will be supported
>>   * and will be used by default.  Otherwise, 'shutdown' will be used by default.
>> + * The 2 test modes are useful only when CONFIG_PM_DEBUG is set. In that case,
>> + * using the 'testproc' mode will do everything upto the freezing of tasks
>> + * and exits after undoing all that it did. The 'test' mode is deeper, in that,
>> + * it does everything except actually hibernating the machine and exits after
>> + * restoring the machine back to its original state.
>>   * The selected option (i.e. the one corresponding to the current value of
>>   * hibernation_mode) is enclosed by a square bracket.
>>   *
> 

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/