Re: [memcg BUG] unable to handle kernel NULL pointer derefence at 00000000

KAMEZAWA Hiroyuki
2008-10-21T08:38:43Z

On Tue, 21 Oct 2008 09:34:54 +0100
mel@skynet.ie (Mel Gorman) wrote:

> On Tue, Oct 21, 2008 at 05:18:01PM +0900, KAMEZAWA Hiroyuki wrote:
> > On Tue, 21 Oct 2008 15:21:07 +0800
> > Li Zefan <lizf@cn.fujitsu.com> wrote:
> > > dmesg is attached.
> > >
> > Thanks....I think I caught some. (added Mel Gorman to CC:)
> >
> > NODE_DATA(nid)->spanned_pages just means sum of zone->spanned_pages in node.
> >
> > So, If there is a hole between zone, node->spanned_pages doesn't mean
> > length of node's memmap....(then, some hole can be skipped.)
> >
>
> This is correct. pgdat->node_spanned_pages is the range of PFNs the node
> covers. In some cases, this can even overlap other nodes. There can be
> memory holes and there is no guarantee there is memmap present for the holes.
> The number of actual pages is pgdat->node_present_pages.
>

Thank you for clarification.

> > OMG....Could you try this ?
> >
> > -Kame
> > ==
> > NODE_DATA(nid)->node_spanned_pages doesn't means width of node's memory
> > but means sum of spanned_pages in all zones of node.
> >
>
> Does not necessarily mean that either. Conceivably there could be gaps
> between the zones.
>
I see.

> > alloc_node_page_cgroup() misunderstand it. This patch tries to use
> > the same algorithm as alloc_node_mem_map() for allocating page_cgroup()
> > for node.
> >
> > Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> >
> > mm/page_cgroup.c | 17 ++++++++++++++---
> > 1 file changed, 14 insertions(+), 3 deletions(-)
> >
> > Index: linux-2.6.27/mm/page_cgroup.c
> > ===================================================================
> > --- linux-2.6.27.orig/mm/page_cgroup.c
> > +++ linux-2.6.27/mm/page_cgroup.c
> > @@ -41,10 +41,18 @@ static int __init alloc_node_page_cgroup
> > {
> > struct page_cgroup *base, *pc;
> > unsigned long table_size;
> > - unsigned long start_pfn, nr_pages, index;
> > + unsigned long start, end, start_pfn, nr_pages, index;
> >
> > - start_pfn = NODE_DATA(nid)->node_start_pfn;
> > - nr_pages = NODE_DATA(nid)->node_spanned_pages;
> > + /*
> > + * Instead of allocating page_cgroup for [start, end)
> > + * We allocate page_cgroup to the same size of mem_map.
> > + * See page_alloc.c::alloc_node_mem_map()
> > + */
> > + start = NODE_DATA(nid)->node_start_pfn & ~(MAX_ORDER_NR_PAGES - 1);
> > + end = NODE_DATA(nid)->node_start_pfn
> > + + NODE_DATA(nid)->node_spanned_pages;
> > + end = ALIGN(end, MAX_ORDER_NR_PAGES);
> > + nr_pages = end - start;
> >
>
> I don't know what this function is doing, but that will calculate nr_pages
> to be the full width of a node, holes and all which is what I think you're
> trying to do. Again, remember this could cover another node as you can have
> a situation where the pfn ranges are
>
> node1_pages | node0_pages | node1_pages
> start <---------------------------------------------->end
>
> Maybe this is not a problem for you. It all depends on how you map a PFN
> to a table. There is also a concern for memory usage as;
>
> > table_size = sizeof(struct page_cgroup) * nr_pages;
> >
>
> this is potentially a very large table.
>

yes. I know. usual big-address-space people will use SPARSEMEM version.

Thanks,
-Kame