2022-12-15 06:27:44

by Shradha Gupta

[permalink] [raw]
Subject: [PATCH] kernel: power: swap: Suppress expected 'Image not found' error on Ubuntu

In 'systemctl hibernate' if resume device is written to the sysfs
resume parameter, a software_resume() call is triggerred. This call
is expected to fail in swsusp_check() call with -EBUSY error and an
'Image not found' error message. This fix suppresses the expected
failure message from getting logged in Ubuntu setups where
CONFIG_DYNAMIC_DEBUG is enabled by default.

Signed-off-by: Shradha Gupta <[email protected]>
---
kernel/power/swap.c | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/kernel/power/swap.c b/kernel/power/swap.c
index 277434b6c0bf..9cbd3edc8339 100644
--- a/kernel/power/swap.c
+++ b/kernel/power/swap.c
@@ -1552,6 +1552,14 @@ int swsusp_check(void)
pr_debug("Image signature found, resuming\n");
} else {
error = PTR_ERR(hib_resume_bdev);
+ /*
+ * If this call is triggered by `systemctl hibernate`, the
+ * call is expected to fail with -EBUSY error.
+ * TODO: Figure out a better way to suppress the error msg
+ * in this case.
+ */
+ if (error == -EBUSY)
+ return error;
}

if (error)
--
2.37.2


2023-01-13 21:09:30

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH] kernel: power: swap: Suppress expected 'Image not found' error on Ubuntu

On Thu, Dec 15, 2022 at 7:20 AM Shradha Gupta
<[email protected]> wrote:
>
> In 'systemctl hibernate' if resume device is written to the sysfs
> resume parameter, a software_resume() call is triggerred. This call
> is expected to fail in swsusp_check() call with -EBUSY error and an
> 'Image not found' error message. This fix suppresses the expected
> failure message from getting logged in Ubuntu setups where
> CONFIG_DYNAMIC_DEBUG is enabled by default.

I see what you mean, so what about this change instead (modulo
GMail-induced white-space breakage):

---
kernel/power/swap.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)

Index: linux-pm/kernel/power/swap.c
===================================================================
--- linux-pm.orig/kernel/power/swap.c
+++ linux-pm/kernel/power/swap.c
@@ -1546,17 +1546,16 @@ int swsusp_check(void)
}

put:
- if (error)
+ if (error) {
blkdev_put(hib_resume_bdev, FMODE_READ | FMODE_EXCL);
- else
+ pr_debug("Image not found (code %d)\n", error);
+ } else {
pr_debug("Image signature found, resuming\n");
+ }
} else {
error = PTR_ERR(hib_resume_bdev);
}

- if (error)
- pr_debug("Image not found (code %d)\n", error);
-
return error;
}

2023-01-14 00:34:42

by Dexuan Cui

[permalink] [raw]
Subject: RE: [PATCH] kernel: power: swap: Suppress expected 'Image not found' error on Ubuntu

> From: Rafael J. Wysocki <[email protected]>
> Sent: Friday, January 13, 2023 12:14 PM
> To: Shradha Gupta <[email protected]>
> Cc: [email protected]; [email protected]; Rafael J. Wysocki
> <[email protected]>; Pavel Machek <[email protected]>; Len Brown
> <[email protected]>; Dexuan Cui <[email protected]>; Michael Kelley
> (LINUX) <[email protected]>
> Subject: Re: [PATCH] kernel: power: swap: Suppress expected 'Image not
> found' error on Ubuntu
>
> On Thu, Dec 15, 2022 at 7:20 AM Shradha Gupta
> <[email protected]> wrote:
> >
> > In 'systemctl hibernate' if resume device is written to the sysfs
> > resume parameter, a software_resume() call is triggerred. This call
> > is expected to fail in swsusp_check() call with -EBUSY error and an
> > 'Image not found' error message. This fix suppresses the expected
> > failure message from getting logged in Ubuntu setups where
> > CONFIG_DYNAMIC_DEBUG is enabled by default.
>
> I see what you mean, so what about this change instead (modulo
> GMail-induced white-space breakage):
>
> ---
> kernel/power/swap.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> Index: linux-pm/kernel/power/swap.c
> ================================================================
> ===
> --- linux-pm.orig/kernel/power/swap.c
> +++ linux-pm/kernel/power/swap.c
> @@ -1546,17 +1546,16 @@ int swsusp_check(void)
> }
>
> put:
> - if (error)
> + if (error) {
> blkdev_put(hib_resume_bdev, FMODE_READ | FMODE_EXCL);
> - else
> + pr_debug("Image not found (code %d)\n", error);
> + } else {
> pr_debug("Image signature found, resuming\n");
> + }
> } else {
> error = PTR_ERR(hib_resume_bdev);
> }
>
> - if (error)
> - pr_debug("Image not found (code %d)\n", error);
> -
> return error;
> }

software_resume() -> swsusp_check() can be used in 2 scenarios:

1) After a VM hibernated and powered off, we start the VM, and the script
in initrd writes to the sys file 'resume', so resume_store() ->
software_resume() is called to resume the VM. In this scenario, if
software_resume() -> swsusp_check() -> blkdev_get_by_dev() hits an
error, we still want to see the message "Image not found", but with the
new proposed change, the message is not printed. With Shradha's change,
the message is printed.

2) When we run "systemctl hibernate", systemctl writes to the 'resume' sys
file, so resume_store() -> software_resume() -> swsusp_check() is called.
Here typically the swap file/partition is already in use as the swap space, so
the blkdev_get_by_dev() in swsusp_check() always fails with -EBUSY. This is
the scenario we want to suppress the misleading "Image not found" message.

IIUC, Shradha's change is better.