* Re: [ 11/48] mm: correctly synchronize rss-counters at exit/exec [not found] <20120701172008.252571630@decadent.org.uk> @ 2012-07-01 19:02 ` Hugh Dickins 2012-07-02 2:01 ` Ben Hutchings 0 siblings, 1 reply; 4+ messages in thread From: Hugh Dickins @ 2012-07-01 19:02 UTC (permalink / raw) To: Ben Hutchings Cc: linux-kernel, stable, torvalds, akpm, alan, Konstantin Khlebnikov, Markus Trippelsdorf, Oleg Nesterov, KAMEZAWA Hiroyuki On Sun, 1 Jul 2012, Ben Hutchings wrote: > 3.2-stable review patch. If anyone has any objections, please let me know. > > ------------------ > > From: Konstantin Khlebnikov <khlebnikov@openvz.org> > > commit 4fe7efdbdfb1c7e7a7f31decfd831c0f31d37091 upstream. > > do_exit() and exec_mmap() call sync_mm_rss() before mm_release() does > put_user(clear_child_tid) which can update task->rss_stat and thus make > mm->rss_stat inconsistent. This triggers the "BUG:" printk in check_mm(). > > Let's fix this bug in the safest way, and optimize/cleanup this later. > > Reported-by: Markus Trippelsdorf <markus@trippelsdorf.de> > Signed-off-by: Konstantin Khlebnikov <khlebnikov@openvz.org> > Cc: Oleg Nesterov <oleg@redhat.com> > Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> > Cc: Hugh Dickins <hughd@google.com> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org> > Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> > [bwh: Backported to 3.2: sync_mm_rss() still takes a struct task_struct *] > Signed-off-by: Ben Hutchings <ben@decadent.org.uk> If you or Konstantin or Oleg have done full diligence on this, to ensure that it is really applicable to 3.2 (not just that the patch applies without rejects), fair enough. But I'd be cautious about it: it went through enough alternatives and revisions that I wouldn't call it trivial; it's easy for me to imagine that some of the affected paths were actually slightly different in 3.2 days than they were in 3.4 days; and the disturbing warning that these mods silence ("BUG: Bad rss-counter state ") did not exist before 3.4 - unless you've ported that too? That's not to assert that we had no rss problem at all before 3.4, but we've not heard of any trouble from it. Caution tells me that this patch might cause more trouble than it's worth. Hugh > --- > --- a/fs/exec.c > +++ b/fs/exec.c > @@ -820,10 +820,10 @@ > /* Notify parent that we're no longer interested in the old VM */ > tsk = current; > old_mm = current->mm; > - sync_mm_rss(tsk, old_mm); > mm_release(tsk, old_mm); > > if (old_mm) { > + sync_mm_rss(tsk, old_mm); > /* > * Make sure that if there is a core dump in progress > * for the old mm, we get out and die instead of going > --- a/kernel/exit.c > +++ b/kernel/exit.c > @@ -641,6 +641,7 @@ > mm_release(tsk, mm); > if (!mm) > return; > + sync_mm_rss(tsk, mm); > /* > * Serialize with any possible pending coredump. > * We must hold mmap_sem around checking core_state > > > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [ 11/48] mm: correctly synchronize rss-counters at exit/exec 2012-07-01 19:02 ` [ 11/48] mm: correctly synchronize rss-counters at exit/exec Hugh Dickins @ 2012-07-02 2:01 ` Ben Hutchings 2012-07-02 16:46 ` Oleg Nesterov 0 siblings, 1 reply; 4+ messages in thread From: Ben Hutchings @ 2012-07-02 2:01 UTC (permalink / raw) To: Hugh Dickins Cc: linux-kernel, stable, torvalds, akpm, alan, Konstantin Khlebnikov, Markus Trippelsdorf, Oleg Nesterov, KAMEZAWA Hiroyuki [-- Attachment #1: Type: text/plain, Size: 2200 bytes --] On Sun, 2012-07-01 at 12:02 -0700, Hugh Dickins wrote: > On Sun, 1 Jul 2012, Ben Hutchings wrote: > > > 3.2-stable review patch. If anyone has any objections, please let me know. > > > > ------------------ > > > > From: Konstantin Khlebnikov <khlebnikov@openvz.org> > > > > commit 4fe7efdbdfb1c7e7a7f31decfd831c0f31d37091 upstream. > > > > do_exit() and exec_mmap() call sync_mm_rss() before mm_release() does > > put_user(clear_child_tid) which can update task->rss_stat and thus make > > mm->rss_stat inconsistent. This triggers the "BUG:" printk in check_mm(). > > > > Let's fix this bug in the safest way, and optimize/cleanup this later. > > > > Reported-by: Markus Trippelsdorf <markus@trippelsdorf.de> > > Signed-off-by: Konstantin Khlebnikov <khlebnikov@openvz.org> > > Cc: Oleg Nesterov <oleg@redhat.com> > > Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> > > Cc: Hugh Dickins <hughd@google.com> > > Signed-off-by: Andrew Morton <akpm@linux-foundation.org> > > Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> > > [bwh: Backported to 3.2: sync_mm_rss() still takes a struct task_struct *] > > Signed-off-by: Ben Hutchings <ben@decadent.org.uk> > > If you or Konstantin or Oleg have done full diligence on this, > to ensure that it is really applicable to 3.2 (not just that > the patch applies without rejects), fair enough. I can't claim that, no. > But I'd be cautious about it: it went through enough alternatives > and revisions that I wouldn't call it trivial; it's easy for me to > imagine that some of the affected paths were actually slightly different > in 3.2 days than they were in 3.4 days; and the disturbing warning that > these mods silence ("BUG: Bad rss-counter state ") did not exist before > 3.4 - unless you've ported that too? No. > That's not to assert that we had no rss problem at all before 3.4, > but we've not heard of any trouble from it. Caution tells me that > this patch might cause more trouble than it's worth. Then I'll assume this should be dropped unless Konstantin or Oleg speaks up for it. Ben. -- Ben Hutchings When in doubt, use brute force. - Ken Thompson [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 828 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [ 11/48] mm: correctly synchronize rss-counters at exit/exec 2012-07-02 2:01 ` Ben Hutchings @ 2012-07-02 16:46 ` Oleg Nesterov 2012-07-04 4:31 ` Ben Hutchings 0 siblings, 1 reply; 4+ messages in thread From: Oleg Nesterov @ 2012-07-02 16:46 UTC (permalink / raw) To: Ben Hutchings Cc: Hugh Dickins, linux-kernel, stable, torvalds, akpm, alan, Konstantin Khlebnikov, Markus Trippelsdorf, KAMEZAWA Hiroyuki On 07/02, Ben Hutchings wrote: > > Then I'll assume this should be dropped unless Konstantin or Oleg speaks > up for it. Just in case, I agree with Hugh. The patch should shut up check_mm() which doesn't exist in 3.2 Oleg. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [ 11/48] mm: correctly synchronize rss-counters at exit/exec 2012-07-02 16:46 ` Oleg Nesterov @ 2012-07-04 4:31 ` Ben Hutchings 0 siblings, 0 replies; 4+ messages in thread From: Ben Hutchings @ 2012-07-04 4:31 UTC (permalink / raw) To: Oleg Nesterov Cc: Hugh Dickins, linux-kernel, stable, torvalds, akpm, alan, Konstantin Khlebnikov, Markus Trippelsdorf, KAMEZAWA Hiroyuki [-- Attachment #1: Type: text/plain, Size: 408 bytes --] On Mon, 2012-07-02 at 18:46 +0200, Oleg Nesterov wrote: > On 07/02, Ben Hutchings wrote: > > > > Then I'll assume this should be dropped unless Konstantin or Oleg speaks > > up for it. > > Just in case, I agree with Hugh. > > The patch should shut up check_mm() which doesn't exist in 3.2 Thanks; I've dropped this. Ben. -- Ben Hutchings When in doubt, use brute force. - Ken Thompson [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 828 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-07-04 4:31 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20120701172008.252571630@decadent.org.uk>
2012-07-01 19:02 ` [ 11/48] mm: correctly synchronize rss-counters at exit/exec Hugh Dickins
2012-07-02 2:01 ` Ben Hutchings
2012-07-02 16:46 ` Oleg Nesterov
2012-07-04 4:31 ` Ben Hutchings
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox