lkml.org 
[lkml]   [2013]   [Jan]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    SubjectRE: [PATCH] suspend: enable freeze timeout configuration through sysctl
    Date
    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@sisk.pl]
    > Sent: Tuesday, January 29, 2013 7:42 PM
    > To: Li, Fei
    > Cc: akpm@linux-foundation.org; linux-kernel@vger.kernel.org;
    > linux-pm@vger.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@intel.com>
    > > Signed-off-by: Li Fei <fei.li@intel.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.
    \
     
     \ /
      Last update: 2013-01-30 08:01    [W:4.240 / U:1.080 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site