Re: [PATCH] [Cleanup] memcg: export memory cgroup's swappiness v2

[KAMEZAWA Hiroyuki]

On Thu, 30 Jun 2011 13:01:34 -0700
Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:

> On Wed, 29 Jun 2011 19:03:25 +0900
> KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx> wrote:
> 
> > Each memory cgroup has 'swappiness' value and it can be accessed by
> > get_swappiness(memcg). The major user is try_to_free_mem_cgroup_pages()
> > and swappiness is passed by argument. It's propagated by scan_control.
> > 
> > get_swappiness is static function but some planned updates will need to
> > get swappiness from files other than memcontrol.c
> > This patch exports get_swappiness() as mem_cgroup_swappiness().
> > By this, we can remove the argument of swapiness from try_to_free...
> > and drop swappiness from scan_control. only memcg uses it.
> 
> x86_64 allnoconfig (aka Documentation/SubmitChecklist, section 2b):
> 
> mm/vmscan.c: In function 'vmscan_swappiness':
> mm/vmscan.c:1734: error: implicit declaration of function 'mem_cgroup_swappiness'
> 
> This is pretty broken.  I think we do want to implement this for
> CONFIG_CGROUP_MEM_RES_CTLR=y, CONFIG_SWAP=n:
> 
> --- a/include/linux/swap.h~memcg-export-memory-cgroups-swappiness-fix
> +++ a/include/linux/swap.h
> @@ -365,17 +365,12 @@ static inline void put_swap_token(struct
>  extern void
>  mem_cgroup_uncharge_swapcache(struct page *page, swp_entry_t ent, bool swapout);
>  extern int mem_cgroup_count_swap_user(swp_entry_t ent, struct page **pagep);
> -extern unsigned int mem_cgroup_swappiness(struct mem_cgroup *mem);
>  #else
>  static inline void
>  mem_cgroup_uncharge_swapcache(struct page *page, swp_entry_t ent, bool swapout)
>  {
>  }
>  
> -static inline unsigned int mem_cgroup_swappiness(struct mem_cgroup *mem)
> -{
> -	return vm_swappiness;
> -}
>  #endif
>  #ifdef CONFIG_CGROUP_MEM_RES_CTLR_SWAP
>  extern void mem_cgroup_uncharge_swap(swp_entry_t ent);
> @@ -510,5 +505,15 @@ mem_cgroup_count_swap_user(swp_entry_t e
>  #endif
>  
>  #endif /* CONFIG_SWAP */
> +
> +#ifdef CONFIG_CGROUP_MEM_RES_CTLR
> +extern unsigned int mem_cgroup_swappiness(struct mem_cgroup *mem);
> +#else
> +static inline unsigned int mem_cgroup_swappiness(struct mem_cgroup *mem)
> +{
> +	return vm_swappiness;
> +}
> +#endif
> +
>  #endif /* __KERNEL__*/
>  #endif /* _LINUX_SWAP_H */
> 
> But what is the role of CONFIG_CGROUP_MEM_RES_CTLR_SWAP?
> 
> And in the above circumstances, vmscan_swappiness() devolves into
> 
> static int vmscan_swappiness(struct scan_control *sc)
> {
>        if (scanning_global_lru(sc))
>                return vm_swappiness;
>        return vm_swappiness;
> }
> 
> which I guess makes sense but seems a bit odd.
> 
> Anyway, my confidence level is low so I think I'll drop this patch. 
> Please have a think about the interplay between
> CONFIG_CGROUP_MEM_RES_CTLR, CONFIG_CGROUP_MEM_RES_CTLR_SWAP and
> CONFIG_SWAP.
> 
> 

Ok, I'll check it. Maybe I miss !CONFIG_SWAP...

Thanks,
-Kame

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/