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

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20110630130134.63a1dd37.akpm@linux-foundation.org>
Date:	Thu, 30 Jun 2011 13:01:34 -0700
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>
Cc:	"linux-mm@...ck.org" <linux-mm@...ck.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"nishimura@....nes.nec.co.jp" <nishimura@....nes.nec.co.jp>,
	"bsingharora@...il.com" <bsingharora@...il.com>,
	Michal Hocko <mhocko@...e.cz>, Ying Han <yinghan@...gle.com>
Subject: Re: [PATCH] [Cleanup] memcg: export memory cgroup's swappiness v2

On Wed, 29 Jun 2011 19:03:25 +0900
KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com> 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.


--
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/