On Tue, 7 Jun 2011 11:18:57 +0100
Mel Gorman <mgorman@xxxxxxx> wrote:
> On Tue, Jun 07, 2011 at 06:33:02PM +0900, KAMEZAWA Hiroyuki wrote:
> > On Tue, 7 Jun 2011 10:09:00 +0100
> > Mel Gorman <mgorman@xxxxxxx> wrote:
> >
> > > I should have said "nodes" even though the end result is the same. The
> > > problem at the moment is cgroup initialisation is checking PFNs outside
> > > node boundaries. It should be ensuring that the start and end PFNs it
> > > uses are within boundaries.
> > >
> > Maybe you like this kind of fix. Yes, this can fix the problem on bugzilla.
> > My concern is this will not work for ARM.
> >
> > This patch (and all other patch) works on my test host.
> > ==
> > make sparsemem's page_cgroup_init to be node aware.
> >
> > With sparsemem, page_cgroup_init scans pfn from 0 to max_pfn.
> > But this may scan a pfn which is not on any node and can access
> > memmap which is not initialized.
> >
> > This makes page_cgroup_init() for SPARSEMEM node aware.
> >
> > Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx>
> > ---
> > mm/page_cgroup.c | 26 ++++++++++++++++++++++----
> > 1 file changed, 22 insertions(+), 4 deletions(-)
> >
> > Index: linux-3.0-rc1/mm/page_cgroup.c
> > ===================================================================
> > --- linux-3.0-rc1.orig/mm/page_cgroup.c
> > +++ linux-3.0-rc1/mm/page_cgroup.c
> > @@ -285,14 +285,32 @@ void __init page_cgroup_init(void)
> > {
> > unsigned long pfn;
> > int fail = 0;
> > + int node;
> >
> > if (mem_cgroup_disabled())
> > return;
> >
> > - for (pfn = 0; !fail && pfn < max_pfn; pfn += PAGES_PER_SECTION) {
> > - if (!pfn_present(pfn))
> > - continue;
> > - fail = init_section_page_cgroup(pfn);
> > + for_each_node_state(node, N_HIGH_MEMORY) {
> > + unsigned long start_pfn, end_pfn;
> > +
> > + start_pfn = NODE_DATA(node)->node_start_pfn;
> > + end_pfn = start_pfn + NODE_DATA(node)->node_spanned_pages;
> > + /*
> > + * This calculation makes sure that this nid detection for
> > + * section can work even if node->start_pfn is not aligned to
> > + * section. For sections on not-node-boundary, we see head
> > + * page of sections.
> > + */
> > + for (pfn = start_pfn;
> > + !fail & (pfn < end_pfn);
>
> && instead of & there?
>
> > + pfn = ALIGN(pfn + PAGES_PER_SECTION, PAGES_PER_SECTION)) {
> > + if (!pfn_present(pfn))
> > + continue;
> > + /* Nodes can be overlapped */
> > + if (pfn_to_nid(pfn) != node)
> > + continue;
> > + fail = init_section_page_cgroup(pfn);
> > + }
>
> So this is finding the first valid PFN in a node. Even the
> overlapping problem should not be an issue here as unless node memory
> initialisation is overwriting flags belonging to other nodes. The
> paranoia does not hurt.
>
> I also don't think the ARM punching holes in the memmap is a problem
> because we'd at least expect the start of the node to be valid.
>
Ok, I'll post a fixed and cleaned one. (above patch has bug ;(
Thanks,
-Kame
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxxx For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>