linux-kernel - RE: [PATCH] suspend: enable freeze timeout configuration through sysctl

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <BEC9F67575FA1E429CA7CF5AE9BE36343F06B6@SHSMSX102.ccr.corp.intel.com>
Date:	Wed, 30 Jan 2013 06:22:55 +0000
From:	"Li, Fei" <fei.li@...el.com>
To:	"Rafael J. Wysocki" <rjw@...k.pl>
CC:	"akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
	"Liu, Chuansheng" <chuansheng.liu@...el.com>
Subject: RE: [PATCH] suspend: enable freeze timeout configuration through
 sysctl

Hello Rafael,
   Thanks for your feedback, and my understanding is interleaved in your email as below.

Best Regards,
Li Fei

> -----Original Message-----
> From: Rafael J. Wysocki [mailto:rjw@...k.pl]
> Sent: Tuesday, January 29, 2013 7:42 PM
> To: Li, Fei
> Cc: akpm@...ux-foundation.org; linux-kernel@...r.kernel.org;
> linux-pm@...r.kernel.org; Liu, Chuansheng
> Subject: Re: [PATCH] suspend: enable freeze timeout configuration through sysctl
> 
> On Tuesday, January 29, 2013 10:58:20 AM 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 sysctl, so that we can tune the value easily
> > for concrete usage, such as smaller value for handheld device such
> > as mobile phone.
> 
> Well, I'd argue that you shouldn't see freeze problems on such systems.
> If you're seeing them, it's better to fix them than to try to hide them
> from users (they are problems after all).
[Li, Fei] 
Thanks for your opinion.
Indeed, I see such freeze problems on mobile phone system using fuse file system.
The scenario is as below:
Thread A with i_mutex held is frozen during waiting for feedback from fuse daemon;
Thread B is trying to lock i_mutex and can't be frozen.
In the case above, 20s waiting is needless, as freezing will fail unavoidably.

I agree with you that we'd better fix them from the root, which may need solution of long term.
I also saw some related discussion on Linux community as below, without final conclusion:
http://lists.linux-foundation.org/pipermail/linux-pm/2008-October/018774.html

So I think if we can enable freezing timeout configuration, it will improve such issue.

> Do you have a specific example in which that new knob will be useful?
[Li, Fei] 
As the scenario stated above, if we can configure the value of timeout to 10s or other small value,
this time of freezing will be aborted in earlier time, and after i_mutex is released during thread A restarting,
the next time of suspend/freeze may succeed in relatively earlier time.

> Why do you want to do that through sysctl and not sysfs?
[Li, Fei] 
Thanks for your suggestion, sysfs is more suitable, and I'll use sysfs in patch V2.

> Rafael
> 
> 
> > Signed-off-by: Liu Chuansheng <chuansheng.liu@...el.com>
> > Signed-off-by: Li Fei <fei.li@...el.com>
> > ---
> >  include/linux/freezer.h |    5 +++++
> >  kernel/power/process.c  |    4 ++--
> >  kernel/sysctl.c         |   12 ++++++++++++
> >  3 files changed, 19 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/linux/freezer.h b/include/linux/freezer.h
> > index e4238ce..f37b3be 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 sysctl_freeze_process_timeout_secs;
> > +
> > +/*
> >   * Check if a process has been frozen
> >   */
> >  static inline bool frozen(struct task_struct *p)
> > diff --git a/kernel/power/process.c b/kernel/power/process.c
> > index d5a258b..f7eb7c9 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 sysctl_freeze_process_timeout_secs = 20;
> >
> >  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 + sysctl_freeze_process_timeout_secs * HZ;
> >
> >  	if (!user_only)
> >  		freeze_workqueues_begin();
> > diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> > index c88878d..f88bcb9 100644
> > --- a/kernel/sysctl.c
> > +++ b/kernel/sysctl.c
> > @@ -90,6 +90,9 @@
> >  #include <linux/nmi.h>
> >  #endif
> >
> > +#ifdef CONFIG_FREEZER
> > +#include <linux/freezer.h>
> > +#endif
> >
> >  #if defined(CONFIG_SYSCTL)
> >
> > @@ -1047,6 +1050,15 @@ static struct ctl_table kern_table[] = {
> >  		.proc_handler	= proc_dointvec,
> >  	},
> >  #endif
> > +#ifdef CONFIG_FREEZER
> > +	{
> > +		.procname	= "freeze_process_timeout_secs",
> > +		.data		= &sysctl_freeze_process_timeout_secs,
> > +		.maxlen		= sizeof(unsigned int),
> > +		.mode		= 0644,
> > +		.proc_handler	= proc_dointvec,
> > +	},
> > +#endif
> >  	{ }
> >  };
> >
> >
> --
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.