Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

PM / Hibernate: Rewrite unlock_system_sleep() to fix s2disk regression

Commit 33e638b, "PM / Sleep: Use the freezer_count() functions in
[un]lock_system_sleep() APIs" introduced an undesirable change in the
behaviour of unlock_system_sleep() since freezer_count() internally calls
try_to_freeze() - which we don't need in unlock_system_sleep().

And commit bcda53f, "PM / Sleep: Replace mutex_[un]lock(&pm_mutex) with
[un]lock_system_sleep()" made these APIs wide-spread. This caused a
regression in suspend-to-disk where snapshot_read() and snapshot_write()
were getting frozen due to the try_to_freeze embedded in
unlock_system_sleep(), since these functions were invoked when the freezing
condition was still in effect.

Fix this by rewriting unlock_system_sleep() by open-coding freezer_count()
and dropping the try_to_freeze() part. Not only will this fix the
regression but this will also ensure that the API only does what it is
intended to do, and nothing more, under the hood.

While at it, make the code more correct and robust by ensuring that the
PF_FREEZER_SKIP flag gets cleared with pm_mutex held, to avoid a race with
the freezer.

Also, to be on the safer side, open-code freezer_do_not_count() as well
(inside lock_system_sleep()), to ensure that any unrelated modification to
freezer[_do_not]_count() does not break things again!

Reported-and-tested-by: Rafael J. Wysocki <rjw@sisk.pl>
Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
Acked-by: Tejun Heo <tj@kernel.org>
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

authored by

Srivatsa S. Bhat and committed by
Rafael J. Wysocki
72081624 160cb5a9

+17 -2
+17 -2
include/linux/suspend.h
··· 357 357 358 358 static inline void lock_system_sleep(void) 359 359 { 360 - freezer_do_not_count(); 360 + current->flags |= PF_FREEZER_SKIP; 361 361 mutex_lock(&pm_mutex); 362 362 } 363 363 364 364 static inline void unlock_system_sleep(void) 365 365 { 366 + /* 367 + * Don't use freezer_count() because we don't want the call to 368 + * try_to_freeze() here. 369 + * 370 + * Reason: 371 + * Fundamentally, we just don't need it, because freezing condition 372 + * doesn't come into effect until we release the pm_mutex lock, 373 + * since the freezer always works with pm_mutex held. 374 + * 375 + * More importantly, in the case of hibernation, 376 + * unlock_system_sleep() gets called in snapshot_read() and 377 + * snapshot_write() when the freezing condition is still in effect. 378 + * Which means, if we use try_to_freeze() here, it would make them 379 + * enter the refrigerator, thus causing hibernation to lockup. 380 + */ 381 + current->flags &= ~PF_FREEZER_SKIP; 366 382 mutex_unlock(&pm_mutex); 367 - freezer_count(); 368 383 } 369 384 370 385 #else /* !CONFIG_PM_SLEEP */