Re: [PATCH v2] memcg: reduce the size of struct memcg 244-fold.

[Johannes Weiner <hannes@cmpxchg.org>]

On Thu, Jan 24, 2013 at 03:51:05PM -0800, Andrew Morton wrote:
> On Wed, 23 Jan 2013 23:50:31 -0800
> Greg Thelen <gthelen@google.com> wrote:
> 
> > > --- 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.
> 
> Can't hurt ;)
> 
> > Tangential question: why use inline here?  I figure that modern
> > compilers are good at making inlining decisions.
> 
> And that'll save some disk space.
> 
> This?
> 
> --- a/mm/memcontrol.c~memcg-reduce-the-size-of-struct-memcg-244-fold-fix
> +++ a/mm/memcontrol.c
> @@ -381,7 +381,7 @@ enum {
>  		((1 << KMEM_ACCOUNTED_ACTIVE) | (1 << KMEM_ACCOUNTED_ACTIVATED))
>  
>  #ifdef CONFIG_MEMCG_KMEM
> -static inline void memcg_kmem_set_active(struct mem_cgroup *memcg)
> +static void memcg_kmem_set_active(struct mem_cgroup *memcg)
>  {
>  	set_bit(KMEM_ACCOUNTED_ACTIVE, &memcg->kmem_account_flags);
>  }

I don't disapprove, but it's the wrong function for this patch.
Should be memcg_size().

--
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>