Linux Container Development
 help / color / mirror / Atom feed
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

  parent reply	other threads:[~2011-09-08  1:05 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1314988070-12244-1-git-send-email-tj@kernel.org>
     [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 setting bug in freezer_change_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
     [not found] ` <1314988070-12244-2-git-send-email-tj@kernel.org>
     [not found]   ` <1314988070-12244-2-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2011-09-04 18:02     ` [PATCH 1/6] cgroup_freezer: fix freezer->state setting bug in freezer_change_state() Oleg Nesterov
     [not found]       ` <20110904180206.GA28520-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2011-09-04 18:11         ` Tejun Heo
     [not found]           ` <20110904181139.GA9807-Gd/HAXX7CRxy/B6EtB590w@public.gmane.org>
2011-09-04 18:20             ` Oleg Nesterov
     [not found] ` <1314988070-12244-7-git-send-email-tj@kernel.org>
     [not found]   ` <1314988070-12244-7-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2011-09-04 18:46     ` [PATCH 6/6] freezer: kill unused set_freezable_with_signal() Oleg Nesterov
     [not found]   ` <20110904184626.GA30101@redhat.com>
     [not found]     ` <20110904184626.GA30101-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2011-09-05  2:33       ` Tejun Heo
     [not found]     ` <20110905023315.GB9807@htj.dyndns.org>
     [not found]       ` <20110905023315.GB9807-Gd/HAXX7CRxy/B6EtB590w@public.gmane.org>
2011-09-05  2:35         ` Tejun Heo
2011-09-05 16:20         ` Oleg Nesterov
     [not found]       ` <20110905023505.GC9807@htj.dyndns.org>
     [not found]         ` <20110905023505.GC9807-Gd/HAXX7CRxy/B6EtB590w@public.gmane.org>
2011-09-05 16:21           ` Oleg Nesterov
     [not found]       ` <20110905162012.GA4445@redhat.com>
     [not found]         ` <20110905162012.GA4445-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2011-09-06  3:28           ` Tejun Heo
     [not found]         ` <20110906032846.GA18425@mtj.dyndns.org>
     [not found]           ` <20110906032846.GA18425-9pTldWuhBndy/B6EtB590w@public.gmane.org>
2011-09-06 15:18             ` Oleg Nesterov
2011-09-08 18:01             ` Matt Helsley
     [not found]               ` <20110908180159.GA4197-52DBMbEzqgQ/wnmkkaCWp/UQ3DHhIser@public.gmane.org>
2011-09-11  1:37                 ` Tejun Heo
     [not found]           ` <20110906151836.GA15568@redhat.com>
     [not found]             ` <20110906151836.GA15568-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2011-09-06 15:25               ` Oleg Nesterov
     [not found]             ` <20110906152539.GA16899@redhat.com>
     [not found]               ` <20110906152539.GA16899-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2011-09-06 15:53                 ` Tejun Heo
     [not found]               ` <20110906155332.GF18425@mtj.dyndns.org>
     [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
     [not found]                 ` <20110907182156.GA13909@redhat.com>
     [not found]                   ` <20110907182156.GA13909-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2011-09-07 18:22                     ` [PATCH 1/1] " Oleg Nesterov
     [not found]                   ` <20110907182217.GB13909@redhat.com>
     [not found]                     ` <20110907182217.GB13909-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2011-09-08  1:05                       ` Tejun Heo [this message]
     [not found]                         ` <20110908010530.GD3987-9pTldWuhBndy/B6EtB590w@public.gmane.org>
2011-09-08 17:59                           ` Oleg Nesterov
     [not found]                             ` <20110908175926.GA26986-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2011-09-11  1:54                               ` Tejun Heo

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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox