Messages in this thread | | | Date | Fri, 1 May 2009 21:02:06 +0530 | From | Balbir Singh <> | Subject | Re: [PATCH mmotm] memcg: fix mem_cgroup_update_mapped_file_stat oops |
| |
* Hugh Dickins <hugh@veritas.com> [2009-05-01 14:55:51]:
> On Thu, 30 Apr 2009, KAMEZAWA Hiroyuki wrote: > > 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. > > I agree with you, it seems strange for it to come alive only now; > but I've not investigated further, may I leave that to you? > > Could it be that all those checks on NULL lookup_page_cgroup() > actually date from before you reworked page cgroup assignment, > and they're now redundant? If so, you'd do better to remove > all the checks, and Balbir put an explicit check in his code. >
I agree, it needs investigation. I would propose converting them to a VM_BUG_ON() and then consider removing them, just to catch potential problems, in case we miss anything.
> Alternatively, could the SPARSEMEM case have been corrupting or > otherwise misbehaving in a hidden way until now? Seems unlikely.
Agreed.
-- Balbir
| |