From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> To: Hugh Dickins <hugh@veritas.com> Cc: Andrew Morton <akpm@linux-foundation.org>, Balbir Singh <balbir@linux.vnet.ibm.com>, KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>, linux-kernel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH mmotm] memcg: fix mem_cgroup_update_mapped_file_stat oops Date: Thu, 30 Apr 2009 09:06:46 +0900 [thread overview] Message-ID: <20090430090646.a1443096.kamezawa.hiroyu@jp.fujitsu.com> (raw) In-Reply-To: <Pine.LNX.4.64.0904292209550.30874@blonde.anvils> On Wed, 29 Apr 2009 22:13:33 +0100 (BST) Hugh Dickins <hugh@veritas.com> wrote: > CONFIG_SPARSEMEM=y CONFIG_CGROUP_MEM_RES_CTLR=y cgroup_disable=memory > bootup is oopsing in mem_cgroup_update_mapped_file_stat(). !SPARSEMEM > is fine because its lookup_page_cgroup() contains an explicit check for > NULL node_page_cgroup, but the SPARSEMEM version was missing a check for > NULL section->page_cgroup. > Ouch, it's curious this bug alive now.. thank you. Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> I think this patch itself is sane but.. Balbir, could you see "caller" ? It seems strange. > Signed-off-by: Hugh Dickins <hugh@veritas.com> > --- > Should go in as a fix to > memcg-add-file-based-rss-accounting.patch > but it's curious that's the first thing to suffer from this divergence. > > Perhaps this is the wrong fix, and there should be an explicit > mem_cgroup_disable() check somewhere else; but it would then seem > dangerous that SPARSEMEM and !SPARSEMEM diverge in this way, > and there are lots of lookup_page_cgroup NULL tests around. > > mm/page_cgroup.c | 2 ++ > 1 file changed, 2 insertions(+) > > --- 2.6.30-rc3-mm1/mm/page_cgroup.c 2009-04-29 21:01:06.000000000 +0100 > +++ mmotm/mm/page_cgroup.c 2009-04-29 21:12:04.000000000 +0100 > @@ -99,6 +99,8 @@ struct page_cgroup *lookup_page_cgroup(s > unsigned long pfn = page_to_pfn(page); > struct mem_section *section = __pfn_to_section(pfn); > > + if (!section->page_cgroup) > + return NULL; > return section->page_cgroup + pfn; > } > >
WARNING: multiple messages have this Message-ID (diff)
From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> To: Hugh Dickins <hugh@veritas.com> Cc: Andrew Morton <akpm@linux-foundation.org>, Balbir Singh <balbir@linux.vnet.ibm.com>, KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>, linux-kernel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH mmotm] memcg: fix mem_cgroup_update_mapped_file_stat oops Date: Thu, 30 Apr 2009 09:06:46 +0900 [thread overview] Message-ID: <20090430090646.a1443096.kamezawa.hiroyu@jp.fujitsu.com> (raw) In-Reply-To: <Pine.LNX.4.64.0904292209550.30874@blonde.anvils> On Wed, 29 Apr 2009 22:13:33 +0100 (BST) Hugh Dickins <hugh@veritas.com> wrote: > CONFIG_SPARSEMEM=y CONFIG_CGROUP_MEM_RES_CTLR=y cgroup_disable=memory > bootup is oopsing in mem_cgroup_update_mapped_file_stat(). !SPARSEMEM > is fine because its lookup_page_cgroup() contains an explicit check for > NULL node_page_cgroup, but the SPARSEMEM version was missing a check for > NULL section->page_cgroup. > Ouch, it's curious this bug alive now.. thank you. Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> I think this patch itself is sane but.. Balbir, could you see "caller" ? It seems strange. > Signed-off-by: Hugh Dickins <hugh@veritas.com> > --- > Should go in as a fix to > memcg-add-file-based-rss-accounting.patch > but it's curious that's the first thing to suffer from this divergence. > > Perhaps this is the wrong fix, and there should be an explicit > mem_cgroup_disable() check somewhere else; but it would then seem > dangerous that SPARSEMEM and !SPARSEMEM diverge in this way, > and there are lots of lookup_page_cgroup NULL tests around. > > mm/page_cgroup.c | 2 ++ > 1 file changed, 2 insertions(+) > > --- 2.6.30-rc3-mm1/mm/page_cgroup.c 2009-04-29 21:01:06.000000000 +0100 > +++ mmotm/mm/page_cgroup.c 2009-04-29 21:12:04.000000000 +0100 > @@ -99,6 +99,8 @@ struct page_cgroup *lookup_page_cgroup(s > unsigned long pfn = page_to_pfn(page); > struct mem_section *section = __pfn_to_section(pfn); > > + if (!section->page_cgroup) > + return NULL; > return section->page_cgroup + pfn; > } > > -- 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:[~2009-04-30 0:08 UTC|newest] Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top 2009-04-29 21:13 [PATCH mmotm] memcg: fix mem_cgroup_update_mapped_file_stat oops Hugh Dickins 2009-04-29 21:13 ` Hugh Dickins 2009-04-30 0:06 ` KAMEZAWA Hiroyuki [this message] 2009-04-30 0:06 ` KAMEZAWA Hiroyuki 2009-04-30 4:52 ` Balbir Singh 2009-04-30 4:52 ` Balbir Singh 2009-04-30 8:27 ` KAMEZAWA Hiroyuki 2009-04-30 8:27 ` KAMEZAWA Hiroyuki 2009-05-01 13:55 ` Hugh Dickins 2009-05-01 13:55 ` Hugh Dickins 2009-05-01 15:32 ` Balbir Singh 2009-05-01 15:32 ` Balbir Singh
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=20090430090646.a1443096.kamezawa.hiroyu@jp.fujitsu.com \ --to=kamezawa.hiroyu@jp.fujitsu.com \ --cc=akpm@linux-foundation.org \ --cc=balbir@linux.vnet.ibm.com \ --cc=hugh@veritas.com \ --cc=kosaki.motohiro@jp.fujitsu.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.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: linkBe 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.