diff mbox series

[04/24] PM: hibernate: move finding the resume device out of software_resume

Message ID 20230523074535.249802-5-hch@lst.de (mailing list archive)
State Handled Elsewhere, archived
Headers show
Series [01/24] driver core: return bool from driver_probe_done | expand

Commit Message

Christoph Hellwig May 23, 2023, 7:45 a.m. UTC
software_resume can be called either from an init call in the boot code,
or from sysfs once the system has finished booting, and the two
invocation methods this can't race with each other.

For the latter case we did just parse the suspend device manually, while
the former might not have one.  Split software_resume so that the search
only happens for the boot case, which also means the special lockdep
nesting annotation can go away as the system transition mutex can be
taken a little later and doesn't have the sysfs locking nest inside it.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 kernel/power/hibernate.c | 80 ++++++++++++++++++++--------------------
 1 file changed, 39 insertions(+), 41 deletions(-)

Comments

Rafael J. Wysocki May 23, 2023, 6:33 p.m. UTC | #1
On Tue, May 23, 2023 at 9:45 AM Christoph Hellwig <hch@lst.de> wrote:
>
> software_resume can be called either from an init call in the boot code,
> or from sysfs once the system has finished booting, and the two
> invocation methods this can't race with each other.
>
> For the latter case we did just parse the suspend device manually, while
> the former might not have one.  Split software_resume so that the search
> only happens for the boot case, which also means the special lockdep
> nesting annotation can go away as the system transition mutex can be
> taken a little later and doesn't have the sysfs locking nest inside it.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Acked-by: Rafael J. Wysocki <rafael@kernel.org>

> ---
>  kernel/power/hibernate.c | 80 ++++++++++++++++++++--------------------
>  1 file changed, 39 insertions(+), 41 deletions(-)
>
> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
> index 78696aa04f5ca3..45e24b02cd50b6 100644
> --- a/kernel/power/hibernate.c
> +++ b/kernel/power/hibernate.c
> @@ -907,7 +907,7 @@ int hibernate_quiet_exec(int (*func)(void *data), void *data)
>  }
>  EXPORT_SYMBOL_GPL(hibernate_quiet_exec);
>
> -static int find_resume_device(void)
> +static int __init find_resume_device(void)
>  {
>         if (!strlen(resume_file))
>                 return -ENOENT;
> @@ -942,53 +942,16 @@ static int find_resume_device(void)
>         return 0;
>  }
>
> -/**
> - * software_resume - Resume from a saved hibernation image.
> - *
> - * This routine is called as a late initcall, when all devices have been
> - * discovered and initialized already.
> - *
> - * The image reading code is called to see if there is a hibernation image
> - * available for reading.  If that is the case, devices are quiesced and the
> - * contents of memory is restored from the saved image.
> - *
> - * If this is successful, control reappears in the restored target kernel in
> - * hibernation_snapshot() which returns to hibernate().  Otherwise, the routine
> - * attempts to recover gracefully and make the kernel return to the normal mode
> - * of operation.
> - */
>  static int software_resume(void)
>  {
>         int error;
>
> -       /*
> -        * If the user said "noresume".. bail out early.
> -        */
> -       if (noresume || !hibernation_available())
> -               return 0;
> -
> -       /*
> -        * name_to_dev_t() below takes a sysfs buffer mutex when sysfs
> -        * is configured into the kernel. Since the regular hibernate
> -        * trigger path is via sysfs which takes a buffer mutex before
> -        * calling hibernate functions (which take system_transition_mutex)
> -        * this can cause lockdep to complain about a possible ABBA deadlock
> -        * which cannot happen since we're in the boot code here and
> -        * sysfs can't be invoked yet. Therefore, we use a subclass
> -        * here to avoid lockdep complaining.
> -        */
> -       mutex_lock_nested(&system_transition_mutex, SINGLE_DEPTH_NESTING);
> -
> -       if (!swsusp_resume_device) {
> -               error = find_resume_device();
> -               if (error)
> -                       goto Unlock;
> -       }
> -
>         pm_pr_dbg("Hibernation image partition %d:%d present\n",
>                 MAJOR(swsusp_resume_device), MINOR(swsusp_resume_device));
>
>         pm_pr_dbg("Looking for hibernation image.\n");
> +
> +       mutex_lock(&system_transition_mutex);
>         error = swsusp_check(false);
>         if (error)
>                 goto Unlock;
> @@ -1035,7 +998,39 @@ static int software_resume(void)
>         goto Finish;
>  }
>
> -late_initcall_sync(software_resume);
> +/**
> + * software_resume_initcall - Resume from a saved hibernation image.
> + *
> + * This routine is called as a late initcall, when all devices have been
> + * discovered and initialized already.
> + *
> + * The image reading code is called to see if there is a hibernation image
> + * available for reading.  If that is the case, devices are quiesced and the
> + * contents of memory is restored from the saved image.
> + *
> + * If this is successful, control reappears in the restored target kernel in
> + * hibernation_snapshot() which returns to hibernate().  Otherwise, the routine
> + * attempts to recover gracefully and make the kernel return to the normal mode
> + * of operation.
> + */
> +static int __init software_resume_initcall(void)
> +{
> +       /*
> +        * If the user said "noresume".. bail out early.
> +        */
> +       if (noresume || !hibernation_available())
> +               return 0;
> +
> +       if (!swsusp_resume_device) {
> +               int error = find_resume_device();
> +
> +               if (error)
> +                       return error;
> +       }
> +
> +       return software_resume();
> +}
> +late_initcall_sync(software_resume_initcall);
>
>
>  static const char * const hibernation_modes[] = {
> @@ -1176,6 +1171,9 @@ static ssize_t resume_store(struct kobject *kobj, struct kobj_attribute *attr,
>         char *name;
>         dev_t res;
>
> +       if (!hibernation_available())
> +               return 0;
> +
>         if (len && buf[len-1] == '\n')
>                 len--;
>         name = kstrndup(buf, len, GFP_KERNEL);
> --
> 2.39.2
>
diff mbox series

Patch

diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index 78696aa04f5ca3..45e24b02cd50b6 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -907,7 +907,7 @@  int hibernate_quiet_exec(int (*func)(void *data), void *data)
 }
 EXPORT_SYMBOL_GPL(hibernate_quiet_exec);
 
-static int find_resume_device(void)
+static int __init find_resume_device(void)
 {
 	if (!strlen(resume_file))
 		return -ENOENT;
@@ -942,53 +942,16 @@  static int find_resume_device(void)
 	return 0;
 }
 
-/**
- * software_resume - Resume from a saved hibernation image.
- *
- * This routine is called as a late initcall, when all devices have been
- * discovered and initialized already.
- *
- * The image reading code is called to see if there is a hibernation image
- * available for reading.  If that is the case, devices are quiesced and the
- * contents of memory is restored from the saved image.
- *
- * If this is successful, control reappears in the restored target kernel in
- * hibernation_snapshot() which returns to hibernate().  Otherwise, the routine
- * attempts to recover gracefully and make the kernel return to the normal mode
- * of operation.
- */
 static int software_resume(void)
 {
 	int error;
 
-	/*
-	 * If the user said "noresume".. bail out early.
-	 */
-	if (noresume || !hibernation_available())
-		return 0;
-
-	/*
-	 * name_to_dev_t() below takes a sysfs buffer mutex when sysfs
-	 * is configured into the kernel. Since the regular hibernate
-	 * trigger path is via sysfs which takes a buffer mutex before
-	 * calling hibernate functions (which take system_transition_mutex)
-	 * this can cause lockdep to complain about a possible ABBA deadlock
-	 * which cannot happen since we're in the boot code here and
-	 * sysfs can't be invoked yet. Therefore, we use a subclass
-	 * here to avoid lockdep complaining.
-	 */
-	mutex_lock_nested(&system_transition_mutex, SINGLE_DEPTH_NESTING);
-
-	if (!swsusp_resume_device) {
-		error = find_resume_device();
-		if (error)
-			goto Unlock;
-	}
-
 	pm_pr_dbg("Hibernation image partition %d:%d present\n",
 		MAJOR(swsusp_resume_device), MINOR(swsusp_resume_device));
 
 	pm_pr_dbg("Looking for hibernation image.\n");
+
+	mutex_lock(&system_transition_mutex);
 	error = swsusp_check(false);
 	if (error)
 		goto Unlock;
@@ -1035,7 +998,39 @@  static int software_resume(void)
 	goto Finish;
 }
 
-late_initcall_sync(software_resume);
+/**
+ * software_resume_initcall - Resume from a saved hibernation image.
+ *
+ * This routine is called as a late initcall, when all devices have been
+ * discovered and initialized already.
+ *
+ * The image reading code is called to see if there is a hibernation image
+ * available for reading.  If that is the case, devices are quiesced and the
+ * contents of memory is restored from the saved image.
+ *
+ * If this is successful, control reappears in the restored target kernel in
+ * hibernation_snapshot() which returns to hibernate().  Otherwise, the routine
+ * attempts to recover gracefully and make the kernel return to the normal mode
+ * of operation.
+ */
+static int __init software_resume_initcall(void)
+{
+	/*
+	 * If the user said "noresume".. bail out early.
+	 */
+	if (noresume || !hibernation_available())
+		return 0;
+
+	if (!swsusp_resume_device) {
+		int error = find_resume_device();
+
+		if (error)
+			return error;
+	}
+
+	return software_resume();
+}
+late_initcall_sync(software_resume_initcall);
 
 
 static const char * const hibernation_modes[] = {
@@ -1176,6 +1171,9 @@  static ssize_t resume_store(struct kobject *kobj, struct kobj_attribute *attr,
 	char *name;
 	dev_t res;
 
+	if (!hibernation_available())
+		return 0;
+
 	if (len && buf[len-1] == '\n')
 		len--;
 	name = kstrndup(buf, len, GFP_KERNEL);