From: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
To: Oleg Nesterov <oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
"Rafael J. Wysocki" <rjw-KKrjLPT3xs0@public.gmane.org>,
linux-pm-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
paul-inf54ven1CmVyaH7bEyXVA@public.gmane.org
Subject: Re: [PATCH 1/1] freezer: fix wait_event_freezable/__thaw_task races
Date: Thu, 8 Sep 2011 10:05:30 +0900 [thread overview]
Message-ID: <20110908010530.GD3987@mtj.dyndns.org> (raw)
In-Reply-To: <20110907182217.GB13909-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Hello,
On Wed, Sep 07, 2011 at 08:22:17PM +0200, Oleg Nesterov wrote:
> wait_event_freezable() and wait_event_freezable_timeout() 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
Indeed, nice catch. This one actually is pretty dangerous. We
probably want to make a separate fix for this and backport it to
-stable?
> 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.
>
> Signed-off-by: Oleg Nesterov <oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Yeap, with freezable_with_signal gone, this looks correct & simpler to
me but it would be nice if you can sprinkle some documentation while
at it. :)
Thanks.
--
tejun
WARNING: multiple messages have this Message-ID (diff)
From: Tejun Heo <tj@kernel.org>
To: Oleg Nesterov <oleg@redhat.com>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>,
matthltc@us.ibm.com, paul@paulmenage.org,
containers@lists.linux-foundation.org,
linux-pm@lists.linux-foundation.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/1] freezer: fix wait_event_freezable/__thaw_task races
Date: Thu, 8 Sep 2011 10:05:30 +0900 [thread overview]
Message-ID: <20110908010530.GD3987@mtj.dyndns.org> (raw)
In-Reply-To: <20110907182217.GB13909@redhat.com>
Hello,
On Wed, Sep 07, 2011 at 08:22:17PM +0200, Oleg Nesterov wrote:
> wait_event_freezable() and wait_event_freezable_timeout() 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
Indeed, nice catch. This one actually is pretty dangerous. We
probably want to make a separate fix for this and backport it to
-stable?
> 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.
>
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Yeap, with freezable_with_signal gone, this looks correct & simpler to
me but it would be nice if you can sprinkle some documentation while
at it. :)
Thanks.
--
tejun
next prev parent reply other threads:[~2011-09-08 1:05 UTC|newest]
Thread overview: 81+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-09-02 18:27 [PATCHSET pm-freezer] freezer: fixes & simplifications Tejun Heo
2011-09-02 18:27 ` [PATCH 1/6] cgroup_freezer: fix freezer->state setting bug in freezer_change_state() Tejun Heo
2011-09-02 18:27 ` Tejun Heo
[not found] ` <1314988070-12244-2-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2011-09-04 18:02 ` Oleg Nesterov
2011-09-04 18:02 ` Oleg Nesterov
[not found] ` <20110904180206.GA28520-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2011-09-04 18:11 ` Tejun Heo
2011-09-04 18:11 ` Tejun Heo
2011-09-04 18:20 ` Oleg Nesterov
[not found] ` <20110904181139.GA9807-Gd/HAXX7CRxy/B6EtB590w@public.gmane.org>
2011-09-04 18:20 ` Oleg Nesterov
2011-09-04 18:20 ` Oleg Nesterov
2011-09-04 18:11 ` Tejun Heo
2011-09-04 18:02 ` Oleg Nesterov
2011-09-02 18:27 ` [PATCH 2/6] freezer: set PF_NOFREEZE on a dying task right before TASK_DEAD " Tejun Heo
2011-09-02 18:27 ` Tejun Heo
[not found] ` <1314988070-12244-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2011-09-02 18:27 ` [PATCH 1/6] cgroup_freezer: fix freezer->state " Tejun Heo
2011-09-02 18:27 ` [PATCH 2/6] freezer: set PF_NOFREEZE on a dying task right before TASK_DEAD " Tejun Heo
2011-09-02 18:27 ` [PATCH 3/6] freezer: restructure __refrigerator() Tejun Heo
2011-09-02 18:27 ` [PATCH 4/6] freezer: use lock_task_sighand() in fake_signal_wake_up() Tejun Heo
2011-09-02 18:27 ` [PATCH 5/6] freezer: remove unused @sig_only from freeze_task() Tejun Heo
2011-09-02 18:27 ` [PATCH 6/6] freezer: kill unused set_freezable_with_signal() Tejun Heo
2011-09-04 18:48 ` [PATCHSET pm-freezer] freezer: fixes & simplifications Oleg Nesterov
2011-09-04 18:48 ` Oleg Nesterov
2011-09-02 18:27 ` [PATCH 3/6] freezer: restructure __refrigerator() Tejun Heo
2011-09-02 18:27 ` Tejun Heo
2011-09-02 18:27 ` [PATCH 4/6] freezer: use lock_task_sighand() in fake_signal_wake_up() Tejun Heo
2011-09-02 18:27 ` Tejun Heo
2011-09-02 18:27 ` [PATCH 5/6] freezer: remove unused @sig_only from freeze_task() Tejun Heo
2011-09-02 18:27 ` Tejun Heo
2011-09-02 18:27 ` [PATCH 6/6] freezer: kill unused set_freezable_with_signal() Tejun Heo
[not found] ` <1314988070-12244-7-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2011-09-04 18:46 ` Oleg Nesterov
2011-09-04 18:46 ` Oleg Nesterov
2011-09-05 2:33 ` Tejun Heo
2011-09-05 2:35 ` Tejun Heo
2011-09-05 16:21 ` Oleg Nesterov
2011-09-05 16:21 ` Oleg Nesterov
[not found] ` <20110905023505.GC9807-Gd/HAXX7CRxy/B6EtB590w@public.gmane.org>
2011-09-05 16:21 ` Oleg Nesterov
2011-09-05 2:35 ` Tejun Heo
[not found] ` <20110905023315.GB9807-Gd/HAXX7CRxy/B6EtB590w@public.gmane.org>
2011-09-05 2:35 ` Tejun Heo
2011-09-05 16:20 ` Oleg Nesterov
2011-09-05 16:20 ` Oleg Nesterov
[not found] ` <20110905162012.GA4445-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2011-09-06 3:28 ` Tejun Heo
2011-09-06 3:28 ` Tejun Heo
[not found] ` <20110906032846.GA18425-9pTldWuhBndy/B6EtB590w@public.gmane.org>
2011-09-06 15:18 ` Oleg Nesterov
2011-09-08 18:01 ` Matt Helsley
2011-09-08 18:01 ` Matt Helsley
[not found] ` <20110908180159.GA4197-52DBMbEzqgQ/wnmkkaCWp/UQ3DHhIser@public.gmane.org>
2011-09-11 1:37 ` Tejun Heo
2011-09-11 1:37 ` Tejun Heo
2011-09-11 1:37 ` Tejun Heo
2011-09-06 15:18 ` Oleg Nesterov
2011-09-06 15:18 ` Oleg Nesterov
2011-09-06 15:25 ` Oleg Nesterov
[not found] ` <20110906151836.GA15568-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2011-09-06 15:25 ` Oleg Nesterov
2011-09-06 15:25 ` Oleg Nesterov
2011-09-06 15:53 ` Tejun Heo
[not found] ` <20110906152539.GA16899-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2011-09-06 15:53 ` Tejun Heo
2011-09-06 15:53 ` Tejun Heo
[not found] ` <20110906155332.GF18425-9pTldWuhBndy/B6EtB590w@public.gmane.org>
2011-09-07 18:21 ` [PATCH 0/1] freezer: fix wait_event_freezable/__thaw_task races Oleg Nesterov
2011-09-07 18:21 ` Oleg Nesterov
2011-09-07 18:21 ` Oleg Nesterov
2011-09-07 18:22 ` [PATCH 1/1] " Oleg Nesterov
[not found] ` <20110907182156.GA13909-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2011-09-07 18:22 ` Oleg Nesterov
2011-09-07 18:22 ` Oleg Nesterov
[not found] ` <20110907182217.GB13909-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2011-09-08 1:05 ` Tejun Heo [this message]
2011-09-08 1:05 ` Tejun Heo
[not found] ` <20110908010530.GD3987-9pTldWuhBndy/B6EtB590w@public.gmane.org>
2011-09-08 17:59 ` Oleg Nesterov
2011-09-08 17:59 ` Oleg Nesterov
2011-09-11 1:54 ` Tejun Heo
[not found] ` <20110908175926.GA26986-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2011-09-11 1:54 ` Tejun Heo
2011-09-11 1:54 ` Tejun Heo
2011-09-11 18:29 ` Oleg Nesterov
2011-09-11 18:41 ` Oleg Nesterov
2011-09-08 17:59 ` Oleg Nesterov
2011-09-08 1:05 ` Tejun Heo
2011-09-08 18:01 ` [PATCH 6/6] freezer: kill unused set_freezable_with_signal() Matt Helsley
2011-09-06 3:28 ` Tejun Heo
2011-09-05 16:20 ` Oleg Nesterov
[not found] ` <20110904184626.GA30101-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2011-09-05 2:33 ` Tejun Heo
2011-09-05 2:33 ` Tejun Heo
2011-09-04 18:46 ` Oleg Nesterov
2011-09-02 18:27 ` Tejun Heo
2011-09-04 18:48 ` [PATCHSET pm-freezer] freezer: fixes & simplifications Oleg Nesterov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20110908010530.GD3987@mtj.dyndns.org \
--to=tj-dgejt+ai2ygdnm+yrofe0a@public.gmane.org \
--cc=containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org \
--cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=linux-pm-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org \
--cc=oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
--cc=paul-inf54ven1CmVyaH7bEyXVA@public.gmane.org \
--cc=rjw-KKrjLPT3xs0@public.gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.