diff mbox series

[1/2] uswsusp: use enter/leave helpers and make a global variable static

Message ID 20200413193718.883336371@gmail.com (mailing list archive)
State Changes Requested, archived
Headers show
Series Preparing to phase out uswsusp | expand

Commit Message

Domenico Andreoli April 13, 2020, 7:08 p.m. UTC 
From: Domenico Andreoli <domenico.andreoli@linux.com>

Regulate access to the userspace software suspend state via helper
functions instead of a global variable.

Signed-off-by: Domenico Andreoli <domenico.andreoli@linux.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Linux PM <linux-pm@vger.kernel.org>

---
 kernel/power/hibernate.c |    8 ++++----
 kernel/power/power.h     |    6 +++---
 kernel/power/user.c      |   20 +++++++++++++++-----
 3 files changed, 22 insertions(+), 12 deletions(-)

Comments

Rafael J. Wysocki April 26, 2020, 4:11 p.m. UTC | #1 
On Monday, April 13, 2020 9:08:44 PM CEST Domenico Andreoli wrote:
> From: Domenico Andreoli <domenico.andreoli@linux.com>
> 
> Regulate access to the userspace software suspend state via helper
> functions instead of a global variable.

IMO this can be done regardless of the second patch.

Some minor naming nits below.

> Signed-off-by: Domenico Andreoli <domenico.andreoli@linux.com>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: Linux PM <linux-pm@vger.kernel.org>
> 
> ---
>  kernel/power/hibernate.c |    8 ++++----
>  kernel/power/power.h     |    6 +++---
>  kernel/power/user.c      |   20 +++++++++++++++-----
>  3 files changed, 22 insertions(+), 12 deletions(-)
> 
> Index: b/kernel/power/user.c
> ===================================================================
> --- a/kernel/power/user.c
> +++ b/kernel/power/user.c
> @@ -37,7 +37,17 @@ static struct snapshot_data {
>  	bool free_bitmaps;
>  } snapshot_state;
>  
> -atomic_t snapshot_device_available = ATOMIC_INIT(1);
> +static atomic_t snapshot_device_available = ATOMIC_INIT(1);
> +
> +bool swsusp_try_enter(void)

Why not to call it snapshot_device_acquire() ->

> +{
> +	return atomic_add_unless(&snapshot_device_available, -1, 0);
> +}
> +
> +void swsusp_leave(void)
> +{

-> and this snapshot_device_release(), or something to that effect?

"swsusp" is a legacy name.

Thanks!
diff mbox series

Patch

Index: b/kernel/power/user.c
===================================================================
--- a/kernel/power/user.c
+++ b/kernel/power/user.c
@@ -37,7 +37,17 @@  static struct snapshot_data {
 	bool free_bitmaps;
 } snapshot_state;
 
-atomic_t snapshot_device_available = ATOMIC_INIT(1);
+static atomic_t snapshot_device_available = ATOMIC_INIT(1);
+
+bool swsusp_try_enter(void)
+{
+	return atomic_add_unless(&snapshot_device_available, -1, 0);
+}
+
+void swsusp_leave(void)
+{
+	atomic_inc(&snapshot_device_available);
+}
 
 static int snapshot_open(struct inode *inode, struct file *filp)
 {
@@ -49,13 +59,13 @@  static int snapshot_open(struct inode *i
 
 	lock_system_sleep();
 
-	if (!atomic_add_unless(&snapshot_device_available, -1, 0)) {
+	if (!swsusp_try_enter()) {
 		error = -EBUSY;
 		goto Unlock;
 	}
 
 	if ((filp->f_flags & O_ACCMODE) == O_RDWR) {
-		atomic_inc(&snapshot_device_available);
+		swsusp_leave();
 		error = -ENOSYS;
 		goto Unlock;
 	}
@@ -92,7 +102,7 @@  static int snapshot_open(struct inode *i
 			__pm_notifier_call_chain(PM_POST_RESTORE, nr_calls, NULL);
 	}
 	if (error)
-		atomic_inc(&snapshot_device_available);
+		swsusp_leave();
 
 	data->frozen = false;
 	data->ready = false;
@@ -122,7 +132,7 @@  static int snapshot_release(struct inode
 	}
 	pm_notifier_call_chain(data->mode == O_RDONLY ?
 			PM_POST_HIBERNATION : PM_POST_RESTORE);
-	atomic_inc(&snapshot_device_available);
+	swsusp_leave();
 
 	unlock_system_sleep();
 
Index: b/kernel/power/power.h
===================================================================
--- a/kernel/power/power.h
+++ b/kernel/power/power.h
@@ -154,13 +154,13 @@  extern int snapshot_write_next(struct sn
 extern void snapshot_write_finalize(struct snapshot_handle *handle);
 extern int snapshot_image_loaded(struct snapshot_handle *handle);
 
-/* If unset, the snapshot device cannot be open. */
-extern atomic_t snapshot_device_available;
-
 extern sector_t alloc_swapdev_block(int swap);
 extern void free_all_swap_pages(int swap);
 extern int swsusp_swap_in_use(void);
 
+bool swsusp_try_enter(void);
+void swsusp_leave(void);
+
 /*
  * Flags that can be passed from the hibernatig hernel to the "boot" kernel in
  * the image header.
Index: b/kernel/power/hibernate.c
===================================================================
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -704,7 +704,7 @@  int hibernate(void)
 
 	lock_system_sleep();
 	/* The snapshot device should not be opened while we're running */
-	if (!atomic_add_unless(&snapshot_device_available, -1, 0)) {
+	if (!swsusp_try_enter()) {
 		error = -EBUSY;
 		goto Unlock;
 	}
@@ -775,7 +775,7 @@  int hibernate(void)
  Exit:
 	__pm_notifier_call_chain(PM_POST_HIBERNATION, nr_calls, NULL);
 	pm_restore_console();
-	atomic_inc(&snapshot_device_available);
+	swsusp_leave();
  Unlock:
 	unlock_system_sleep();
 	pr_info("hibernation exit\n");
@@ -880,7 +880,7 @@  static int software_resume(void)
 		goto Unlock;
 
 	/* The snapshot device should not be opened while we're running */
-	if (!atomic_add_unless(&snapshot_device_available, -1, 0)) {
+	if (!swsusp_try_enter()) {
 		error = -EBUSY;
 		swsusp_close(FMODE_READ);
 		goto Unlock;
@@ -904,7 +904,7 @@  static int software_resume(void)
 	__pm_notifier_call_chain(PM_POST_RESTORE, nr_calls, NULL);
 	pm_restore_console();
 	pr_info("resume failed (%d)\n", error);
-	atomic_inc(&snapshot_device_available);
+	swsusp_leave();
 	/* For success case, the suspend path will release the lock */
  Unlock:
 	mutex_unlock(&system_transition_mutex);