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