From mboxrd@z Thu Jan 1 00:00:00 1970 From: Greg Thelen Subject: Re: [PATCH v2] memcg: reduce the size of struct memcg 244-fold. Date: Wed, 23 Jan 2013 23:50:31 -0800 Message-ID: References: <1359009996-5350-1-git-send-email-glommer@parallels.com> Mime-Version: 1.0 Return-path: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-received:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version:content-type; bh=yesNbGQqRiWnL4IRNHII/+fwPXVTyOTgMqWU/tMSQSw=; b=pbHFtUSDI4Jh9ulS2aeN0cBXqlqkO+iSHkEF6W7+jzQOs4Kd6fYzKGPZE5RAZ15D6F nrKJHndFKK2SDXPMBBn/lOv4ePmzDF8z1jseRVx/Bryswj3ANGXYXhchVOAnUfhnzFGe Lfax1UwQHn7fy9e+IFgpKwBh8xOxnBqaQUc+/rN4QDFE2jTeCvJR2W8KUU/Z0G0T5wJD ZEHyK6GuKUXNAUpfJEtNIajf59cBVJP2KQlW+u3v44aakxTH8TaaFA94zu++pNaUZtDg njykxi9S0y2qCb9Z47p/FtvOvU+ZIQZJgd9iDOexc4/UeaHR27SeyTWMQUPgFS5aqb8X lK9w== In-Reply-To: <1359009996-5350-1-git-send-email-glommer@parallels.com> (Glauber Costa's message of "Thu, 24 Jan 2013 10:46:35 +0400") Sender: owner-linux-mm@kvack.org List-ID: Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Glauber Costa Cc: linux-mm@kvack.org, cgroups@vger.kernel.org, Michal Hocko , Kamezawa Hiroyuki , Johannes Weiner , Hugh Dickins , Ying Han , Mel Gorman , Rik van Riel On Wed, Jan 23 2013, Glauber Costa wrote: > In order to maintain all the memcg bookkeeping, we need per-node > descriptors, which will in turn contain a per-zone descriptor. > > Because we want to statically allocate those, this array ends up being > very big. Part of the reason is that we allocate something large enough > to hold MAX_NUMNODES, the compile time constant that holds the maximum > number of nodes we would ever consider. > > However, we can do better in some cases if the firmware help us. This is > true for modern x86 machines; coincidentally one of the architectures in > which MAX_NUMNODES tends to be very big. > > By using the firmware-provided maximum number of nodes instead of > MAX_NUMNODES, we can reduce the memory footprint of struct memcg > considerably. In the extreme case in which we have only one node, this > reduces the size of the structure from ~ 64k to ~2k. This is > particularly important because it means that we will no longer resort to > the vmalloc area for the struct memcg on defconfigs. We also have enough > room for an extra node and still be outside vmalloc. > > One also has to keep in mind that with the industry's ability to fit > more processors in a die as fast as the FED prints money, a nodes = 2 > configuration is already respectably big. > > [ v2: use size_t for size calculations ] > Signed-off-by: Glauber Costa > Cc: Michal Hocko > Cc: Kamezawa Hiroyuki > Cc: Johannes Weiner > Cc: Greg Thelen > Cc: Hugh Dickins > Cc: Ying Han > Cc: Mel Gorman > Cc: Rik van Riel Reviewed-by: Greg Thelen > --- > mm/memcontrol.c | 40 +++++++++++++++++++++++++--------------- > 1 file changed, 25 insertions(+), 15 deletions(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 09255ec..09d8b02 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -172,7 +172,7 @@ struct mem_cgroup_per_node { > }; > > struct mem_cgroup_lru_info { > - struct mem_cgroup_per_node *nodeinfo[MAX_NUMNODES]; > + struct mem_cgroup_per_node *nodeinfo[0]; It seems like a VM_BUG_ON() in mem_cgroup_zoneinfo() asserting that the nid index is less than nr_node_ids would be good at catching illegal indexes. I don't see any illegal indexes in your patch, but I fear that someday a MAX_NUMNODES based for() loop might sneak in. > }; > > /* > @@ -276,17 +276,6 @@ struct mem_cgroup { > */ > struct res_counter kmem; > /* > - * Per cgroup active and inactive list, similar to the > - * per zone LRU lists. > - */ > - struct mem_cgroup_lru_info info; > - int last_scanned_node; > -#if MAX_NUMNODES > 1 > - nodemask_t scan_nodes; > - atomic_t numainfo_events; > - atomic_t numainfo_updating; > -#endif > - /* > * Should the accounting and control be hierarchical, per subtree? > */ > bool use_hierarchy; > @@ -349,8 +338,29 @@ struct mem_cgroup { > /* Index in the kmem_cache->memcg_params->memcg_caches array */ > int kmemcg_id; > #endif > + > + int last_scanned_node; > +#if MAX_NUMNODES > 1 > + nodemask_t scan_nodes; > + atomic_t numainfo_events; > + atomic_t numainfo_updating; > +#endif > + /* > + * Per cgroup active and inactive list, similar to the > + * per zone LRU lists. > + * > + * WARNING: This has to be the last element of the struct. Don't > + * add new fields after this point. > + */ > + struct mem_cgroup_lru_info info; > }; > > +static inline size_t memcg_size(void) > +{ > + return sizeof(struct mem_cgroup) + > + nr_node_ids * sizeof(struct mem_cgroup_per_node); > +} > + Tangential question: why use inline here? I figure that modern compilers are good at making inlining decisions. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org