From: Sergey Dyasly <dserrg@gmail.com>
To: Oleg Nesterov <oleg@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
David Rientjes <rientjes@google.com>,
Frederic Weisbecker <fweisbec@gmail.com>,
Mandeep Singh Baines <msb@chromium.org>,
"Ma, Xindong" <xindong.ma@intel.com>,
Michal Hocko <mhocko@suse.cz>, Sameer Nanda <snanda@chromium.org>,
"Tu, Xiaobing" <xiaobing.tu@intel.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 0/2] initial while_each_thread() fixes
Date: Tue, 3 Dec 2013 19:46:35 +0400 [thread overview]
Message-ID: <20131203194635.7be1c07ae042ee395bc83527@gmail.com> (raw)
In-Reply-To: <20131202152423.GA10878@redhat.com>
Hi Oleg,
I was waiting for this one!
On Mon, 2 Dec 2013 16:24:23 +0100
Oleg Nesterov <oleg@redhat.com> wrote:
> Hello.
>
> This was reported several times, I believe the first report is
> http://marc.info/?l=linux-kernel&m=127688978121665. Hmm, 3 years
> ago. The lockless while_each_thread() is racy and broken, almost
> every user can loop forever.
>
> Recently people started to report they actually hit this problem in
> oom_kill.c. This doesn't really matter and I can be wrong, but in
> fact I do not think they really hit this race, it is very unlikely.
The race is very easy to catch if you have a process with several threads,
all of which allocates memory simultaneously. This leads to:
1) OOMk selects and sends SIGKILL to one of the threads
2) another thread invokes OOMk and the first thread gets selected,
but it gets unhashed before while_each_thread...
> Another problem with while_each_thread() is that it is very easy
> to use it wrongly, and oom_kill.c is the good example.
>
> I came to conclusion that it is practically impossible to send a
> single series which fixes all problems, too many different users.
>
> So 1/2 adds the new for_each_thread() interface, and 2/2 fixes oom
> kill as an example.
>
> We obviously need a lot more changes like 2/2 before we can kill
> while_each_thread() and task_struct->thread_group, but I hope they
> will be straighforward. And in fact I hope that task->thread_group
> can go away before we change all users of while_each_thread().
>
> David, et al, I din't actually test 2/2, I do not know how. Please
> review, although it looks simple.
The patches look correct and my test case no longer hangs, so
Reviewed-and-Tested-by: Sergey Dyasly <dserrg@gmail.com>
>
> Oleg.
>
> include/linux/init_task.h | 2 ++
> include/linux/sched.h | 12 ++++++++++++
> kernel/exit.c | 1 +
> kernel/fork.c | 7 +++++++
> mm/oom_kill.c | 37 ++++++++++++++++++++-----------------
> 5 files changed, 42 insertions(+), 17 deletions(-)
>
--
Sergey Dyasly <dserrg@gmail.com>
next prev parent reply other threads:[~2013-12-03 15:46 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-12-02 15:24 [PATCH 0/2] initial while_each_thread() fixes Oleg Nesterov
2013-12-02 15:24 ` [PATCH 1/2] introduce for_each_thread() to replace the buggy while_each_thread() Oleg Nesterov
2013-12-03 19:24 ` Sameer Nanda
2013-12-02 15:24 ` [PATCH 2/2] oom_kill: change oom_kill.c to use for_each_thread() Oleg Nesterov
2013-12-03 18:57 ` Sameer Nanda
2013-12-03 20:05 ` Oleg Nesterov
2013-12-03 20:50 ` Oleg Nesterov
2013-12-04 12:57 ` Oleg Nesterov
2013-12-02 15:34 ` [PATCH 0/2] initial while_each_thread() fixes Oleg Nesterov
2013-12-03 15:46 ` Sergey Dyasly [this message]
2013-12-03 18:52 ` Oleg Nesterov
2013-12-03 19:01 ` Oleg Nesterov
2013-12-03 16:53 ` William Dauchy
2013-12-03 20:22 ` Oleg Nesterov
2013-12-03 20:28 ` William Dauchy
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=20131203194635.7be1c07ae042ee395bc83527@gmail.com \
--to=dserrg@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=fweisbec@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mhocko@suse.cz \
--cc=msb@chromium.org \
--cc=oleg@redhat.com \
--cc=rientjes@google.com \
--cc=snanda@chromium.org \
--cc=xiaobing.tu@intel.com \
--cc=xindong.ma@intel.com \
/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.