The step_after_suspend_test verifies that the system successfully suspended and resumed by setting a timerfd and checking whether the timer fully expired. However, this method is unreliable due to timing races.
In practice, the system may take time to enter suspend, during which the timer may expire just before or during the transition. As a result, the remaining time after resume may show non-zero nanoseconds, even if suspend/resume completed successfully. This leads to false test failures.
Replace the timer-based check with a read from /sys/power/suspend_stats/success. This counter is incremented only after a full suspend/resume cycle, providing a reliable and race-free indicator.
Also remove the unused file descriptor for /sys/power/state, which remained after switching to a system() call to trigger suspend [1].
[1] https://lore.kernel.org/all/20240930224025.2858767-1-yifei.l.liu@oracle.com/
Fixes: c66be905cda2 ("selftests: breakpoints: use remaining time to check if suspend succeed") Signed-off-by: Moon Hee Lee moonhee.lee.ca@gmail.com --- .../breakpoints/step_after_suspend_test.c | 41 ++++++++++++++----- 1 file changed, 31 insertions(+), 10 deletions(-)
diff --git a/tools/testing/selftests/breakpoints/step_after_suspend_test.c b/tools/testing/selftests/breakpoints/step_after_suspend_test.c index 8d275f03e977..8d233ac95696 100644 --- a/tools/testing/selftests/breakpoints/step_after_suspend_test.c +++ b/tools/testing/selftests/breakpoints/step_after_suspend_test.c @@ -127,22 +127,42 @@ int run_test(int cpu) return KSFT_PASS; }
+/* + * Reads the suspend success count from sysfs. + * Returns the count on success or exits on failure. + */ +static int get_suspend_success_count_or_fail(void) +{ + FILE *fp; + int val; + + fp = fopen("/sys/power/suspend_stats/success", "r"); + if (!fp) + ksft_exit_fail_msg( + "Failed to open suspend_stats/success: %s\n", + strerror(errno)); + + if (fscanf(fp, "%d", &val) != 1) { + fclose(fp); + ksft_exit_fail_msg( + "Failed to read suspend success count\n"); + } + + fclose(fp); + return val; +} + void suspend(void) { - int power_state_fd; int timerfd; int err; + int count_before; + int count_after; struct itimerspec spec = {};
if (getuid() != 0) ksft_exit_skip("Please run the test as root - Exiting.\n");
- power_state_fd = open("/sys/power/state", O_RDWR); - if (power_state_fd < 0) - ksft_exit_fail_msg( - "open("/sys/power/state") failed %s)\n", - strerror(errno)); - timerfd = timerfd_create(CLOCK_BOOTTIME_ALARM, 0); if (timerfd < 0) ksft_exit_fail_msg("timerfd_create() failed\n"); @@ -152,14 +172,15 @@ void suspend(void) if (err < 0) ksft_exit_fail_msg("timerfd_settime() failed\n");
+ count_before = get_suspend_success_count_or_fail(); + system("(echo mem > /sys/power/state) 2> /dev/null");
- timerfd_gettime(timerfd, &spec); - if (spec.it_value.tv_sec != 0 || spec.it_value.tv_nsec != 0) + count_after = get_suspend_success_count_or_fail(); + if (count_after <= count_before) ksft_exit_fail_msg("Failed to enter Suspend state\n");
close(timerfd); - close(power_state_fd); }
int main(int argc, char **argv)
On 6/26/25 13:16, Moon Hee Lee wrote:
The step_after_suspend_test verifies that the system successfully suspended and resumed by setting a timerfd and checking whether the timer fully expired. However, this method is unreliable due to timing races.
In practice, the system may take time to enter suspend, during which the timer may expire just before or during the transition. As a result, the remaining time after resume may show non-zero nanoseconds, even if suspend/resume completed successfully. This leads to false test failures.
Replace the timer-based check with a read from /sys/power/suspend_stats/success. This counter is incremented only after a full suspend/resume cycle, providing a reliable and race-free indicator.
Also remove the unused file descriptor for /sys/power/state, which remained after switching to a system() call to trigger suspend [1].
[1] https://lore.kernel.org/all/20240930224025.2858767-1-yifei.l.liu@oracle.com/
Fixes: c66be905cda2 ("selftests: breakpoints: use remaining time to check if suspend succeed") Signed-off-by: Moon Hee Lee moonhee.lee.ca@gmail.com
Applied to linux-kselftest next branch for Linux 6.17-rc1
thanks, -- Shuah
Le 26/06/2025 à 21:16, Moon Hee Lee a écrit :
[...]
Replace the timer-based check with a read from /sys/power/suspend_stats/success. This counter is incremented only after a full suspend/resume cycle, providing a reliable and race-free indicator.
Also remove the unused file descriptor for /sys/power/state, which remained after switching to a system() call to trigger suspend [1].
[1]https://lore.kernel.org/all/20240930224025.2858767-1-yifei.l.liu@oracle.com/
Fixes: c66be905cda2 ("selftests: breakpoints: use remaining time to check if suspend succeed") Signed-off-by: Moon Hee Leemoonhee.lee.ca@gmail.com
.../breakpoints/step_after_suspend_test.c | 41 ++++++++++++++----- 1 file changed, 31 insertions(+), 10 deletions(-)
[...]
void suspend(void) {
int power_state_fd; int timerfd; int err;
int count_before;
int count_after; struct itimerspec spec = {}; if (getuid() != 0) ksft_exit_skip("Please run the test as root - Exiting.\n");
power_state_fd = open("/sys/power/state", O_RDWR);
if (power_state_fd < 0)
ksft_exit_fail_msg(
"open(\"/sys/power/state\") failed %s)\n",
strerror(errno));
timerfd = timerfd_create(CLOCK_BOOTTIME_ALARM, 0); if (timerfd < 0) ksft_exit_fail_msg("timerfd_create() failed\n");
@@ -152,14 +172,15 @@ void suspend(void) if (err < 0) ksft_exit_fail_msg("timerfd_settime() failed\n");
count_before = get_suspend_success_count_or_fail();
system("(echo mem > /sys/power/state) 2> /dev/null");
timerfd_gettime(timerfd, &spec);
if (spec.it_value.tv_sec != 0 || spec.it_value.tv_nsec != 0)
Hi,
Shouldn't you also remove the timerfd variable? It seems to be of no functional use after this change. -- This message and any attachments herein are, unless otherwise stated, confidential, intended solely for the addressees and are SoftAtHome’s ownership. Any unauthorized use, reproduction or dissemination is prohibited unless formaly agreed beforehand by the sender. If you are not the intended addressee of this message, please immediately delete it and all its attachments from your computer system and notify the sender. SoftAtHome reserves the right to monitor all email communications through its networks. Any views or opinions presented are solely those of its author and do not necessarily represent those of SoftAtHome. The internet cannot guarantee the integrity of this message. SoftAtHome not shall be liable for the message if altered, changed or falsified. While we take all reasonable precautions to ensure that viruses are not transmitted via emails, we recommend that you take your own measures to prevent viruses from entering your computer system. SoftAtHome is a French Société Anonyme with a Board of Directors, having a capital of 6 450 699 Euros having its registered office located at 9-11 rue du débarcadère – 92700 – Colombes – France – Tel + 33 (0)1 57 66 88 88 – Fax + 33 (0)1 57 66 88 89 - RCS Nanterre B 500 440 813 – Intra-Community VAT: FR 04500440813 -- Ce message et toutes les pièces jointes qui y sont incluses sont, sauf indication contraire, confidentiels, destinés uniquement aux destinataires et sont la propriété de SoftAtHome. Toute utilisation non autorisée, reproduction ou diffusion est interdite, sauf accord formel préalable de l'expéditeur. Si vous n'êtes pas le destinataire prévu de ce message, veuillez le supprimer immédiatement ainsi que toutes ses pièces jointes de votre système informatique et en informer l'expéditeur. SoftAtHome se réserve le droit de surveiller toutes les communications par e-mail via ses réseaux. Les opinions exprimées dans ce message sont celles de leur auteur et ne représentent pas nécessairement celles de SoftAtHome. L’Internet ne permettant pas d’assurer l’intégrité de ce message, SoftAtHome décline toute responsabilité à ce titre, dans l’hypothèse où il aurait été altéré, déformé ou falsifié. Par ailleurs et malgré toutes les précautions prises pour éviter la présence de virus dans nos envois, nous vous recommandons de prendre, de votre côté, les mesures permettant d'assurer la non-introduction de virus dans votre système informatique. SoftAtHome est une Société Anonyme française à Conseil d’Administration ayant un capital de 6 450 699 euros, dont le siège social est situé au 9-11 rue du débarcadère - 92700 - Colombes - France - Tel + 33 (0)1 57 66 88 88 - Fax + 33 (0)1 57 66 88 89 RCS Nanterre B 500 440 813 - TVA intracommunautaire : FR 04500440813
Hi Olivier,
On Fri, Aug 1, 2025 at 5:39 AM Olivier Blin olivier.blin@softathome.com wrote:
Shouldn't you also remove the timerfd variable? It seems to be of no functional use after this change.
The timerfd is still required because it provides the wake-up event for this test. Just before suspend the code programs a CLOCK_BOOTTIME_ALARM to expire in five seconds. While the system is asleep it needs an interrupt to resume; this timer supplies it. The patch only changes how success is checked: we read /sys/power/suspend_stats/success after resume, instead of comparing time diffs on the timerfd. If the timerfd were removed, the machine could suspend with no scheduled event to bring it back, turning the test into a hang rather than yielding a result.
Thanks for the review.
Regards, Moon
linux-kselftest-mirror@lists.linaro.org