xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 4/5] PM: Add visible HIBERNATION_INTERFACE and hide HIBERNATION
@ 2011-03-12  5:07 Shriram Rajagopalan
  2011-03-15 21:25 ` Shriram Rajagopalan
  2011-03-15 21:38 ` Rafael J. Wysocki
  0 siblings, 2 replies; 11+ messages in thread
From: Shriram Rajagopalan @ 2011-03-12  5:07 UTC (permalink / raw)
  To: xen-devel; +Cc: Shriram Rajagopalan, linux-pm

HIBERNATION covers the main hibernation control code and freeze-thaw
pm events, that xen's save/restore also uses. Explicitly enabling
an independant hibernation functionality to enable xen's save/restore
is a bit ugly. Define a new user visible symbol HIBERNATION_INTERFACE
that "selects" HIBERNATION and covers the main hibernation control code
instead of HIBERNATION. This way, we can also make XEN_SAVE_RESTORE
"select" HIBERNATION, enabling only the freeze-thaw code.

Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>
---
 kernel/power/Kconfig     |    9 +++++++--
 kernel/power/hibernate.c |    4 ++++
 kernel/power/main.c      |    2 +-
 kernel/power/user.c      |    2 ++
 4 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
index 4603f08..493c678 100644
--- a/kernel/power/Kconfig
+++ b/kernel/power/Kconfig
@@ -19,10 +19,15 @@ config SUSPEND_FREEZER
 	  Turning OFF this setting is NOT recommended! If in doubt, say Y.
 
 config HIBERNATION
-	bool "Hibernation (aka 'suspend to disk')"
-	depends on SWAP && ARCH_HIBERNATION_POSSIBLE
+	def_bool n
+	depends on ARCH_HIBERNATION_POSSIBLE
 	select LZO_COMPRESS
 	select LZO_DECOMPRESS
+
+config HIBERNATION_INTERFACE
+	bool "Hibernation (aka 'suspend to disk')"
+	depends on SWAP
+	select HIBERNATION
 	---help---
 	  Enable the suspend to disk (STD) functionality, which is usually
 	  called "hibernation" in user interfaces.  STD checkpoints the
diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index 1832bd2..13bcf69 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -592,6 +592,7 @@ static int prepare_processes(void)
  *	hibernate - The granpappy of the built-in hibernation management
  */
 
+#ifdef CONFIG_HIBERNATION_INTERFACE
 int hibernate(void)
 {
 	int error;
@@ -667,6 +668,8 @@ int hibernate(void)
 	return error;
 }
 
+#else /* !CONFIG_HIBERNATION_INTERFACE */
+int hibernate(void) { return -ENOSYS; }
 
 /**
  *	software_resume - Resume from a saved image.
@@ -1029,3 +1032,4 @@ __setup("noresume", noresume_setup);
 __setup("resume_offset=", resume_offset_setup);
 __setup("resume=", resume_setup);
 __setup("hibernate=", hibernate_setup);
+#endif /* !CONFIG_HIBERNATION_INTERFACE */
diff --git a/kernel/power/main.c b/kernel/power/main.c
index 8eaba5f..686a130 100644
--- a/kernel/power/main.c
+++ b/kernel/power/main.c
@@ -156,7 +156,7 @@ static ssize_t state_show(struct kobject *kobj, struct kobj_attribute *attr,
 			s += sprintf(s,"%s ", pm_states[i]);
 	}
 #endif
-#ifdef CONFIG_HIBERNATION
+#ifdef CONFIG_HIBERNATION_INTERFACE
 	s += sprintf(s, "%s\n", "disk");
 #else
 	if (s != buf)
diff --git a/kernel/power/user.c b/kernel/power/user.c
index c36c3b9..5f36ee7 100644
--- a/kernel/power/user.c
+++ b/kernel/power/user.c
@@ -458,6 +458,7 @@ static long snapshot_ioctl(struct file *filp, unsigned int cmd,
 	return error;
 }
 
+#ifdef CONFIG_HIBERNATION_INTERFACE
 static const struct file_operations snapshot_fops = {
 	.open = snapshot_open,
 	.release = snapshot_release,
@@ -479,3 +480,4 @@ static int __init snapshot_device_init(void)
 };
 
 device_initcall(snapshot_device_init);
+#endif /* CONFIG_HIBERNATION_INTERFACE */
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 4/5] PM: Add visible HIBERNATION_INTERFACE and hide HIBERNATION
  2011-03-12  5:07 [PATCH v3 4/5] PM: Add visible HIBERNATION_INTERFACE and hide HIBERNATION Shriram Rajagopalan
@ 2011-03-15 21:25 ` Shriram Rajagopalan
  2011-03-15 21:39   ` Rafael J. Wysocki
  2011-03-15 21:38 ` Rafael J. Wysocki
  1 sibling, 1 reply; 11+ messages in thread
From: Shriram Rajagopalan @ 2011-03-15 21:25 UTC (permalink / raw)
  To: rjw; +Cc: Shriram Rajagopalan, linux-pm, xen-devel, Konrad Rzeszutek Wilk

On Fri, Mar 11, 2011 at 9:07 PM, Shriram Rajagopalan <rshriram@cs.ubc.ca> wrote:
> HIBERNATION covers the main hibernation control code and freeze-thaw
> pm events, that xen's save/restore also uses. Explicitly enabling
> an independant hibernation functionality to enable xen's save/restore
> is a bit ugly. Define a new user visible symbol HIBERNATION_INTERFACE
> that "selects" HIBERNATION and covers the main hibernation control code
> instead of HIBERNATION. This way, we can also make XEN_SAVE_RESTORE
> "select" HIBERNATION, enabling only the freeze-thaw code.
>
> Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>
> ---
>  kernel/power/Kconfig     |    9 +++++++--
>  kernel/power/hibernate.c |    4 ++++
>  kernel/power/main.c      |    2 +-
>  kernel/power/user.c      |    2 ++
>  4 files changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
> index 4603f08..493c678 100644
> --- a/kernel/power/Kconfig
> +++ b/kernel/power/Kconfig
> @@ -19,10 +19,15 @@ config SUSPEND_FREEZER
>          Turning OFF this setting is NOT recommended! If in doubt, say Y.
>
>  config HIBERNATION
> -       bool "Hibernation (aka 'suspend to disk')"
> -       depends on SWAP && ARCH_HIBERNATION_POSSIBLE
> +       def_bool n
> +       depends on ARCH_HIBERNATION_POSSIBLE
>        select LZO_COMPRESS
>        select LZO_DECOMPRESS
> +
> +config HIBERNATION_INTERFACE
> +       bool "Hibernation (aka 'suspend to disk')"
> +       depends on SWAP
> +       select HIBERNATION
>        ---help---
>          Enable the suspend to disk (STD) functionality, which is usually
>          called "hibernation" in user interfaces.  STD checkpoints the
> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
> index 1832bd2..13bcf69 100644
> --- a/kernel/power/hibernate.c
> +++ b/kernel/power/hibernate.c
> @@ -592,6 +592,7 @@ static int prepare_processes(void)
>  *     hibernate - The granpappy of the built-in hibernation management
>  */
>
> +#ifdef CONFIG_HIBERNATION_INTERFACE
>  int hibernate(void)
>  {
>        int error;
> @@ -667,6 +668,8 @@ int hibernate(void)
>        return error;
>  }
>
> +#else /* !CONFIG_HIBERNATION_INTERFACE */
> +int hibernate(void) { return -ENOSYS; }
>
>  /**
>  *     software_resume - Resume from a saved image.
> @@ -1029,3 +1032,4 @@ __setup("noresume", noresume_setup);
>  __setup("resume_offset=", resume_offset_setup);
>  __setup("resume=", resume_setup);
>  __setup("hibernate=", hibernate_setup);
> +#endif /* !CONFIG_HIBERNATION_INTERFACE */
> diff --git a/kernel/power/main.c b/kernel/power/main.c
> index 8eaba5f..686a130 100644
> --- a/kernel/power/main.c
> +++ b/kernel/power/main.c
> @@ -156,7 +156,7 @@ static ssize_t state_show(struct kobject *kobj, struct kobj_attribute *attr,
>                        s += sprintf(s,"%s ", pm_states[i]);
>        }
>  #endif
> -#ifdef CONFIG_HIBERNATION
> +#ifdef CONFIG_HIBERNATION_INTERFACE
>        s += sprintf(s, "%s\n", "disk");
>  #else
>        if (s != buf)
> diff --git a/kernel/power/user.c b/kernel/power/user.c
> index c36c3b9..5f36ee7 100644
> --- a/kernel/power/user.c
> +++ b/kernel/power/user.c
> @@ -458,6 +458,7 @@ static long snapshot_ioctl(struct file *filp, unsigned int cmd,
>        return error;
>  }
>
> +#ifdef CONFIG_HIBERNATION_INTERFACE
>  static const struct file_operations snapshot_fops = {
>        .open = snapshot_open,
>        .release = snapshot_release,
> @@ -479,3 +480,4 @@ static int __init snapshot_device_init(void)
>  };
>
>  device_initcall(snapshot_device_init);
> +#endif /* CONFIG_HIBERNATION_INTERFACE */
> --
> 1.7.0.4
>
>

Rafael, do you have any objections to this patch?

As discussed earlier
http://lists.xensource.com/archives/html/xen-devel/2011-03/msg00267.html
these patches are against a merged tree (your linux-next and stefano's
linux-next).
Konrad would pull this tree into his branch and push it in the end
(after your & stefano's trees
have gone in).

Alternatively, if you would like to carry patches 3/5 and 4/5 directly
in your tree, then the other
patches could go into the xen tree.

shriram

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 4/5] PM: Add visible HIBERNATION_INTERFACE and hide HIBERNATION
  2011-03-12  5:07 [PATCH v3 4/5] PM: Add visible HIBERNATION_INTERFACE and hide HIBERNATION Shriram Rajagopalan
  2011-03-15 21:25 ` Shriram Rajagopalan
@ 2011-03-15 21:38 ` Rafael J. Wysocki
  2011-03-15 22:16   ` Shriram Rajagopalan
  1 sibling, 1 reply; 11+ messages in thread
From: Rafael J. Wysocki @ 2011-03-15 21:38 UTC (permalink / raw)
  To: Shriram Rajagopalan; +Cc: linux-pm, xen-devel

Hi,

Sorry, I didn't have the time to review the patch in detail before.

On Saturday, March 12, 2011, Shriram Rajagopalan wrote:
> HIBERNATION covers the main hibernation control code and freeze-thaw
> pm events, that xen's save/restore also uses. Explicitly enabling
> an independant hibernation functionality to enable xen's save/restore
> is a bit ugly. Define a new user visible symbol HIBERNATION_INTERFACE
> that "selects" HIBERNATION and covers the main hibernation control code
> instead of HIBERNATION. This way, we can also make XEN_SAVE_RESTORE
> "select" HIBERNATION, enabling only the freeze-thaw code.
> 
> Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>
> ---
>  kernel/power/Kconfig     |    9 +++++++--
>  kernel/power/hibernate.c |    4 ++++
>  kernel/power/main.c      |    2 +-
>  kernel/power/user.c      |    2 ++
>  4 files changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
> index 4603f08..493c678 100644
> --- a/kernel/power/Kconfig
> +++ b/kernel/power/Kconfig
> @@ -19,10 +19,15 @@ config SUSPEND_FREEZER
>  	  Turning OFF this setting is NOT recommended! If in doubt, say Y.
>  
>  config HIBERNATION
> -	bool "Hibernation (aka 'suspend to disk')"
> -	depends on SWAP && ARCH_HIBERNATION_POSSIBLE
> +	def_bool n

You don't need that.  Simply use "bool"

> +	depends on ARCH_HIBERNATION_POSSIBLE

That need not depend on it, HIBERNATION_INTERFACE should instead.

>  	select LZO_COMPRESS
>  	select LZO_DECOMPRESS

These two selects may be moved to HIBERNATION_INTERFACE too.

> +
> +config HIBERNATION_INTERFACE
> +	bool "Hibernation (aka 'suspend to disk')"
> +	depends on SWAP
> +	select HIBERNATION
>  	---help---
>  	  Enable the suspend to disk (STD) functionality, which is usually
>  	  called "hibernation" in user interfaces.  STD checkpoints the
> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
> index 1832bd2..13bcf69 100644
> --- a/kernel/power/hibernate.c
> +++ b/kernel/power/hibernate.c
> @@ -592,6 +592,7 @@ static int prepare_processes(void)
>   *	hibernate - The granpappy of the built-in hibernation management
>   */
>  
> +#ifdef CONFIG_HIBERNATION_INTERFACE

Please don't do that.  Please make the whole hibernate.c depend on
CONFIG_HIBERNATION_INTERFACE.

>  int hibernate(void)
>  {
>  	int error;
> @@ -667,6 +668,8 @@ int hibernate(void)
>  	return error;
>  }
>  
> +#else /* !CONFIG_HIBERNATION_INTERFACE */
> +int hibernate(void) { return -ENOSYS; }

And move that to a header.

>  /**
>   *	software_resume - Resume from a saved image.
> @@ -1029,3 +1032,4 @@ __setup("noresume", noresume_setup);
>  __setup("resume_offset=", resume_offset_setup);
>  __setup("resume=", resume_setup);
>  __setup("hibernate=", hibernate_setup);
> +#endif /* !CONFIG_HIBERNATION_INTERFACE */
> diff --git a/kernel/power/main.c b/kernel/power/main.c
> index 8eaba5f..686a130 100644
> --- a/kernel/power/main.c
> +++ b/kernel/power/main.c
> @@ -156,7 +156,7 @@ static ssize_t state_show(struct kobject *kobj, struct kobj_attribute *attr,
>  			s += sprintf(s,"%s ", pm_states[i]);
>  	}
>  #endif
> -#ifdef CONFIG_HIBERNATION
> +#ifdef CONFIG_HIBERNATION_INTERFACE
>  	s += sprintf(s, "%s\n", "disk");
>  #else
>  	if (s != buf)
> diff --git a/kernel/power/user.c b/kernel/power/user.c
> index c36c3b9..5f36ee7 100644
> --- a/kernel/power/user.c
> +++ b/kernel/power/user.c
> @@ -458,6 +458,7 @@ static long snapshot_ioctl(struct file *filp, unsigned int cmd,
>  	return error;
>  }
>  
> +#ifdef CONFIG_HIBERNATION_INTERFACE
>  static const struct file_operations snapshot_fops = {
>  	.open = snapshot_open,
>  	.release = snapshot_release,
> @@ -479,3 +480,4 @@ static int __init snapshot_device_init(void)
>  };
>  
>  device_initcall(snapshot_device_init);
> +#endif /* CONFIG_HIBERNATION_INTERFACE */
> 

Again, please make the entire user.c depend on CONFIG_HIBERNATION_INTERFACE
instead of adding #ifdefs to the file.

Thanks,
Rafael

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 4/5] PM: Add visible HIBERNATION_INTERFACE and hide HIBERNATION
  2011-03-15 21:25 ` Shriram Rajagopalan
@ 2011-03-15 21:39   ` Rafael J. Wysocki
  0 siblings, 0 replies; 11+ messages in thread
From: Rafael J. Wysocki @ 2011-03-15 21:39 UTC (permalink / raw)
  To: rshriram; +Cc: linux-pm, xen-devel, Konrad Rzeszutek Wilk

On Tuesday, March 15, 2011, Shriram Rajagopalan wrote:
> On Fri, Mar 11, 2011 at 9:07 PM, Shriram Rajagopalan <rshriram@cs.ubc.ca> wrote:
> > HIBERNATION covers the main hibernation control code and freeze-thaw
> > pm events, that xen's save/restore also uses. Explicitly enabling
> > an independant hibernation functionality to enable xen's save/restore
> > is a bit ugly. Define a new user visible symbol HIBERNATION_INTERFACE
> > that "selects" HIBERNATION and covers the main hibernation control code
> > instead of HIBERNATION. This way, we can also make XEN_SAVE_RESTORE
> > "select" HIBERNATION, enabling only the freeze-thaw code.
> >
> > Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>
> > ---
> >  kernel/power/Kconfig     |    9 +++++++--
> >  kernel/power/hibernate.c |    4 ++++
> >  kernel/power/main.c      |    2 +-
> >  kernel/power/user.c      |    2 ++
> >  4 files changed, 14 insertions(+), 3 deletions(-)
> >
> > diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
> > index 4603f08..493c678 100644
> > --- a/kernel/power/Kconfig
> > +++ b/kernel/power/Kconfig
> > @@ -19,10 +19,15 @@ config SUSPEND_FREEZER
> >          Turning OFF this setting is NOT recommended! If in doubt, say Y.
> >
> >  config HIBERNATION
> > -       bool "Hibernation (aka 'suspend to disk')"
> > -       depends on SWAP && ARCH_HIBERNATION_POSSIBLE
> > +       def_bool n
> > +       depends on ARCH_HIBERNATION_POSSIBLE
> >        select LZO_COMPRESS
> >        select LZO_DECOMPRESS
> > +
> > +config HIBERNATION_INTERFACE
> > +       bool "Hibernation (aka 'suspend to disk')"
> > +       depends on SWAP
> > +       select HIBERNATION
> >        ---help---
> >          Enable the suspend to disk (STD) functionality, which is usually
> >          called "hibernation" in user interfaces.  STD checkpoints the
> > diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
> > index 1832bd2..13bcf69 100644
> > --- a/kernel/power/hibernate.c
> > +++ b/kernel/power/hibernate.c
> > @@ -592,6 +592,7 @@ static int prepare_processes(void)
> >  *     hibernate - The granpappy of the built-in hibernation management
> >  */
> >
> > +#ifdef CONFIG_HIBERNATION_INTERFACE
> >  int hibernate(void)
> >  {
> >        int error;
> > @@ -667,6 +668,8 @@ int hibernate(void)
> >        return error;
> >  }
> >
> > +#else /* !CONFIG_HIBERNATION_INTERFACE */
> > +int hibernate(void) { return -ENOSYS; }
> >
> >  /**
> >  *     software_resume - Resume from a saved image.
> > @@ -1029,3 +1032,4 @@ __setup("noresume", noresume_setup);
> >  __setup("resume_offset=", resume_offset_setup);
> >  __setup("resume=", resume_setup);
> >  __setup("hibernate=", hibernate_setup);
> > +#endif /* !CONFIG_HIBERNATION_INTERFACE */
> > diff --git a/kernel/power/main.c b/kernel/power/main.c
> > index 8eaba5f..686a130 100644
> > --- a/kernel/power/main.c
> > +++ b/kernel/power/main.c
> > @@ -156,7 +156,7 @@ static ssize_t state_show(struct kobject *kobj, struct kobj_attribute *attr,
> >                        s += sprintf(s,"%s ", pm_states[i]);
> >        }
> >  #endif
> > -#ifdef CONFIG_HIBERNATION
> > +#ifdef CONFIG_HIBERNATION_INTERFACE
> >        s += sprintf(s, "%s\n", "disk");
> >  #else
> >        if (s != buf)
> > diff --git a/kernel/power/user.c b/kernel/power/user.c
> > index c36c3b9..5f36ee7 100644
> > --- a/kernel/power/user.c
> > +++ b/kernel/power/user.c
> > @@ -458,6 +458,7 @@ static long snapshot_ioctl(struct file *filp, unsigned int cmd,
> >        return error;
> >  }
> >
> > +#ifdef CONFIG_HIBERNATION_INTERFACE
> >  static const struct file_operations snapshot_fops = {
> >        .open = snapshot_open,
> >        .release = snapshot_release,
> > @@ -479,3 +480,4 @@ static int __init snapshot_device_init(void)
> >  };
> >
> >  device_initcall(snapshot_device_init);
> > +#endif /* CONFIG_HIBERNATION_INTERFACE */
> > --
> > 1.7.0.4
> >
> >
> 
> Rafael, do you have any objections to this patch?

Actually, I do.  Please see the comments I've just sent in a reply to the
patch itself.

> As discussed earlier
> http://lists.xensource.com/archives/html/xen-devel/2011-03/msg00267.html
> these patches are against a merged tree (your linux-next and stefano's
> linux-next).
> Konrad would pull this tree into his branch and push it in the end
> (after your & stefano's trees
> have gone in).
> 
> Alternatively, if you would like to carry patches 3/5 and 4/5 directly
> in your tree, then the other
> patches could go into the xen tree.

I'd prefer that, once 4/5 has been modified as requested.

Thanks,
Rafael

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 4/5] PM: Add visible HIBERNATION_INTERFACE and hide HIBERNATION
  2011-03-15 21:38 ` Rafael J. Wysocki
@ 2011-03-15 22:16   ` Shriram Rajagopalan
  2011-03-16  0:53     ` Rafael J. Wysocki
  0 siblings, 1 reply; 11+ messages in thread
From: Shriram Rajagopalan @ 2011-03-15 22:16 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: linux-pm, xen-devel

2011/3/15 Rafael J. Wysocki <rjw@sisk.pl>:
> Hi,
>
> Sorry, I didn't have the time to review the patch in detail before.
>
> On Saturday, March 12, 2011, Shriram Rajagopalan wrote:
>> HIBERNATION covers the main hibernation control code and freeze-thaw
>> pm events, that xen's save/restore also uses. Explicitly enabling
>> an independant hibernation functionality to enable xen's save/restore
>> is a bit ugly. Define a new user visible symbol HIBERNATION_INTERFACE
>> that "selects" HIBERNATION and covers the main hibernation control code
>> instead of HIBERNATION. This way, we can also make XEN_SAVE_RESTORE
>> "select" HIBERNATION, enabling only the freeze-thaw code.
>>
>> Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>
>> ---
>>  kernel/power/Kconfig     |    9 +++++++--
>>  kernel/power/hibernate.c |    4 ++++
>>  kernel/power/main.c      |    2 +-
>>  kernel/power/user.c      |    2 ++
>>  4 files changed, 14 insertions(+), 3 deletions(-)
>>
>> diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
>> index 4603f08..493c678 100644
>> --- a/kernel/power/Kconfig
>> +++ b/kernel/power/Kconfig
>> @@ -19,10 +19,15 @@ config SUSPEND_FREEZER
>>         Turning OFF this setting is NOT recommended! If in doubt, say Y.
>>
>>  config HIBERNATION
>> -     bool "Hibernation (aka 'suspend to disk')"
>> -     depends on SWAP && ARCH_HIBERNATION_POSSIBLE
>> +     def_bool n
>
> You don't need that.  Simply use "bool"
>
>> +     depends on ARCH_HIBERNATION_POSSIBLE
>
> That need not depend on it, HIBERNATION_INTERFACE should instead.
>
Does that mean the pm_ops interface does not depend on the ARCH_HIB.. ?
>>       select LZO_COMPRESS
>>       select LZO_DECOMPRESS
>
> These two selects may be moved to HIBERNATION_INTERFACE too.
>
>> +
>> +config HIBERNATION_INTERFACE
>> +     bool "Hibernation (aka 'suspend to disk')"
>> +     depends on SWAP
>> +     select HIBERNATION
>>       ---help---
>>         Enable the suspend to disk (STD) functionality, which is usually
>>         called "hibernation" in user interfaces.  STD checkpoints the
>> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
>> index 1832bd2..13bcf69 100644
>> --- a/kernel/power/hibernate.c
>> +++ b/kernel/power/hibernate.c
>> @@ -592,6 +592,7 @@ static int prepare_processes(void)
>>   *   hibernate - The granpappy of the built-in hibernation management
>>   */
>>
>> +#ifdef CONFIG_HIBERNATION_INTERFACE
>
> Please don't do that.  Please make the whole hibernate.c depend on
> CONFIG_HIBERNATION_INTERFACE.
>
This #if only wraps the hibernate() function. There is a whole lot of code
(global functions too) before the hibernate() function.

As i mentioned in the first email 0/5, simply making
hibernate.c and user.c depend on HIBERNATION_INTERFACE does not work.

I got the following linker errors.

kernel/built-in.o: In function `sys_reboot':
kernel/sys.c:440: undefined reference to `hibernate'
kernel/built-in.o: In function `state_store':
kernel/power/main.c:185: undefined reference to `hibernate'
kernel/built-in.o: In function `hibernate_preallocate_memory':
kernel/power/snapshot.c:1411: undefined reference to `swsusp_show_speed'
kernel/built-in.o: In function `swsusp_check':
kernel/power/swap.c:933: undefined reference to `swsusp_resume_device'
kernel/power/swap.c:938: undefined reference to `swsusp_resume_block'
kernel/power/swap.c:946: undefined reference to `swsusp_resume_block'
kernel/built-in.o: In function `load_image_lzo':
kernel/power/swap.c:878: undefined reference to `swsusp_show_speed'
kernel/built-in.o: In function `load_image':
kernel/power/swap.c:741: undefined reference to `swsusp_show_speed'
kernel/built-in.o: In function `save_image_lzo':
kernel/power/swap.c:502: undefined reference to `lzo1x_1_compress'
kernel/power/swap.c:543: undefined reference to `swsusp_show_speed'
kernel/built-in.o: In function `swsusp_swap_check':
kernel/power/swap.c:221: undefined reference to `swsusp_resume_block'
kernel/power/swap.c:221: undefined reference to `swsusp_resume_device'
kernel/built-in.o: In function `mark_swapfiles':
kernel/power/swap.c:195: undefined reference to `swsusp_resume_block'
kernel/power/swap.c:202: undefined reference to `swsusp_resume_block'
kernel/built-in.o: In function `save_image':
kernel/power/swap.c:418: undefined reference to `swsusp_show_speed'
drivers/built-in.o: In function `acpi_suspend':
drivers/acpi/sleep.c:584: undefined reference to `hibernate'
drivers/built-in.o: In function `ata_scsi_start_stop_xlat':
drivers/ata/libata-scsi.c:1331: undefined reference to
`system_entering_hibernation'
drivers/built-in.o: In function `acpi_sleep_init':
drivers/acpi/sleep.c:782: undefined reference to `hibernation_set_ops'
arch/x86/power/built-in.o: In function `restore_registers':
arch/x86/power/hibernate_asm_64.S:145: undefined reference to `in_suspend'

Seeing errors from arch/.. was also the reason why I made HIBERNATE depend on
ARCH_HIBERNATION_POSSIBLE

>>  int hibernate(void)
>>  {
>>       int error;
>> @@ -667,6 +668,8 @@ int hibernate(void)
>>       return error;
>>  }
>>
>> +#else /* !CONFIG_HIBERNATION_INTERFACE */
>> +int hibernate(void) { return -ENOSYS; }
>
> And move that to a header.
was done for the same reason as above. linux/suspend.h
has a similar definition but it encompasses a whole set of
swsusp_* functions, register_nosave stuff. But defining alternate
versions there means hibernate.c, snapshot.c etc have to depend on
HIBERNATION_INTERFACE - which causes errors stated above.

>
>>  /**
>>   *   software_resume - Resume from a saved image.
>> @@ -1029,3 +1032,4 @@ __setup("noresume", noresume_setup);
>>  __setup("resume_offset=", resume_offset_setup);
>>  __setup("resume=", resume_setup);
>>  __setup("hibernate=", hibernate_setup);
>> +#endif /* !CONFIG_HIBERNATION_INTERFACE */
>> diff --git a/kernel/power/main.c b/kernel/power/main.c
>> index 8eaba5f..686a130 100644
>> --- a/kernel/power/main.c
>> +++ b/kernel/power/main.c
>> @@ -156,7 +156,7 @@ static ssize_t state_show(struct kobject *kobj, struct kobj_attribute *attr,
>>                       s += sprintf(s,"%s ", pm_states[i]);
>>       }
>>  #endif
>> -#ifdef CONFIG_HIBERNATION
>> +#ifdef CONFIG_HIBERNATION_INTERFACE
>>       s += sprintf(s, "%s\n", "disk");
>>  #else
>>       if (s != buf)
>> diff --git a/kernel/power/user.c b/kernel/power/user.c
>> index c36c3b9..5f36ee7 100644
>> --- a/kernel/power/user.c
>> +++ b/kernel/power/user.c
>> @@ -458,6 +458,7 @@ static long snapshot_ioctl(struct file *filp, unsigned int cmd,
>>       return error;
>>  }
>>
>> +#ifdef CONFIG_HIBERNATION_INTERFACE
>>  static const struct file_operations snapshot_fops = {
>>       .open = snapshot_open,
>>       .release = snapshot_release,
>> @@ -479,3 +480,4 @@ static int __init snapshot_device_init(void)
>>  };
>>
>>  device_initcall(snapshot_device_init);
>> +#endif /* CONFIG_HIBERNATION_INTERFACE */
>>
>
> Again, please make the entire user.c depend on CONFIG_HIBERNATION_INTERFACE
> instead of adding #ifdefs to the file.
>
As above.

> Thanks,
> Rafael
>
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 4/5] PM: Add visible HIBERNATION_INTERFACE and hide HIBERNATION
  2011-03-15 22:16   ` Shriram Rajagopalan
@ 2011-03-16  0:53     ` Rafael J. Wysocki
  2011-03-18 21:36       ` [linux-pm] " Rafael J. Wysocki
  0 siblings, 1 reply; 11+ messages in thread
From: Rafael J. Wysocki @ 2011-03-16  0:53 UTC (permalink / raw)
  To: rshriram; +Cc: linux-pm, xen-devel

On Tuesday, March 15, 2011, Shriram Rajagopalan wrote:
> 2011/3/15 Rafael J. Wysocki <rjw@sisk.pl>:
> > Hi,
> >
> > Sorry, I didn't have the time to review the patch in detail before.
> >
> > On Saturday, March 12, 2011, Shriram Rajagopalan wrote:
> >> HIBERNATION covers the main hibernation control code and freeze-thaw
> >> pm events, that xen's save/restore also uses. Explicitly enabling
> >> an independant hibernation functionality to enable xen's save/restore
> >> is a bit ugly. Define a new user visible symbol HIBERNATION_INTERFACE
> >> that "selects" HIBERNATION and covers the main hibernation control code
> >> instead of HIBERNATION. This way, we can also make XEN_SAVE_RESTORE
> >> "select" HIBERNATION, enabling only the freeze-thaw code.
> >>
> >> Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>
> >> ---
> >>  kernel/power/Kconfig     |    9 +++++++--
> >>  kernel/power/hibernate.c |    4 ++++
> >>  kernel/power/main.c      |    2 +-
> >>  kernel/power/user.c      |    2 ++
> >>  4 files changed, 14 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
> >> index 4603f08..493c678 100644
> >> --- a/kernel/power/Kconfig
> >> +++ b/kernel/power/Kconfig
> >> @@ -19,10 +19,15 @@ config SUSPEND_FREEZER
> >>         Turning OFF this setting is NOT recommended! If in doubt, say Y.
> >>
> >>  config HIBERNATION
> >> -     bool "Hibernation (aka 'suspend to disk')"
> >> -     depends on SWAP && ARCH_HIBERNATION_POSSIBLE
> >> +     def_bool n
> >
> > You don't need that.  Simply use "bool"
> >
> >> +     depends on ARCH_HIBERNATION_POSSIBLE
> >
> > That need not depend on it, HIBERNATION_INTERFACE should instead.
> >
> Does that mean the pm_ops interface does not depend on the ARCH_HIB.. ?

That's as far as I remember, but no, it only depends on HIBERNATION.

> >>       select LZO_COMPRESS
> >>       select LZO_DECOMPRESS
> >
> > These two selects may be moved to HIBERNATION_INTERFACE too.
> >
> >> +
> >> +config HIBERNATION_INTERFACE
> >> +     bool "Hibernation (aka 'suspend to disk')"
> >> +     depends on SWAP
> >> +     select HIBERNATION
> >>       ---help---
> >>         Enable the suspend to disk (STD) functionality, which is usually
> >>         called "hibernation" in user interfaces.  STD checkpoints the
> >> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
> >> index 1832bd2..13bcf69 100644
> >> --- a/kernel/power/hibernate.c
> >> +++ b/kernel/power/hibernate.c
> >> @@ -592,6 +592,7 @@ static int prepare_processes(void)
> >>   *   hibernate - The granpappy of the built-in hibernation management
> >>   */
> >>
> >> +#ifdef CONFIG_HIBERNATION_INTERFACE
> >
> > Please don't do that.  Please make the whole hibernate.c depend on
> > CONFIG_HIBERNATION_INTERFACE.
> >
> This #if only wraps the hibernate() function. There is a whole lot of code
> (global functions too) before the hibernate() function.
> 
> As i mentioned in the first email 0/5, simply making
> hibernate.c and user.c depend on HIBERNATION_INTERFACE does not work.
> 
> I got the following linker errors.
> 
> kernel/built-in.o: In function `sys_reboot':
> kernel/sys.c:440: undefined reference to `hibernate'
> kernel/built-in.o: In function `state_store':
> kernel/power/main.c:185: undefined reference to `hibernate'
> kernel/built-in.o: In function `hibernate_preallocate_memory':
> kernel/power/snapshot.c:1411: undefined reference to `swsusp_show_speed'
> kernel/built-in.o: In function `swsusp_check':
> kernel/power/swap.c:933: undefined reference to `swsusp_resume_device'
> kernel/power/swap.c:938: undefined reference to `swsusp_resume_block'
> kernel/power/swap.c:946: undefined reference to `swsusp_resume_block'
> kernel/built-in.o: In function `load_image_lzo':
> kernel/power/swap.c:878: undefined reference to `swsusp_show_speed'
> kernel/built-in.o: In function `load_image':
> kernel/power/swap.c:741: undefined reference to `swsusp_show_speed'
> kernel/built-in.o: In function `save_image_lzo':
> kernel/power/swap.c:502: undefined reference to `lzo1x_1_compress'
> kernel/power/swap.c:543: undefined reference to `swsusp_show_speed'
> kernel/built-in.o: In function `swsusp_swap_check':
> kernel/power/swap.c:221: undefined reference to `swsusp_resume_block'
> kernel/power/swap.c:221: undefined reference to `swsusp_resume_device'
> kernel/built-in.o: In function `mark_swapfiles':
> kernel/power/swap.c:195: undefined reference to `swsusp_resume_block'
> kernel/power/swap.c:202: undefined reference to `swsusp_resume_block'
> kernel/built-in.o: In function `save_image':
> kernel/power/swap.c:418: undefined reference to `swsusp_show_speed'
> drivers/built-in.o: In function `acpi_suspend':
> drivers/acpi/sleep.c:584: undefined reference to `hibernate'
> drivers/built-in.o: In function `ata_scsi_start_stop_xlat':
> drivers/ata/libata-scsi.c:1331: undefined reference to
> `system_entering_hibernation'
> drivers/built-in.o: In function `acpi_sleep_init':
> drivers/acpi/sleep.c:782: undefined reference to `hibernation_set_ops'
> arch/x86/power/built-in.o: In function `restore_registers':
> arch/x86/power/hibernate_asm_64.S:145: undefined reference to `in_suspend'
> 
> Seeing errors from arch/.. was also the reason why I made HIBERNATE depend on
> ARCH_HIBERNATION_POSSIBLE

Well, all of those things above are fixable.

Basically, I can do the patch for you, but that'll take some time.
Stay tuned. :-)

Thanks,
Rafael

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [linux-pm] [PATCH v3 4/5] PM: Add visible HIBERNATION_INTERFACE and hide HIBERNATION
  2011-03-16  0:53     ` Rafael J. Wysocki
@ 2011-03-18 21:36       ` Rafael J. Wysocki
  2011-03-20  1:25         ` Shriram Rajagopalan
  0 siblings, 1 reply; 11+ messages in thread
From: Rafael J. Wysocki @ 2011-03-18 21:36 UTC (permalink / raw)
  To: linux-pm; +Cc: rshriram, xen-devel

On Wednesday, March 16, 2011, Rafael J. Wysocki wrote:
> On Tuesday, March 15, 2011, Shriram Rajagopalan wrote:
> > 2011/3/15 Rafael J. Wysocki <rjw@sisk.pl>:
...
> 
> Basically, I can do the patch for you, but that'll take some time.
> Stay tuned. :-)

The patch below appears to work for me, please check if it works
for you when you put the Xen patch selecting HIBERNATION on top
of it.

Thanks,
Rafael

---
 arch/x86/power/Makefile |    3 ++-
 include/linux/suspend.h |   15 +++++----------
 kernel/power/Kconfig    |    4 ++++
 kernel/power/Makefile   |    4 ++--
 kernel/power/main.c     |    2 +-
 kernel/power/power.h    |    6 +++---
 6 files changed, 17 insertions(+), 17 deletions(-)

Index: linux-2.6/kernel/power/Kconfig
===================================================================
--- linux-2.6.orig/kernel/power/Kconfig
+++ linux-2.6/kernel/power/Kconfig
@@ -19,8 +19,12 @@ config SUSPEND_FREEZER
 	  Turning OFF this setting is NOT recommended! If in doubt, say Y.
 
 config HIBERNATION
+	bool
+
+config HIBERNATE_INTERFACE
 	bool "Hibernation (aka 'suspend to disk')"
 	depends on SWAP && ARCH_HIBERNATION_POSSIBLE
+	select HIBERNATION
 	select LZO_COMPRESS
 	select LZO_DECOMPRESS
 	---help---
Index: linux-2.6/kernel/power/Makefile
===================================================================
--- linux-2.6.orig/kernel/power/Makefile
+++ linux-2.6/kernel/power/Makefile
@@ -5,7 +5,7 @@ obj-$(CONFIG_PM_SLEEP)		+= console.o
 obj-$(CONFIG_FREEZER)		+= process.o
 obj-$(CONFIG_SUSPEND)		+= suspend.o
 obj-$(CONFIG_PM_TEST_SUSPEND)	+= suspend_test.o
-obj-$(CONFIG_HIBERNATION)	+= hibernate.o snapshot.o swap.o user.o \
-				   block_io.o
+obj-$(CONFIG_HIBERNATE_INTERFACE)	+= hibernate.o snapshot.o swap.o \
+						user.o block_io.o
 
 obj-$(CONFIG_MAGIC_SYSRQ)	+= poweroff.o
Index: linux-2.6/include/linux/suspend.h
===================================================================
--- linux-2.6.orig/include/linux/suspend.h
+++ linux-2.6/include/linux/suspend.h
@@ -229,7 +229,7 @@ struct platform_hibernation_ops {
 	void (*recover)(void);
 };
 
-#ifdef CONFIG_HIBERNATION
+#ifdef CONFIG_HIBERNATE_INTERFACE
 /* kernel/power/snapshot.c */
 extern void __register_nosave_region(unsigned long b, unsigned long e, int km);
 static inline void __init register_nosave_region(unsigned long b, unsigned long e)
@@ -248,7 +248,9 @@ extern unsigned long get_safe_page(gfp_t
 extern void hibernation_set_ops(const struct platform_hibernation_ops *ops);
 extern int hibernate(void);
 extern bool system_entering_hibernation(void);
-#else /* CONFIG_HIBERNATION */
+#else /* !CONFIG_HIBERNATE_INTERFACE */
+static inline void register_nosave_region(unsigned long b, unsigned long e) {}
+static inline void register_nosave_region_late(unsigned long b, unsigned long e) {}
 static inline int swsusp_page_is_forbidden(struct page *p) { return 0; }
 static inline void swsusp_set_page_free(struct page *p) {}
 static inline void swsusp_unset_page_free(struct page *p) {}
@@ -256,7 +258,7 @@ static inline void swsusp_unset_page_fre
 static inline void hibernation_set_ops(const struct platform_hibernation_ops *ops) {}
 static inline int hibernate(void) { return -ENOSYS; }
 static inline bool system_entering_hibernation(void) { return false; }
-#endif /* CONFIG_HIBERNATION */
+#endif /* !CONFIG_HIBERNATE_INTERFACE */
 
 #ifdef CONFIG_PM_SLEEP
 void save_processor_state(void);
@@ -298,13 +300,6 @@ static inline bool pm_wakeup_pending(voi
 extern struct mutex pm_mutex;
 
 #ifndef CONFIG_HIBERNATION
-static inline void register_nosave_region(unsigned long b, unsigned long e)
-{
-}
-static inline void register_nosave_region_late(unsigned long b, unsigned long e)
-{
-}
-
 static inline void lock_system_sleep(void) {}
 static inline void unlock_system_sleep(void) {}
 
Index: linux-2.6/arch/x86/power/Makefile
===================================================================
--- linux-2.6.orig/arch/x86/power/Makefile
+++ linux-2.6/arch/x86/power/Makefile
@@ -4,4 +4,5 @@ nostackp := $(call cc-option, -fno-stack
 CFLAGS_cpu.o	:= $(nostackp)
 
 obj-$(CONFIG_PM_SLEEP)		+= cpu.o
-obj-$(CONFIG_HIBERNATION)	+= hibernate_$(BITS).o hibernate_asm_$(BITS).o
+obj-$(CONFIG_HIBERNATE_INTERFACE)	+= hibernate_$(BITS).o \
+						hibernate_asm_$(BITS).o
Index: linux-2.6/kernel/power/main.c
===================================================================
--- linux-2.6.orig/kernel/power/main.c
+++ linux-2.6/kernel/power/main.c
@@ -156,7 +156,7 @@ static ssize_t state_show(struct kobject
 			s += sprintf(s,"%s ", pm_states[i]);
 	}
 #endif
-#ifdef CONFIG_HIBERNATION
+#ifdef CONFIG_HIBERNATE_INTERFACE
 	s += sprintf(s, "%s\n", "disk");
 #else
 	if (s != buf)
Index: linux-2.6/kernel/power/power.h
===================================================================
--- linux-2.6.orig/kernel/power/power.h
+++ linux-2.6/kernel/power/power.h
@@ -13,7 +13,7 @@ struct swsusp_info {
 	unsigned long		size;
 } __attribute__((aligned(PAGE_SIZE)));
 
-#ifdef CONFIG_HIBERNATION
+#ifdef CONFIG_HIBERNATE_INTERFACE
 /* kernel/power/snapshot.c */
 extern void __init hibernate_image_size_init(void);
 
@@ -53,10 +53,10 @@ extern int hibernation_snapshot(int plat
 extern int hibernation_restore(int platform_mode);
 extern int hibernation_platform_enter(void);
 
-#else /* !CONFIG_HIBERNATION */
+#else /* !CONFIG_HIBERNATE_INTERFACE */
 
 static inline void hibernate_image_size_init(void) {}
-#endif /* !CONFIG_HIBERNATION */
+#endif /* !CONFIG_HIBERNATE_INTERFACE */
 
 extern int pfn_is_nosave(unsigned long);

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [linux-pm] [PATCH v3 4/5] PM: Add visible HIBERNATION_INTERFACE and hide HIBERNATION
  2011-03-18 21:36       ` [linux-pm] " Rafael J. Wysocki
@ 2011-03-20  1:25         ` Shriram Rajagopalan
  2011-03-25  7:23           ` Shriram Rajagopalan
  0 siblings, 1 reply; 11+ messages in thread
From: Shriram Rajagopalan @ 2011-03-20  1:25 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: linux-pm, xen-devel


[-- Attachment #1.1: Type: text/plain, Size: 6369 bytes --]

On Fri, Mar 18, 2011 at 2:36 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:

> On Wednesday, March 16, 2011, Rafael J. Wysocki wrote:
> > On Tuesday, March 15, 2011, Shriram Rajagopalan wrote:
> > > 2011/3/15 Rafael J. Wysocki <rjw@sisk.pl>:
> ...
> >
> > Basically, I can do the patch for you, but that'll take some time.
> > Stay tuned. :-)
>
> The patch below appears to work for me, please check if it works
> for you when you put the Xen patch selecting HIBERNATION on top
> of it.
>
> Thanks,
> Rafael
>
> ---
>  arch/x86/power/Makefile |    3 ++-
>  include/linux/suspend.h |   15 +++++----------
>  kernel/power/Kconfig    |    4 ++++
>  kernel/power/Makefile   |    4 ++--
>  kernel/power/main.c     |    2 +-
>  kernel/power/power.h    |    6 +++---
>  6 files changed, 17 insertions(+), 17 deletions(-)
>
> Index: linux-2.6/kernel/power/Kconfig
> ===================================================================
> --- linux-2.6.orig/kernel/power/Kconfig
> +++ linux-2.6/kernel/power/Kconfig
> @@ -19,8 +19,12 @@ config SUSPEND_FREEZER
>           Turning OFF this setting is NOT recommended! If in doubt, say Y.
>
>  config HIBERNATION
> +       bool
> +
> +config HIBERNATE_INTERFACE
>         bool "Hibernation (aka 'suspend to disk')"
>         depends on SWAP && ARCH_HIBERNATION_POSSIBLE
> +       select HIBERNATION
>         select LZO_COMPRESS
>        select LZO_DECOMPRESS
>         ---help---
> Index: linux-2.6/kernel/power/Makefile
> ===================================================================
> --- linux-2.6.orig/kernel/power/Makefile
> +++ linux-2.6/kernel/power/Makefile
> @@ -5,7 +5,7 @@ obj-$(CONFIG_PM_SLEEP)          += console.o
>  obj-$(CONFIG_FREEZER)          += process.o
>  obj-$(CONFIG_SUSPEND)          += suspend.o
>  obj-$(CONFIG_PM_TEST_SUSPEND)  += suspend_test.o
> -obj-$(CONFIG_HIBERNATION)      += hibernate.o snapshot.o swap.o user.o \
> -                                  block_io.o
> +obj-$(CONFIG_HIBERNATE_INTERFACE)      += hibernate.o snapshot.o swap.o \
> +                                               user.o block_io.o
>
>  obj-$(CONFIG_MAGIC_SYSRQ)      += poweroff.o
> Index: linux-2.6/include/linux/suspend.h
> ===================================================================
> --- linux-2.6.orig/include/linux/suspend.h
> +++ linux-2.6/include/linux/suspend.h
> @@ -229,7 +229,7 @@ struct platform_hibernation_ops {
>        void (*recover)(void);
>  };
>
> -#ifdef CONFIG_HIBERNATION
> +#ifdef CONFIG_HIBERNATE_INTERFACE
>  /* kernel/power/snapshot.c */
>  extern void __register_nosave_region(unsigned long b, unsigned long e, int
> km);
>  static inline void __init register_nosave_region(unsigned long b, unsigned
> long e)
> @@ -248,7 +248,9 @@ extern unsigned long get_safe_page(gfp_t
>  extern void hibernation_set_ops(const struct platform_hibernation_ops
> *ops);
>  extern int hibernate(void);
>  extern bool system_entering_hibernation(void);
> -#else /* CONFIG_HIBERNATION */
> +#else /* !CONFIG_HIBERNATE_INTERFACE */
> +static inline void register_nosave_region(unsigned long b, unsigned long
> e) {}
> +static inline void register_nosave_region_late(unsigned long b, unsigned
> long e) {}
>  static inline int swsusp_page_is_forbidden(struct page *p) { return 0; }
>  static inline void swsusp_set_page_free(struct page *p) {}
>  static inline void swsusp_unset_page_free(struct page *p) {}
> @@ -256,7 +258,7 @@ static inline void swsusp_unset_page_fre
>  static inline void hibernation_set_ops(const struct
> platform_hibernation_ops *ops) {}
>  static inline int hibernate(void) { return -ENOSYS; }
>  static inline bool system_entering_hibernation(void) { return false; }
> -#endif /* CONFIG_HIBERNATION */
> +#endif /* !CONFIG_HIBERNATE_INTERFACE */
>
>  #ifdef CONFIG_PM_SLEEP
>  void save_processor_state(void);
> @@ -298,13 +300,6 @@ static inline bool pm_wakeup_pending(voi
>  extern struct mutex pm_mutex;
>
>  #ifndef CONFIG_HIBERNATION
> -static inline void register_nosave_region(unsigned long b, unsigned long
> e)
> -{
> -}
> -static inline void register_nosave_region_late(unsigned long b, unsigned
> long e)
> -{
> -}
> -
>  static inline void lock_system_sleep(void) {}
>  static inline void unlock_system_sleep(void) {}
>
> Index: linux-2.6/arch/x86/power/Makefile
> ===================================================================
> --- linux-2.6.orig/arch/x86/power/Makefile
> +++ linux-2.6/arch/x86/power/Makefile
> @@ -4,4 +4,5 @@ nostackp := $(call cc-option, -fno-stack
>  CFLAGS_cpu.o   := $(nostackp)
>
>  obj-$(CONFIG_PM_SLEEP)         += cpu.o
> -obj-$(CONFIG_HIBERNATION)      += hibernate_$(BITS).o
> hibernate_asm_$(BITS).o
> +obj-$(CONFIG_HIBERNATE_INTERFACE)      += hibernate_$(BITS).o \
> +                                               hibernate_asm_$(BITS).o
> Index: linux-2.6/kernel/power/main.c
> ===================================================================
> --- linux-2.6.orig/kernel/power/main.c
> +++ linux-2.6/kernel/power/main.c
> @@ -156,7 +156,7 @@ static ssize_t state_show(struct kobject
>                         s += sprintf(s,"%s ", pm_states[i]);
>        }
>  #endif
> -#ifdef CONFIG_HIBERNATION
> +#ifdef CONFIG_HIBERNATE_INTERFACE
>         s += sprintf(s, "%s\n", "disk");
>  #else
>        if (s != buf)
> Index: linux-2.6/kernel/power/power.h
> ===================================================================
> --- linux-2.6.orig/kernel/power/power.h
> +++ linux-2.6/kernel/power/power.h
> @@ -13,7 +13,7 @@ struct swsusp_info {
>        unsigned long           size;
>  } __attribute__((aligned(PAGE_SIZE)));
>
> -#ifdef CONFIG_HIBERNATION
> +#ifdef CONFIG_HIBERNATE_INTERFACE
>  /* kernel/power/snapshot.c */
>  extern void __init hibernate_image_size_init(void);
>
> @@ -53,10 +53,10 @@ extern int hibernation_snapshot(int plat
>  extern int hibernation_restore(int platform_mode);
>  extern int hibernation_platform_enter(void);
>
> -#else /* !CONFIG_HIBERNATION */
> +#else /* !CONFIG_HIBERNATE_INTERFACE */
>
>  static inline void hibernate_image_size_init(void) {}
> -#endif /* !CONFIG_HIBERNATION */
> +#endif /* !CONFIG_HIBERNATE_INTERFACE */
>
>  extern int pfn_is_nosave(unsigned long);
>
>
> The code compiles perfectly. However, I am not able to test it
in my installation (cant seem to boot a PV kernel - nothing to do
with your patch).
Will respond once I get the boot issue sorted.

shriram

[-- Attachment #1.2: Type: text/html, Size: 7412 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [linux-pm] [PATCH v3 4/5] PM: Add visible HIBERNATION_INTERFACE and hide HIBERNATION
  2011-03-20  1:25         ` Shriram Rajagopalan
@ 2011-03-25  7:23           ` Shriram Rajagopalan
  2011-03-25 22:35             ` Rafael J. Wysocki
  0 siblings, 1 reply; 11+ messages in thread
From: Shriram Rajagopalan @ 2011-03-25  7:23 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: linux-pm, xen-devel


[-- Attachment #1.1: Type: text/plain, Size: 7162 bytes --]

On Sat, Mar 19, 2011 at 6:25 PM, Shriram Rajagopalan <rshriram@cs.ubc.ca>wrote:

> On Fri, Mar 18, 2011 at 2:36 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>
>> On Wednesday, March 16, 2011, Rafael J. Wysocki wrote:
>> > On Tuesday, March 15, 2011, Shriram Rajagopalan wrote:
>> > > 2011/3/15 Rafael J. Wysocki <rjw@sisk.pl>:
>> ...
>> >
>> > Basically, I can do the patch for you, but that'll take some time.
>> > Stay tuned. :-)
>>
>> The patch below appears to work for me, please check if it works
>> for you when you put the Xen patch selecting HIBERNATION on top
>> of it.
>>
>> Thanks,
>> Rafael
>>
>> ---
>>  arch/x86/power/Makefile |    3 ++-
>>  include/linux/suspend.h |   15 +++++----------
>>  kernel/power/Kconfig    |    4 ++++
>>  kernel/power/Makefile   |    4 ++--
>>  kernel/power/main.c     |    2 +-
>>  kernel/power/power.h    |    6 +++---
>>  6 files changed, 17 insertions(+), 17 deletions(-)
>>
>> Index: linux-2.6/kernel/power/Kconfig
>> ===================================================================
>> --- linux-2.6.orig/kernel/power/Kconfig
>> +++ linux-2.6/kernel/power/Kconfig
>> @@ -19,8 +19,12 @@ config SUSPEND_FREEZER
>>           Turning OFF this setting is NOT recommended! If in doubt, say Y.
>>
>>  config HIBERNATION
>> +       bool
>> +
>> +config HIBERNATE_INTERFACE
>>         bool "Hibernation (aka 'suspend to disk')"
>>         depends on SWAP && ARCH_HIBERNATION_POSSIBLE
>> +       select HIBERNATION
>>         select LZO_COMPRESS
>>        select LZO_DECOMPRESS
>>         ---help---
>> Index: linux-2.6/kernel/power/Makefile
>> ===================================================================
>> --- linux-2.6.orig/kernel/power/Makefile
>> +++ linux-2.6/kernel/power/Makefile
>> @@ -5,7 +5,7 @@ obj-$(CONFIG_PM_SLEEP)          += console.o
>>  obj-$(CONFIG_FREEZER)          += process.o
>>  obj-$(CONFIG_SUSPEND)          += suspend.o
>>  obj-$(CONFIG_PM_TEST_SUSPEND)  += suspend_test.o
>> -obj-$(CONFIG_HIBERNATION)      += hibernate.o snapshot.o swap.o user.o \
>> -                                  block_io.o
>> +obj-$(CONFIG_HIBERNATE_INTERFACE)      += hibernate.o snapshot.o swap.o \
>> +                                               user.o block_io.o
>>
>>  obj-$(CONFIG_MAGIC_SYSRQ)      += poweroff.o
>> Index: linux-2.6/include/linux/suspend.h
>> ===================================================================
>> --- linux-2.6.orig/include/linux/suspend.h
>> +++ linux-2.6/include/linux/suspend.h
>> @@ -229,7 +229,7 @@ struct platform_hibernation_ops {
>>        void (*recover)(void);
>>  };
>>
>> -#ifdef CONFIG_HIBERNATION
>> +#ifdef CONFIG_HIBERNATE_INTERFACE
>>  /* kernel/power/snapshot.c */
>>  extern void __register_nosave_region(unsigned long b, unsigned long e,
>> int km);
>>  static inline void __init register_nosave_region(unsigned long b,
>> unsigned long e)
>> @@ -248,7 +248,9 @@ extern unsigned long get_safe_page(gfp_t
>>  extern void hibernation_set_ops(const struct platform_hibernation_ops
>> *ops);
>>  extern int hibernate(void);
>>  extern bool system_entering_hibernation(void);
>> -#else /* CONFIG_HIBERNATION */
>> +#else /* !CONFIG_HIBERNATE_INTERFACE */
>> +static inline void register_nosave_region(unsigned long b, unsigned long
>> e) {}
>> +static inline void register_nosave_region_late(unsigned long b, unsigned
>> long e) {}
>>  static inline int swsusp_page_is_forbidden(struct page *p) { return 0; }
>>  static inline void swsusp_set_page_free(struct page *p) {}
>>  static inline void swsusp_unset_page_free(struct page *p) {}
>> @@ -256,7 +258,7 @@ static inline void swsusp_unset_page_fre
>>  static inline void hibernation_set_ops(const struct
>> platform_hibernation_ops *ops) {}
>>  static inline int hibernate(void) { return -ENOSYS; }
>>  static inline bool system_entering_hibernation(void) { return false; }
>> -#endif /* CONFIG_HIBERNATION */
>> +#endif /* !CONFIG_HIBERNATE_INTERFACE */
>>
>>  #ifdef CONFIG_PM_SLEEP
>>  void save_processor_state(void);
>> @@ -298,13 +300,6 @@ static inline bool pm_wakeup_pending(voi
>>  extern struct mutex pm_mutex;
>>
>>  #ifndef CONFIG_HIBERNATION
>> -static inline void register_nosave_region(unsigned long b, unsigned long
>> e)
>> -{
>> -}
>> -static inline void register_nosave_region_late(unsigned long b, unsigned
>> long e)
>> -{
>> -}
>> -
>>  static inline void lock_system_sleep(void) {}
>>  static inline void unlock_system_sleep(void) {}
>>
>> Index: linux-2.6/arch/x86/power/Makefile
>> ===================================================================
>> --- linux-2.6.orig/arch/x86/power/Makefile
>> +++ linux-2.6/arch/x86/power/Makefile
>> @@ -4,4 +4,5 @@ nostackp := $(call cc-option, -fno-stack
>>  CFLAGS_cpu.o   := $(nostackp)
>>
>>  obj-$(CONFIG_PM_SLEEP)         += cpu.o
>> -obj-$(CONFIG_HIBERNATION)      += hibernate_$(BITS).o
>> hibernate_asm_$(BITS).o
>> +obj-$(CONFIG_HIBERNATE_INTERFACE)      += hibernate_$(BITS).o \
>> +                                               hibernate_asm_$(BITS).o
>> Index: linux-2.6/kernel/power/main.c
>> ===================================================================
>> --- linux-2.6.orig/kernel/power/main.c
>> +++ linux-2.6/kernel/power/main.c
>> @@ -156,7 +156,7 @@ static ssize_t state_show(struct kobject
>>                         s += sprintf(s,"%s ", pm_states[i]);
>>        }
>>  #endif
>> -#ifdef CONFIG_HIBERNATION
>> +#ifdef CONFIG_HIBERNATE_INTERFACE
>>         s += sprintf(s, "%s\n", "disk");
>>  #else
>>        if (s != buf)
>> Index: linux-2.6/kernel/power/power.h
>> ===================================================================
>> --- linux-2.6.orig/kernel/power/power.h
>> +++ linux-2.6/kernel/power/power.h
>> @@ -13,7 +13,7 @@ struct swsusp_info {
>>        unsigned long           size;
>>  } __attribute__((aligned(PAGE_SIZE)));
>>
>> -#ifdef CONFIG_HIBERNATION
>> +#ifdef CONFIG_HIBERNATE_INTERFACE
>>  /* kernel/power/snapshot.c */
>>  extern void __init hibernate_image_size_init(void);
>>
>> @@ -53,10 +53,10 @@ extern int hibernation_snapshot(int plat
>>  extern int hibernation_restore(int platform_mode);
>>  extern int hibernation_platform_enter(void);
>>
>> -#else /* !CONFIG_HIBERNATION */
>> +#else /* !CONFIG_HIBERNATE_INTERFACE */
>>
>>  static inline void hibernate_image_size_init(void) {}
>> -#endif /* !CONFIG_HIBERNATION */
>> +#endif /* !CONFIG_HIBERNATE_INTERFACE */
>>
>>  extern int pfn_is_nosave(unsigned long);
>>
>>
>> The code compiles perfectly. However, I am not able to test it
> in my installation (cant seem to boot a PV kernel - nothing to do
> with your patch).
> Will respond once I get the boot issue sorted.
>
> shriram
>
Hi,
sorry for the delay. I finally managed to get my system back in
one piece.

So the patch you sent works perfectly. I tested xen save/restore with
both HIBERNATE_INTERFACE enabled and disabled. When disabled,
I dont see sys/power/disk.. (as expected).

so how do you want to go about these two patches? I am ok with you
pushing "4/5 Add visible HIBERNATE_INTERFACE..." under your own
authorship (as you did the refactoring anyway :) ). That way, you would
just have to take "5/5 fix XEN_SAVE_RESTORE Kconfig dependencies".

shriram

[-- Attachment #1.2: Type: text/html, Size: 8362 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [linux-pm] [PATCH v3 4/5] PM: Add visible HIBERNATION_INTERFACE and hide HIBERNATION
  2011-03-25  7:23           ` Shriram Rajagopalan
@ 2011-03-25 22:35             ` Rafael J. Wysocki
  2011-03-25 22:41               ` Shriram Rajagopalan
  0 siblings, 1 reply; 11+ messages in thread
From: Rafael J. Wysocki @ 2011-03-25 22:35 UTC (permalink / raw)
  To: rshriram; +Cc: linux-pm, xen-devel

On Friday, March 25, 2011, Shriram Rajagopalan wrote:
> On Sat, Mar 19, 2011 at 6:25 PM, Shriram Rajagopalan <rshriram@cs.ubc.ca>wrote:
> 
> > On Fri, Mar 18, 2011 at 2:36 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> >
> >> On Wednesday, March 16, 2011, Rafael J. Wysocki wrote:
> >> > On Tuesday, March 15, 2011, Shriram Rajagopalan wrote:
> >> > > 2011/3/15 Rafael J. Wysocki <rjw@sisk.pl>:
> >> ...
> >> >
> >> > Basically, I can do the patch for you, but that'll take some time.
> >> > Stay tuned. :-)
> >>
> >> The patch below appears to work for me, please check if it works
> >> for you when you put the Xen patch selecting HIBERNATION on top
> >> of it.
> >>
> >> Thanks,
> >> Rafael
> >>
> >> ---
> >>  arch/x86/power/Makefile |    3 ++-
> >>  include/linux/suspend.h |   15 +++++----------
> >>  kernel/power/Kconfig    |    4 ++++
> >>  kernel/power/Makefile   |    4 ++--
> >>  kernel/power/main.c     |    2 +-
> >>  kernel/power/power.h    |    6 +++---
> >>  6 files changed, 17 insertions(+), 17 deletions(-)
> >>
> >> Index: linux-2.6/kernel/power/Kconfig
> >> ===================================================================
> >> --- linux-2.6.orig/kernel/power/Kconfig
> >> +++ linux-2.6/kernel/power/Kconfig
> >> @@ -19,8 +19,12 @@ config SUSPEND_FREEZER
> >>           Turning OFF this setting is NOT recommended! If in doubt, say Y.
> >>
> >>  config HIBERNATION
> >> +       bool
> >> +
> >> +config HIBERNATE_INTERFACE
> >>         bool "Hibernation (aka 'suspend to disk')"
> >>         depends on SWAP && ARCH_HIBERNATION_POSSIBLE
> >> +       select HIBERNATION
> >>         select LZO_COMPRESS
> >>        select LZO_DECOMPRESS
> >>         ---help---
> >> Index: linux-2.6/kernel/power/Makefile
> >> ===================================================================
> >> --- linux-2.6.orig/kernel/power/Makefile
> >> +++ linux-2.6/kernel/power/Makefile
> >> @@ -5,7 +5,7 @@ obj-$(CONFIG_PM_SLEEP)          += console.o
> >>  obj-$(CONFIG_FREEZER)          += process.o
> >>  obj-$(CONFIG_SUSPEND)          += suspend.o
> >>  obj-$(CONFIG_PM_TEST_SUSPEND)  += suspend_test.o
> >> -obj-$(CONFIG_HIBERNATION)      += hibernate.o snapshot.o swap.o user.o \
> >> -                                  block_io.o
> >> +obj-$(CONFIG_HIBERNATE_INTERFACE)      += hibernate.o snapshot.o swap.o \
> >> +                                               user.o block_io.o
> >>
> >>  obj-$(CONFIG_MAGIC_SYSRQ)      += poweroff.o
> >> Index: linux-2.6/include/linux/suspend.h
> >> ===================================================================
> >> --- linux-2.6.orig/include/linux/suspend.h
> >> +++ linux-2.6/include/linux/suspend.h
> >> @@ -229,7 +229,7 @@ struct platform_hibernation_ops {
> >>        void (*recover)(void);
> >>  };
> >>
> >> -#ifdef CONFIG_HIBERNATION
> >> +#ifdef CONFIG_HIBERNATE_INTERFACE
> >>  /* kernel/power/snapshot.c */
> >>  extern void __register_nosave_region(unsigned long b, unsigned long e,
> >> int km);
> >>  static inline void __init register_nosave_region(unsigned long b,
> >> unsigned long e)
> >> @@ -248,7 +248,9 @@ extern unsigned long get_safe_page(gfp_t
> >>  extern void hibernation_set_ops(const struct platform_hibernation_ops
> >> *ops);
> >>  extern int hibernate(void);
> >>  extern bool system_entering_hibernation(void);
> >> -#else /* CONFIG_HIBERNATION */
> >> +#else /* !CONFIG_HIBERNATE_INTERFACE */
> >> +static inline void register_nosave_region(unsigned long b, unsigned long
> >> e) {}
> >> +static inline void register_nosave_region_late(unsigned long b, unsigned
> >> long e) {}
> >>  static inline int swsusp_page_is_forbidden(struct page *p) { return 0; }
> >>  static inline void swsusp_set_page_free(struct page *p) {}
> >>  static inline void swsusp_unset_page_free(struct page *p) {}
> >> @@ -256,7 +258,7 @@ static inline void swsusp_unset_page_fre
> >>  static inline void hibernation_set_ops(const struct
> >> platform_hibernation_ops *ops) {}
> >>  static inline int hibernate(void) { return -ENOSYS; }
> >>  static inline bool system_entering_hibernation(void) { return false; }
> >> -#endif /* CONFIG_HIBERNATION */
> >> +#endif /* !CONFIG_HIBERNATE_INTERFACE */
> >>
> >>  #ifdef CONFIG_PM_SLEEP
> >>  void save_processor_state(void);
> >> @@ -298,13 +300,6 @@ static inline bool pm_wakeup_pending(voi
> >>  extern struct mutex pm_mutex;
> >>
> >>  #ifndef CONFIG_HIBERNATION
> >> -static inline void register_nosave_region(unsigned long b, unsigned long
> >> e)
> >> -{
> >> -}
> >> -static inline void register_nosave_region_late(unsigned long b, unsigned
> >> long e)
> >> -{
> >> -}
> >> -
> >>  static inline void lock_system_sleep(void) {}
> >>  static inline void unlock_system_sleep(void) {}
> >>
> >> Index: linux-2.6/arch/x86/power/Makefile
> >> ===================================================================
> >> --- linux-2.6.orig/arch/x86/power/Makefile
> >> +++ linux-2.6/arch/x86/power/Makefile
> >> @@ -4,4 +4,5 @@ nostackp := $(call cc-option, -fno-stack
> >>  CFLAGS_cpu.o   := $(nostackp)
> >>
> >>  obj-$(CONFIG_PM_SLEEP)         += cpu.o
> >> -obj-$(CONFIG_HIBERNATION)      += hibernate_$(BITS).o
> >> hibernate_asm_$(BITS).o
> >> +obj-$(CONFIG_HIBERNATE_INTERFACE)      += hibernate_$(BITS).o \
> >> +                                               hibernate_asm_$(BITS).o
> >> Index: linux-2.6/kernel/power/main.c
> >> ===================================================================
> >> --- linux-2.6.orig/kernel/power/main.c
> >> +++ linux-2.6/kernel/power/main.c
> >> @@ -156,7 +156,7 @@ static ssize_t state_show(struct kobject
> >>                         s += sprintf(s,"%s ", pm_states[i]);
> >>        }
> >>  #endif
> >> -#ifdef CONFIG_HIBERNATION
> >> +#ifdef CONFIG_HIBERNATE_INTERFACE
> >>         s += sprintf(s, "%s\n", "disk");
> >>  #else
> >>        if (s != buf)
> >> Index: linux-2.6/kernel/power/power.h
> >> ===================================================================
> >> --- linux-2.6.orig/kernel/power/power.h
> >> +++ linux-2.6/kernel/power/power.h
> >> @@ -13,7 +13,7 @@ struct swsusp_info {
> >>        unsigned long           size;
> >>  } __attribute__((aligned(PAGE_SIZE)));
> >>
> >> -#ifdef CONFIG_HIBERNATION
> >> +#ifdef CONFIG_HIBERNATE_INTERFACE
> >>  /* kernel/power/snapshot.c */
> >>  extern void __init hibernate_image_size_init(void);
> >>
> >> @@ -53,10 +53,10 @@ extern int hibernation_snapshot(int plat
> >>  extern int hibernation_restore(int platform_mode);
> >>  extern int hibernation_platform_enter(void);
> >>
> >> -#else /* !CONFIG_HIBERNATION */
> >> +#else /* !CONFIG_HIBERNATE_INTERFACE */
> >>
> >>  static inline void hibernate_image_size_init(void) {}
> >> -#endif /* !CONFIG_HIBERNATION */
> >> +#endif /* !CONFIG_HIBERNATE_INTERFACE */
> >>
> >>  extern int pfn_is_nosave(unsigned long);
> >>
> >>
> >> The code compiles perfectly. However, I am not able to test it
> > in my installation (cant seem to boot a PV kernel - nothing to do
> > with your patch).
> > Will respond once I get the boot issue sorted.
> >
> > shriram
> >
> Hi,
> sorry for the delay. I finally managed to get my system back in
> one piece.
> 
> So the patch you sent works perfectly. I tested xen save/restore with
> both HIBERNATE_INTERFACE enabled and disabled. When disabled,
> I dont see sys/power/disk.. (as expected).

Cool. :-)

> so how do you want to go about these two patches? I am ok with you
> pushing "4/5 Add visible HIBERNATE_INTERFACE..." under your own
> authorship (as you did the refactoring anyway :) ). That way, you would
> just have to take "5/5 fix XEN_SAVE_RESTORE Kconfig dependencies".

I will push my patch to Linus, but quite likely after -rc1, since I've just
prepared a pull request I'm going to send later today.

I'll add your Tested-by to the patch if you don't mind.

Thanks,
Rafael

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [linux-pm] [PATCH v3 4/5] PM: Add visible HIBERNATION_INTERFACE and hide HIBERNATION
  2011-03-25 22:35             ` Rafael J. Wysocki
@ 2011-03-25 22:41               ` Shriram Rajagopalan
  0 siblings, 0 replies; 11+ messages in thread
From: Shriram Rajagopalan @ 2011-03-25 22:41 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: linux-pm, xen-devel


[-- Attachment #1.1: Type: text/plain, Size: 8214 bytes --]

2011/3/25 Rafael J. Wysocki <rjw@sisk.pl>

> On Friday, March 25, 2011, Shriram Rajagopalan wrote:
> > On Sat, Mar 19, 2011 at 6:25 PM, Shriram Rajagopalan <rshriram@cs.ubc.ca
> >wrote:
> >
> > > On Fri, Mar 18, 2011 at 2:36 PM, Rafael J. Wysocki <rjw@sisk.pl>
> wrote:
> > >
> > >> On Wednesday, March 16, 2011, Rafael J. Wysocki wrote:
> > >> > On Tuesday, March 15, 2011, Shriram Rajagopalan wrote:
> > >> > > 2011/3/15 Rafael J. Wysocki <rjw@sisk.pl>:
> > >> ...
> > >> >
> > >> > Basically, I can do the patch for you, but that'll take some time.
> > >> > Stay tuned. :-)
> > >>
> > >> The patch below appears to work for me, please check if it works
> > >> for you when you put the Xen patch selecting HIBERNATION on top
> > >> of it.
> > >>
> > >> Thanks,
> > >> Rafael
> > >>
> > >> ---
> > >>  arch/x86/power/Makefile |    3 ++-
> > >>  include/linux/suspend.h |   15 +++++----------
> > >>  kernel/power/Kconfig    |    4 ++++
> > >>  kernel/power/Makefile   |    4 ++--
> > >>  kernel/power/main.c     |    2 +-
> > >>  kernel/power/power.h    |    6 +++---
> > >>  6 files changed, 17 insertions(+), 17 deletions(-)
> > >>
> > >> Index: linux-2.6/kernel/power/Kconfig
> > >> ===================================================================
> > >> --- linux-2.6.orig/kernel/power/Kconfig
> > >> +++ linux-2.6/kernel/power/Kconfig
> > >> @@ -19,8 +19,12 @@ config SUSPEND_FREEZER
> > >>           Turning OFF this setting is NOT recommended! If in doubt,
> say Y.
> > >>
> > >>  config HIBERNATION
> > >> +       bool
> > >> +
> > >> +config HIBERNATE_INTERFACE
> > >>         bool "Hibernation (aka 'suspend to disk')"
> > >>         depends on SWAP && ARCH_HIBERNATION_POSSIBLE
> > >> +       select HIBERNATION
> > >>         select LZO_COMPRESS
> > >>        select LZO_DECOMPRESS
> > >>         ---help---
> > >> Index: linux-2.6/kernel/power/Makefile
> > >> ===================================================================
> > >> --- linux-2.6.orig/kernel/power/Makefile
> > >> +++ linux-2.6/kernel/power/Makefile
> > >> @@ -5,7 +5,7 @@ obj-$(CONFIG_PM_SLEEP)          += console.o
> > >>  obj-$(CONFIG_FREEZER)          += process.o
> > >>  obj-$(CONFIG_SUSPEND)          += suspend.o
> > >>  obj-$(CONFIG_PM_TEST_SUSPEND)  += suspend_test.o
> > >> -obj-$(CONFIG_HIBERNATION)      += hibernate.o snapshot.o swap.o
> user.o \
> > >> -                                  block_io.o
> > >> +obj-$(CONFIG_HIBERNATE_INTERFACE)      += hibernate.o snapshot.o
> swap.o \
> > >> +                                               user.o block_io.o
> > >>
> > >>  obj-$(CONFIG_MAGIC_SYSRQ)      += poweroff.o
> > >> Index: linux-2.6/include/linux/suspend.h
> > >> ===================================================================
> > >> --- linux-2.6.orig/include/linux/suspend.h
> > >> +++ linux-2.6/include/linux/suspend.h
> > >> @@ -229,7 +229,7 @@ struct platform_hibernation_ops {
> > >>        void (*recover)(void);
> > >>  };
> > >>
> > >> -#ifdef CONFIG_HIBERNATION
> > >> +#ifdef CONFIG_HIBERNATE_INTERFACE
> > >>  /* kernel/power/snapshot.c */
> > >>  extern void __register_nosave_region(unsigned long b, unsigned long
> e,
> > >> int km);
> > >>  static inline void __init register_nosave_region(unsigned long b,
> > >> unsigned long e)
> > >> @@ -248,7 +248,9 @@ extern unsigned long get_safe_page(gfp_t
> > >>  extern void hibernation_set_ops(const struct platform_hibernation_ops
> > >> *ops);
> > >>  extern int hibernate(void);
> > >>  extern bool system_entering_hibernation(void);
> > >> -#else /* CONFIG_HIBERNATION */
> > >> +#else /* !CONFIG_HIBERNATE_INTERFACE */
> > >> +static inline void register_nosave_region(unsigned long b, unsigned
> long
> > >> e) {}
> > >> +static inline void register_nosave_region_late(unsigned long b,
> unsigned
> > >> long e) {}
> > >>  static inline int swsusp_page_is_forbidden(struct page *p) { return
> 0; }
> > >>  static inline void swsusp_set_page_free(struct page *p) {}
> > >>  static inline void swsusp_unset_page_free(struct page *p) {}
> > >> @@ -256,7 +258,7 @@ static inline void swsusp_unset_page_fre
> > >>  static inline void hibernation_set_ops(const struct
> > >> platform_hibernation_ops *ops) {}
> > >>  static inline int hibernate(void) { return -ENOSYS; }
> > >>  static inline bool system_entering_hibernation(void) { return false;
> }
> > >> -#endif /* CONFIG_HIBERNATION */
> > >> +#endif /* !CONFIG_HIBERNATE_INTERFACE */
> > >>
> > >>  #ifdef CONFIG_PM_SLEEP
> > >>  void save_processor_state(void);
> > >> @@ -298,13 +300,6 @@ static inline bool pm_wakeup_pending(voi
> > >>  extern struct mutex pm_mutex;
> > >>
> > >>  #ifndef CONFIG_HIBERNATION
> > >> -static inline void register_nosave_region(unsigned long b, unsigned
> long
> > >> e)
> > >> -{
> > >> -}
> > >> -static inline void register_nosave_region_late(unsigned long b,
> unsigned
> > >> long e)
> > >> -{
> > >> -}
> > >> -
> > >>  static inline void lock_system_sleep(void) {}
> > >>  static inline void unlock_system_sleep(void) {}
> > >>
> > >> Index: linux-2.6/arch/x86/power/Makefile
> > >> ===================================================================
> > >> --- linux-2.6.orig/arch/x86/power/Makefile
> > >> +++ linux-2.6/arch/x86/power/Makefile
> > >> @@ -4,4 +4,5 @@ nostackp := $(call cc-option, -fno-stack
> > >>  CFLAGS_cpu.o   := $(nostackp)
> > >>
> > >>  obj-$(CONFIG_PM_SLEEP)         += cpu.o
> > >> -obj-$(CONFIG_HIBERNATION)      += hibernate_$(BITS).o
> > >> hibernate_asm_$(BITS).o
> > >> +obj-$(CONFIG_HIBERNATE_INTERFACE)      += hibernate_$(BITS).o \
> > >> +
> hibernate_asm_$(BITS).o
> > >> Index: linux-2.6/kernel/power/main.c
> > >> ===================================================================
> > >> --- linux-2.6.orig/kernel/power/main.c
> > >> +++ linux-2.6/kernel/power/main.c
> > >> @@ -156,7 +156,7 @@ static ssize_t state_show(struct kobject
> > >>                         s += sprintf(s,"%s ", pm_states[i]);
> > >>        }
> > >>  #endif
> > >> -#ifdef CONFIG_HIBERNATION
> > >> +#ifdef CONFIG_HIBERNATE_INTERFACE
> > >>         s += sprintf(s, "%s\n", "disk");
> > >>  #else
> > >>        if (s != buf)
> > >> Index: linux-2.6/kernel/power/power.h
> > >> ===================================================================
> > >> --- linux-2.6.orig/kernel/power/power.h
> > >> +++ linux-2.6/kernel/power/power.h
> > >> @@ -13,7 +13,7 @@ struct swsusp_info {
> > >>        unsigned long           size;
> > >>  } __attribute__((aligned(PAGE_SIZE)));
> > >>
> > >> -#ifdef CONFIG_HIBERNATION
> > >> +#ifdef CONFIG_HIBERNATE_INTERFACE
> > >>  /* kernel/power/snapshot.c */
> > >>  extern void __init hibernate_image_size_init(void);
> > >>
> > >> @@ -53,10 +53,10 @@ extern int hibernation_snapshot(int plat
> > >>  extern int hibernation_restore(int platform_mode);
> > >>  extern int hibernation_platform_enter(void);
> > >>
> > >> -#else /* !CONFIG_HIBERNATION */
> > >> +#else /* !CONFIG_HIBERNATE_INTERFACE */
> > >>
> > >>  static inline void hibernate_image_size_init(void) {}
> > >> -#endif /* !CONFIG_HIBERNATION */
> > >> +#endif /* !CONFIG_HIBERNATE_INTERFACE */
> > >>
> > >>  extern int pfn_is_nosave(unsigned long);
> > >>
> > >>
> > >> The code compiles perfectly. However, I am not able to test it
> > > in my installation (cant seem to boot a PV kernel - nothing to do
> > > with your patch).
> > > Will respond once I get the boot issue sorted.
> > >
> > > shriram
> > >
> > Hi,
> > sorry for the delay. I finally managed to get my system back in
> > one piece.
> >
> > So the patch you sent works perfectly. I tested xen save/restore with
> > both HIBERNATE_INTERFACE enabled and disabled. When disabled,
> > I dont see sys/power/disk.. (as expected).
>
> Cool. :-)
>
> > so how do you want to go about these two patches? I am ok with you
> > pushing "4/5 Add visible HIBERNATE_INTERFACE..." under your own
> > authorship (as you did the refactoring anyway :) ). That way, you would
> > just have to take "5/5 fix XEN_SAVE_RESTORE Kconfig dependencies".
>
> I will push my patch to Linus, but quite likely after -rc1, since I've just
> prepared a pull request I'm going to send later today.
>
> I'll add your Tested-by to the patch if you don't mind.
>
> Thanks,
> Rafael
>
> yep. sure

[-- Attachment #1.2: Type: text/html, Size: 10961 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2011-03-25 22:41 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-03-12  5:07 [PATCH v3 4/5] PM: Add visible HIBERNATION_INTERFACE and hide HIBERNATION Shriram Rajagopalan
2011-03-15 21:25 ` Shriram Rajagopalan
2011-03-15 21:39   ` Rafael J. Wysocki
2011-03-15 21:38 ` Rafael J. Wysocki
2011-03-15 22:16   ` Shriram Rajagopalan
2011-03-16  0:53     ` Rafael J. Wysocki
2011-03-18 21:36       ` [linux-pm] " Rafael J. Wysocki
2011-03-20  1:25         ` Shriram Rajagopalan
2011-03-25  7:23           ` Shriram Rajagopalan
2011-03-25 22:35             ` Rafael J. Wysocki
2011-03-25 22:41               ` Shriram Rajagopalan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).