[PATCH] selftests: breakpoints: use suspend_stats to reliably check suspend success

[PATCH] selftests: breakpoints: use suspend_stats to reliably check suspend success

[Moon Hee Lee]

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)
-- 
2.43.0

Re: [PATCH] selftests: breakpoints: use suspend_stats to reliably check suspend success

[Shuah Khan]

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

Re: [PATCH] selftests: breakpoints: use suspend_stats to reliably check suspend success

[Olivier Blin]

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 Lee<moonhee.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

Re: [PATCH] selftests: breakpoints: use suspend_stats to reliably check suspend success

[Moon Hee Lee]

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