lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <alpine.LSU.2.00.1210181129180.2137@eggly.anvils>
Date:	Thu, 18 Oct 2012 12:03:44 -0700 (PDT)
From:	Hugh Dickins <hughd@...gle.com>
To:	Wen Congyang <wency@...fujitsu.com>
cc:	Johannes Weiner <hannes@...xchg.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	cgroups@...r.kernel.org, linux-mm@...ck.org,
	Jiang Liu <liuj97@...il.com>, mhocko@...e.cz,
	bsingharora@...il.com,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Konstantin Khlebnikov <khlebnikov@...nvz.org>,
	paul.gortmaker@...driver.com
Subject: Re: [PATCH] memory cgroup: update root memory cgroup when node is
 onlined

On Tue, 16 Oct 2012, Wen Congyang wrote:
> At 09/14/2012 09:36 AM, Hugh Dickins Wrote:
> > 
> > Description to be filled in later: would it be needed for -stable,
> > or is onlining already broken in other ways that you're now fixing up?
> > 
> > Reported-by: Tang Chen <tangchen@...fujitsu.com>
> > Signed-off-by: Hugh Dickins <hughd@...gle.com>
> 
> Hi, all:
> 
> What about the status of this patch?

Sorry I'm being so unresponsive at the moment (or, as usual).

When I sent the fixed version afterwards (minus mistaken VM_BUG_ON,
plus safer mem_cgroup_force_empty_list), I expected you or Konstantin
to respond with a patch to fix it as you preferred (at offline/online);
so this was on hold until we could compare and decide between them.

In the meantime, I assume, we've all come to feel that this way is
simple, and probably the best way for now; or at least good enough,
and we all have better things to do than play with alternatives.

I'll write up the description of the fixed version, and post it for
3.7, including the Acks from Hannes and KAMEZAWA (assuming they carry
forward to the second version) - but probably not today or tomorrow.

But please help me: I still don't know if it's needed for -stable.
We introduced the bug in 3.5, but I see lots of memory hotplug fixes
coming by from you and others, so I do not know if this lruvec->zone
fix is useful by itself in 3.5 and 3.6, or not - please tell me.

Thanks,
Hugh

> 
> Thanks
> Wen Congyang
> 
> > ---
> > 
> >  include/linux/mmzone.h |    2 -
> >  mm/memcontrol.c        |   40 ++++++++++++++++++++++++++++++++-------
> >  mm/mmzone.c            |    6 -----
> >  mm/page_alloc.c        |    2 -
> >  4 files changed, 36 insertions(+), 14 deletions(-)
> > 
> > --- 3.6-rc5/include/linux/mmzone.h	2012-08-03 08:31:26.892842267 -0700
> > +++ linux/include/linux/mmzone.h	2012-09-13 17:07:51.893772372 -0700
> > @@ -744,7 +744,7 @@ extern int init_currently_empty_zone(str
> >  				     unsigned long size,
> >  				     enum memmap_context context);
> >  
> > -extern void lruvec_init(struct lruvec *lruvec, struct zone *zone);
> > +extern void lruvec_init(struct lruvec *lruvec);
> >  
> >  static inline struct zone *lruvec_zone(struct lruvec *lruvec)
> >  {
> > --- 3.6-rc5/mm/memcontrol.c	2012-08-03 08:31:27.060842270 -0700
> > +++ linux/mm/memcontrol.c	2012-09-13 17:46:36.870804625 -0700
> > @@ -1061,12 +1061,25 @@ struct lruvec *mem_cgroup_zone_lruvec(st
> >  				      struct mem_cgroup *memcg)
> >  {
> >  	struct mem_cgroup_per_zone *mz;
> > +	struct lruvec *lruvec;
> >  
> > -	if (mem_cgroup_disabled())
> > -		return &zone->lruvec;
> > +	if (mem_cgroup_disabled()) {
> > +		lruvec = &zone->lruvec;
> > +		goto out;
> > +	}
> >  
> >  	mz = mem_cgroup_zoneinfo(memcg, zone_to_nid(zone), zone_idx(zone));
> > -	return &mz->lruvec;
> > +	lruvec = &mz->lruvec;
> > +out:
> > +	/*
> > +	 * Since a node can be onlined after the mem_cgroup was created,
> > +	 * we have to be prepared to initialize lruvec->zone here.
> > +	 */
> > +	if (unlikely(lruvec->zone != zone)) {
> > +		VM_BUG_ON(lruvec->zone);
> > +		lruvec->zone = zone;
> > +	}
> > +	return lruvec;
> >  }
> >  
> >  /*
> > @@ -1093,9 +1106,12 @@ struct lruvec *mem_cgroup_page_lruvec(st
> >  	struct mem_cgroup_per_zone *mz;
> >  	struct mem_cgroup *memcg;
> >  	struct page_cgroup *pc;
> > +	struct lruvec *lruvec;
> >  
> > -	if (mem_cgroup_disabled())
> > -		return &zone->lruvec;
> > +	if (mem_cgroup_disabled()) {
> > +		lruvec = &zone->lruvec;
> > +		goto out;
> > +	}
> >  
> >  	pc = lookup_page_cgroup(page);
> >  	memcg = pc->mem_cgroup;
> > @@ -1113,7 +1129,17 @@ struct lruvec *mem_cgroup_page_lruvec(st
> >  		pc->mem_cgroup = memcg = root_mem_cgroup;
> >  
> >  	mz = page_cgroup_zoneinfo(memcg, page);
> > -	return &mz->lruvec;
> > +	lruvec = &mz->lruvec;
> > +out:
> > +	/*
> > +	 * Since a node can be onlined after the mem_cgroup was created,
> > +	 * we have to be prepared to initialize lruvec->zone here.
> > +	 */
> > +	if (unlikely(lruvec->zone != zone)) {
> > +		VM_BUG_ON(lruvec->zone);
> > +		lruvec->zone = zone;
> > +	}
> > +	return lruvec;
> >  }
> >  
> >  /**
> > @@ -4742,7 +4768,7 @@ static int alloc_mem_cgroup_per_zone_inf
> >  
> >  	for (zone = 0; zone < MAX_NR_ZONES; zone++) {
> >  		mz = &pn->zoneinfo[zone];
> > -		lruvec_init(&mz->lruvec, &NODE_DATA(node)->node_zones[zone]);
> > +		lruvec_init(&mz->lruvec);
> >  		mz->usage_in_excess = 0;
> >  		mz->on_tree = false;
> >  		mz->memcg = memcg;
> > --- 3.6-rc5/mm/mmzone.c	2012-08-03 08:31:27.064842271 -0700
> > +++ linux/mm/mmzone.c	2012-09-13 17:06:28.921766001 -0700
> > @@ -87,7 +87,7 @@ int memmap_valid_within(unsigned long pf
> >  }
> >  #endif /* CONFIG_ARCH_HAS_HOLES_MEMORYMODEL */
> >  
> > -void lruvec_init(struct lruvec *lruvec, struct zone *zone)
> > +void lruvec_init(struct lruvec *lruvec)
> >  {
> >  	enum lru_list lru;
> >  
> > @@ -95,8 +95,4 @@ void lruvec_init(struct lruvec *lruvec,
> >  
> >  	for_each_lru(lru)
> >  		INIT_LIST_HEAD(&lruvec->lists[lru]);
> > -
> > -#ifdef CONFIG_MEMCG
> > -	lruvec->zone = zone;
> > -#endif
> >  }
> > --- 3.6-rc5/mm/page_alloc.c	2012-08-22 14:25:39.508279046 -0700
> > +++ linux/mm/page_alloc.c	2012-09-13 17:06:08.265763526 -0700
> > @@ -4456,7 +4456,7 @@ static void __paginginit free_area_init_
> >  		zone->zone_pgdat = pgdat;
> >  
> >  		zone_pcp_init(zone);
> > -		lruvec_init(&zone->lruvec, zone);
> > +		lruvec_init(&zone->lruvec);
> >  		if (!size)
> >  			continue;
> >  
> > 
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ