On 07/27/2011 09:58 AM, Michal Hocko wrote:
On Tue 26-07-11 14:17:54, Andrew Morton wrote:
On Fri, 10 Jun 2011 18:57:40 +0200
Igor Mammedov<imammedo@xxxxxxxxxx> wrote:
On 06/08/2011 11:09 PM, Andrew Morton wrote:
The original patch:
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -4707,7 +4707,6 @@ static int alloc_mem_cgroup_per_zone_info(struct mem_cgroup *mem, int node)
if (!pn)
return 1;
- mem->info.nodeinfo[node] = pn;
for (zone = 0; zone< MAX_NR_ZONES; zone++) {
mz =&pn->zoneinfo[zone];
for_each_lru(l)
@@ -4716,6 +4715,7 @@ static int alloc_mem_cgroup_per_zone_info(struct mem_cgroup *mem, int node)
mz->on_tree = false;
mz->mem = mem;
}
+ mem->info.nodeinfo[node] = pn;
return 0;
}
looks like a really good idea. But it needs a new changelog and I'd be
a bit reluctant to merge it as it appears that the aptch removes our
only known way of reproducing a bug.
So for now I think I'll queue the patch up unchangelogged so the issue
doesn't get forgotten about.
Problem was in rhel's xen hv.
It was missing fix for imul emulation.
Details here
http://lists.xensource.com/archives/html/xen-devel/2011-06/msg00801.html
Thanks to Tim Deegan and everyone who was involved in the discussion.
I really don't want to trawl through a lengthy xen bug report
The bug turned out to be Xen specific and this patch just hidden the bug
in Xen.
The problem was in incorrect imul instruction emulation in xen and as
consequence incorrect attempt to initialize list at invalid memory location.
and write your changelog for you.
We still have no changelog for this patch. Please send one.
Appart from a better programming style is there any other reason for
taking it? If applied it might hide potential bugs when somebody is
touching data too early.
If it ever happens and somebody is touching data too early, it would be
a bit easier to diagnose a problem when dereferencing NULL at
mem->info.nodeinfo[node] than wondering at partly initialized
mem_cgroup_per_zone. Aside from that it is purely cosmetic change.
Here is proposed change log:
Subject: Cleanup: memcg: Expose only initialized mem_cgroup_per_node to world
If somebody is touching data too early, it might be easier to diagnose
a problem when dereferencing NULL at mem->info.nodeinfo[node] than
trying to understand why mem_cgroup_per_zone is [un|partly]initialized.
Michal will you agree with such commit message?
--
Thanks,
Igor
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. 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>