2003-07-26 22:43:23

by Pavel Machek

Subject: [PM] suspend.c cleanups

Hi!

These are only cleanups.
Pavel

Index: linux/kernel/suspend.c
===================================================================
--- linux.orig/kernel/suspend.c 2003-07-22 13:39:43.000000000 +0200
+++ linux/kernel/suspend.c 2003-07-22 13:46:26.000000000 +0200
@@ -139,40 +139,45 @@

static const char name_suspend[] = "Suspend Machine: ";
static const char name_resume[] = "Resume Machine: ";
#endif

/*
* Debug
*/
-#define DEBUG_DEFAULT
-#undef DEBUG_PROCESS
+#undef DEBUG_DEFAULT
#undef DEBUG_SLOW
-#define TEST_SWSUSP 0 /* Set to 1 to reboot instead of halt machine after suspension */
+#define TEST_SWSUSP 1 /* Set to 1 to reboot instead of halt machine after suspension */

#ifdef DEBUG_DEFAULT
-# define PRINTK(f, a...) printk(f, ## a)
+# define PRINTK(f, a...) printk(f, ## a)
#else
-# define PRINTK(f, a...)
+# define PRINTK(f, a...) do {} while (0)
#endif

#ifdef DEBUG_SLOW
#define MDELAY(a) mdelay(a)
#else
-#define MDELAY(a)
+#define MDELAY(a) do {} while (0)
#endif

@@ -283,17 +329,6 @@
return 0;
}

-/*
- * This is our sync function. With this solution we probably won't sleep
- * but that should not be a problem since tasks are stopped..
- */
-
-static inline void do_suspend_sync(void)
-{
- blk_run_queues();
-#warning This might be broken. We need to somehow wait for data to reach the disk
-}
-
/* We memorize in swapfile_used what swap devices are used for suspension */
#define SWAPFILE_UNUSED 0
#define SWAPFILE_SUSPEND 1 /* This is the suspending device */
@@ -861,20 +861,13 @@
if (prepare_suspend_console())
printk( "%sCan't allocate a console... proceeding\n", name_suspend);
if (!prepare_suspend_processes()) {
-
/* At this point, all user processes and "dangerous"
kernel threads are stopped. Free some memory, as we
need half of memory free. */
-
free_some_memory();

- /* No need to invalidate any vfsmnt list -- they will be valid after resume, anyway.
- *
- * We sync here -- so you have consistent filesystem state when things go wrong.
- * -- so that noone writes to disk after we do atomic copy of data.
- */
- PRINTK("Syncing disks before copy\n");
- do_suspend_sync();
+ /* No need to invalidate any vfsmnt list -- they will be valid after resume, anyway. */
+ blk_run_queues();

/* Save state of all device drivers, and stop them. */
if(drivers_suspend()==0)
@@ -906,7 +898,7 @@
return;

software_suspend_enabled = 0;
- BUG_ON(in_interrupt());
+ BUG_ON(in_atomic());
do_software_suspend();
}



--
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

2003-08-05 00:56:01

by Patrick Mochel

Subject: Re: [PM] suspend.c cleanups

> These are only cleanups.
> Pavel
>
> Index: linux/kernel/suspend.c
> ===================================================================
> --- linux.orig/kernel/suspend.c 2003-07-22 13:39:43.000000000 +0200
> +++ linux/kernel/suspend.c 2003-07-22 13:46:26.000000000 +0200
> @@ -139,40 +139,45 @@
>
> static const char name_suspend[] = "Suspend Machine: ";
> static const char name_resume[] = "Resume Machine: ";
> #endif
>
> /*
> * Debug
> */
> -#define DEBUG_DEFAULT
> -#undef DEBUG_PROCESS
> +#undef DEBUG_DEFAULT
> #undef DEBUG_SLOW
> -#define TEST_SWSUSP 0 /* Set to 1 to reboot instead of halt machine after suspension */
> +#define TEST_SWSUSP 1 /* Set to 1 to reboot instead of halt machine after suspension */

This is not a cleanup, it changes behavior, so I didn't apply this first
part (since I had to make the other changes by hand anyway).

> @@ -283,17 +329,6 @@

This part was applied.

> @@ -906,7 +898,7 @@
> return;
>
> software_suspend_enabled = 0;
> - BUG_ON(in_interrupt());
> + BUG_ON(in_atomic());
> do_software_suspend();
> }

I replaced the BUG() with might_sleep(), since it will produce a stack
trace, and is a bit friendlier.

Thanks,


-pat

2003-08-05 09:17:00

by Pavel Machek

Subject: Re: [PM] suspend.c cleanups

Hi!

> > -#define TEST_SWSUSP 0 /* Set to 1 to reboot instead of halt machine after suspension */
> > +#define TEST_SWSUSP 1 /* Set to 1 to reboot instead of halt machine after suspension */
>
> This is not a cleanup, it changes behavior, so I didn't apply this first
> part (since I had to make the other changes by hand anyway).

Sorry for that.

> > @@ -906,7 +898,7 @@
> > return;
> >
> > software_suspend_enabled = 0;
> > - BUG_ON(in_interrupt());
> > + BUG_ON(in_atomic());
> > do_software_suspend();
> > }
>
> I replaced the BUG() with might_sleep(), since it will produce a stack
> trace, and is a bit friendlier.

Yes, thats better.
Pavel

--
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]