RE: [PATCH V2] suspend: enable freeze timeout configuration throughsys

[Li, Fei]

Hello Rafael,
   I update it as below in V3, do you think it makes sense?

At present, the value of timeout for freezing is 20s, which is
meaningless in case that one thread is frozen with mutex locked
and another thread is trying to lock the mutex, as this time of
freezing will fail unavoidably.
And if there is no new wakeup event registered, the system will
waste at most 20s for such meaningless trying of freezing.

With this patch, the value of timeout can be configured to smaller
value, so such meaningless trying of freezing will be aborted in
earlier time, and later freezing can be also triggered in earlier
time. And more power will be saved.
In normal case on mobile phone, it costs real little time to freeze
processes. On some platform, it only costs about 20ms to freeze
user space processes and 10ms to freeze kernel freezable threads.

Thanks a lot!

Best Regards,
Li Fei
TEL: +86 (0)21 6116 5140
Mob: +86 (0)139 1696 7435
Platform System Integration Shanghai



> -----Original Message-----
> From: Rafael J. Wysocki [mailto:rjw@xxxxxxx]
> Sent: Wednesday, January 30, 2013 9:10 PM
> To: Li, Fei
> Cc: akpm@xxxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> linux-pm@xxxxxxxxxxxxxxx; Liu, Chuansheng
> Subject: Re: [PATCH V2] suspend: enable freeze timeout configuration through
> sys
> 
> On Wednesday, January 30, 2013 02:23:39 PM fli24 wrote:
> >
> > At present, the timeout value for freezing tasks is fixed as 20s,
> > which is too long for handheld device usage, especially for mobile
> > phone.
> >
> > In order to improve user experience, we enable freeze timeout
> > configuration through sys, so that we can tune the value easily
> > for concrete usage, such as smaller value for handheld device such
> > as mobile phone.
> 
> As I said, I'd like to know what's the exact problem you need to work around
> using this.
> 
> Thanks,
> Rafael
> 
> 
> > Signed-off-by: Liu Chuansheng <chuansheng.liu@xxxxxxxxx>
> > Signed-off-by: Li Fei <fei.li@xxxxxxxxx>
> > ---
> >  Documentation/power/freezing-of-tasks.txt |    5 +++++
> >  include/linux/freezer.h                   |    5 +++++
> >  kernel/power/main.c                       |   27
> +++++++++++++++++++++++++++
> >  kernel/power/process.c                    |    4 ++--
> >  4 files changed, 39 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/power/freezing-of-tasks.txt
> b/Documentation/power/freezing-of-tasks.txt
> > index 6ec291e..85894d8 100644
> > --- a/Documentation/power/freezing-of-tasks.txt
> > +++ b/Documentation/power/freezing-of-tasks.txt
> > @@ -223,3 +223,8 @@ since they ask the freezer to skip freezing this task,
> since it is anyway
> >  only after the entire suspend/hibernation sequence is complete.
> >  So, to summarize, use [un]lock_system_sleep() instead of directly using
> >  mutex_[un]lock(&pm_mutex). That would prevent freezing failures.
> > +
> > +V. Miscellaneous
> > +/sys/power/pm_freeze_timeout controls how long it will cost at most to
> freeze
> > +all user space processes or all freezable kernel threads, in unit of millisecond.
> > +The default value is 20000, with range of unsigned integer.
> > diff --git a/include/linux/freezer.h b/include/linux/freezer.h
> > index e4238ce..5a24a33 100644
> > --- a/include/linux/freezer.h
> > +++ b/include/linux/freezer.h
> > @@ -13,6 +13,11 @@ extern bool pm_freezing;		/* PM freezing in effect
> */
> >  extern bool pm_nosig_freezing;		/* PM nosig freezing in effect */
> >
> >  /*
> > + * Timeout for stopping processes
> > + */
> > +extern unsigned int sys_freeze_process_timeout_msecs;
> > +
> > +/*
> >   * Check if a process has been frozen
> >   */
> >  static inline bool frozen(struct task_struct *p)
> > diff --git a/kernel/power/main.c b/kernel/power/main.c
> > index 1c16f91..453ead1 100644
> > --- a/kernel/power/main.c
> > +++ b/kernel/power/main.c
> > @@ -553,6 +553,30 @@ power_attr(pm_trace_dev_match);
> >
> >  #endif /* CONFIG_PM_TRACE */
> >
> > +#ifdef CONFIG_FREEZER
> > +static ssize_t pm_freeze_timeout_show(struct kobject *kobj,
> > +				      struct kobj_attribute *attr, char *buf)
> > +{
> > +	return sprintf(buf, "%u\n", sys_freeze_process_timeout_msecs);
> > +}
> > +
> > +static ssize_t pm_freeze_timeout_store(struct kobject *kobj,
> > +				       struct kobj_attribute *attr,
> > +				       const char *buf, size_t n)
> > +{
> > +	unsigned long val;
> > +
> > +	if (kstrtoul(buf, 10, &val))
> > +		return -EINVAL;
> > +
> > +	sys_freeze_process_timeout_msecs = val;
> > +	return n;
> > +}
> > +
> > +power_attr(pm_freeze_timeout);
> > +
> > +#endif	/* CONFIG_FREEZER*/
> > +
> >  static struct attribute * g[] = {
> >  	&state_attr.attr,
> >  #ifdef CONFIG_PM_TRACE
> > @@ -576,6 +600,9 @@ static struct attribute * g[] = {
> >  	&pm_print_times_attr.attr,
> >  #endif
> >  #endif
> > +#ifdef CONFIG_FREEZER
> > +	&pm_freeze_timeout_attr.attr,
> > +#endif
> >  	NULL,
> >  };
> >
> > diff --git a/kernel/power/process.c b/kernel/power/process.c
> > index d5a258b..ba45a26 100644
> > --- a/kernel/power/process.c
> > +++ b/kernel/power/process.c
> > @@ -21,7 +21,7 @@
> >  /*
> >   * Timeout for stopping processes
> >   */
> > -#define TIMEOUT	(20 * HZ)
> > +unsigned int __read_mostly sys_freeze_process_timeout_msecs = 20000;
> >
> >  static int try_to_freeze_tasks(bool user_only)
> >  {
> > @@ -36,7 +36,7 @@ static int try_to_freeze_tasks(bool user_only)
> >
> >  	do_gettimeofday(&start);
> >
> > -	end_time = jiffies + TIMEOUT;
> > +	end_time = jiffies + msecs_to_jiffies(sys_freeze_process_timeout_msecs);
> >
> >  	if (!user_only)
> >  		freeze_workqueues_begin();
> >
> --
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.
¢éì®&Þ~º&¶¬–+-±éÝ¥Šw®žË±Êâmébžìdz¹Þ)í…æèw*jg¬±¨¶‰šŽŠÝj/êäz¹ÞŠà2ŠÞ¨è­Ú&¢)ß«a¶Úþø®G«éh®æj:+v‰¨Šwè†Ù>Wš±êÞiÛaxPjØm¶ŸÿÃ-»+ƒùdš_