From: Linus Torvalds <torvalds@linux-foundation.org>
To: Markus Trippelsdorf <markus@trippelsdorf.de>
Cc: akpm@linux-foundation.org,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
linux-mm <linux-mm@kvack.org>,
khlebnikov@openvz.org, hughd@google.com,
kamezawa.hiroyu@jp.fujitsu.com, oleg@redhat.com,
stable@vger.kernel.org
Subject: Re: [patch 12/12] mm: correctly synchronize rss-counters at exit/exec
Date: Thu, 7 Jun 2012 18:18:28 -0700 [thread overview]
Message-ID: <CA+55aFwuA3ex+XXW+TzOee8ax0g1NK9Mm5F3nYtY1m6YtvUFhQ@mail.gmail.com> (raw)
In-Reply-To: <20120608010520.GA25317@x4>
On Thu, Jun 7, 2012 at 6:05 PM, Markus Trippelsdorf
<markus@trippelsdorf.de> wrote:
>
> You've somehow merged the wrong patch.
> The correct version can be found here:
> http://marc.info/?l=linux-kernel&m=133848759505805
It looks like Andrew sent me a bad version.
However, that patch you point at isn't good *either*.
It does totally insane things in xacct_add_tsk(). You can't call
"sync_mm_rss(mm)" on somebody elses mm, yet that is exactly what it
does (and you can't pass in another thread pointer either, since the
whole point of the per-thread counters is that they don't have locking
and aren't atomic, so you can't read them from any other context than
"current").
The thing is, the *only* point where it makes sense to sync the rss
pointers is when you detach the mm from the current thread. And
possibly at "fork()" time, *before* you duplicate the "struct
task_struct" and pollute the new one with stale rss counter values
from the old one.
So doing sync_mm_rss() in xacct_add_tsk() is crazy. Doing it
*anywhere* where mm is not clearly "current->mm" is wrong. If there is
a "get_task_mm()" or similar nearby, it's wrong, it's crap, and it
shouldn't be done.
Oleg, please rescue me? Your patch looks much closer to sane, but it's
not quite there..
Linus
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2012-06-08 1:18 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20120607212114.E4F5AA02F8@akpm.mtv.corp.google.com>
[not found] ` <CA+55aFxOWR_h1vqRLAd_h5_woXjFBLyBHP--P8F7WsYrciXdmA@mail.gmail.com>
2012-06-08 0:25 ` Linus Torvalds
2012-06-08 1:05 ` Markus Trippelsdorf
2012-06-08 1:18 ` Linus Torvalds [this message]
2012-06-08 12:18 ` Oleg Nesterov
2012-06-11 10:25 ` Kamezawa Hiroyuki
2012-06-08 1:16 ` Hugh Dickins
2012-06-08 1:19 ` Linus Torvalds
2012-06-08 5:28 ` Hugh Dickins
2012-06-08 10:20 ` Konstantin Khlebnikov
2012-06-08 12:24 ` Oleg Nesterov
2012-06-08 13:29 ` Konstantin Khlebnikov
2012-06-08 17:01 ` Oleg Nesterov
2012-06-09 9:43 ` [PATCH] " Konstantin Khlebnikov
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=CA+55aFwuA3ex+XXW+TzOee8ax0g1NK9Mm5F3nYtY1m6YtvUFhQ@mail.gmail.com \
--to=torvalds@linux-foundation.org \
--cc=akpm@linux-foundation.org \
--cc=hughd@google.com \
--cc=kamezawa.hiroyu@jp.fujitsu.com \
--cc=khlebnikov@openvz.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=markus@trippelsdorf.de \
--cc=oleg@redhat.com \
--cc=stable@vger.kernel.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