lkml.org 
[lkml]   [2011]   [Nov]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH pm] freezer: fix wait_event_freezable/__thaw_task races
Date
On Tuesday, November 01, 2011, Tejun Heo wrote:
> From: Oleg Nesterov <oleg@redhat.com>
>
> wait_event_freezable() and friends stop the waiting if try_to_freeze()
> fails. This is not right, we can race with __thaw_task() and in this
> case
>
> - wait_event_freezable() returns the wrong ERESTARTSYS
>
> - wait_event_freezable_timeout() can return the positive
> value while condition == F
>
> Change the code to always check __retval/condition before return.
>
> Note: with or without this patch the timeout logic looks strange,
> probably we should recalc timeout if try_to_freeze() returns T.
>
> tj: Updated to apply to wait_event_freezekillable() too.
>
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>
> Acked-by: Tejun Heo <tj@kernel.org>

Applied to linux-pm/linux-next.

Thanks,
Rafael


> ---
>
> So, this should be the last of lost patches. It's on top of
>
> linus/master
> +[1] freezer: fix various bugs and simplify implementation, take#2
> +[2] usb_storage: don't use set_freezable_with_signal()" + [3]
> +[3] freezer: kill unused set_freezable_with_signal()
>
> and available in the following git branch.
>
> git://git.kernel.org/pub/scm/linux/kernel/git/tj/misc.git pm-fix-wait_freezable
>
> Thank you.
>
> [1] http://thread.gmane.org/gmane.linux.kernel/1209247
> [2] http://thread.gmane.org/gmane.linux.kernel/1209416
> [3] http://thread.gmane.org/gmane.linux.kernel/1209416/focus=1209417
>
> include/linux/freezer.h | 27 ++++++++++++++-------------
> 1 files changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/include/linux/freezer.h b/include/linux/freezer.h
> index 5f32321..4a1d933 100644
> --- a/include/linux/freezer.h
> +++ b/include/linux/freezer.h
> @@ -113,42 +113,43 @@ static inline int freezer_should_skip(struct task_struct *p)
> #define wait_event_freezekillable(wq, condition) \
> ({ \
> int __retval; \
> - do { \
> + for (;;) { \
> __retval = wait_event_killable(wq, \
> (condition) || freezing(current)); \
> - if (__retval && !freezing(current)) \
> + if (__retval || (condition)) \
> break; \
> - else if (!(condition)) \
> - __retval = -ERESTARTSYS; \
> - } while (try_to_freeze()); \
> + try_to_freeze(); \
> + } \
> __retval; \
> })
>
> #define wait_event_freezable(wq, condition) \
> ({ \
> int __retval; \
> - do { \
> + for (;;) { \
> __retval = wait_event_interruptible(wq, \
> (condition) || freezing(current)); \
> - if (__retval && !freezing(current)) \
> + if (__retval || (condition)) \
> break; \
> - else if (!(condition)) \
> - __retval = -ERESTARTSYS; \
> - } while (try_to_freeze()); \
> + try_to_freeze(); \
> + } \
> __retval; \
> })
>
> -
> #define wait_event_freezable_timeout(wq, condition, timeout) \
> ({ \
> long __retval = timeout; \
> - do { \
> + for (;;) { \
> __retval = wait_event_interruptible_timeout(wq, \
> (condition) || freezing(current), \
> __retval); \
> - } while (try_to_freeze()); \
> + if (__retval <= 0 || (condition)) \
> + break; \
> + try_to_freeze(); \
> + } \
> __retval; \
> })
> +
> #else /* !CONFIG_FREEZER */
> static inline bool frozen(struct task_struct *p) { return false; }
> static inline bool freezing(struct task_struct *p) { return false; }
>



\
 
 \ /
  Last update: 2011-11-03 01:21    [W:0.038 / U:5.236 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site