memcg: charge page tables (x86) and pipe buffers

Currently, to charge a page to kmemcg one should use alloc_kmem_pages helper. When the page is not needed anymore it must be freed with free_kmem_pages helper, which will uncharge the page before freeing it. Such a design is acceptable for thread info pages and kmalloc large allocations, which are currently the only users of alloc_kmem_pages, but it gets extremely inconvenient if one wants to make use of batched free (e.g. to charge page tables - see release_pages) or page reference counter (pipe buffers - see anon_pipe_buf_release).

To overcome this limitation, this patch moves kmemcg uncharge code to the generic free path and zaps free_kmem_pages helper. To distinguish kmem pages from other page types, it makes alloc_kmem_pages initialize page->_mapcount to a special value and introduces a new PageKmem helper, which returns true if it sees this value.

Quoted 9 lines · On Sat, 26 Sep 2015 13:45:53 +0300 Vladimir Davydov <[email protected]>…

Currently, to charge a page to kmemcg one should use alloc_kmem_pages helper. When the page is not needed anymore it must be freed with free_kmem_pages helper, which will uncharge the page before freeing it. Such a design is acceptable for thread info pages and kmalloc large allocations, which are currently the only users of alloc_kmem_pages, but it gets extremely inconvenient if one wants to make use of batched free (e.g. to charge page tables - see release_pages) or page reference counter (pipe buffers - see anon_pipe_buf_release).

Quoted 5 lines

To overcome this limitation, this patch moves kmemcg uncharge code to the generic free path and zaps free_kmem_pages helper. To distinguish kmem pages from other page types, it makes alloc_kmem_pages initialize page->_mapcount to a special value and introduces a new PageKmem helper, which returns true if it sees this value.

As far as I can tell, this new use of page._mapcount is OK, but...

- The documentation for _mapcount needs to be updated (mm_types.h)

- Don't believe the documentation! Because someone else may have done what you tried to do. Please manually audit mm/ for _mapcount uses.

- One such use is "For recording whether a page is in the buddy system, we set ->_mapcount PAGE_BUDDY_MAPCOUNT_VALUE". Please update the comment for this while you're in there. (Including description of the state's lifetime).

- And please update _mapcount docs for PageBalloon()

- Why is the code accessing ->_mapcount directly? afaict page_mapcount() and friends will work OK?

- The patch adds overhead to all kernels, even non-kmemcg and non-memcg kernels. Bad. Fixable?

- PAGE_BUDDY_MAPCOUNT_VALUE, PAGE_BALLOON_MAPCOUNT_VALUE and PAGE_KMEM_MAPCOUNT_VALUE should all be put next to each other so readers can see all the possible values and so we don't get duplicates, etc.

Currently, to charge a page to kmemcg one should use alloc_kmem_pages helper. When the page is not needed anymore it must be freed with free_kmem_pages helper, which will uncharge the page before freeing it. Such a design is acceptable for thread info pages and kmalloc large allocations, which are currently the only users of alloc_kmem_pages, but it gets extremely inconvenient if one wants to make use of batched free (e.g. to charge page tables - see release_pages) or page reference counter (pipe buffers - see anon_pipe_buf_release).

To overcome this limitation, this patch moves kmemcg uncharge code to the generic free path and zaps free_kmem_pages helper. To distinguish kmem pages from other page types, it makes alloc_kmem_pages initialize page->_mapcount to a special value and introduces a new PageKmem helper, which returns true if it sees this value.

Signed-off-by Vladimir Davydov <[email protected]>

---
 include/linux/gfp.h        |  3 ---
 include/linux/page-flags.h | 22 ++++++++++++++++++++++
 kernel/fork.c              |  2 +-
 mm/page_alloc.c            | 26 ++++++++------------------
 mm/slub.c                  |  2 +-
 mm/swap.c                  |  3 ++-
 6 files changed, 34 insertions(+), 24 deletions(-)

include/linux/gfp.h +0 -3

index f92cbd2f4450..b46147c45966 100644
@@ -384,9 +384,6 @@ extern void *__alloc_page_frag(struct page_frag_cache *nc,
 			       unsigned int fragsz, gfp_t gfp_mask);
 extern void __free_page_frag(void *addr);
 
-extern void __free_kmem_pages(struct page *page, unsigned int order);
-extern void free_kmem_pages(unsigned long addr, unsigned int order);
-
 #define __free_page(page) __free_pages((page), 0)
 #define free_page(addr) free_pages((addr), 0)
 

include/linux/page-flags.h +4 -0

index 416509e26d6d..a190719c2f46 100644
@@ -594,6 +594,28 @@ static inline void __ClearPageBalloon(struct page *page)
 }
 
 /*
+ * PageKmem() returns true if the page was allocated with alloc_kmem_pages().
+ */
+#define PAGE_KMEM_MAPCOUNT_VALUE (-512)
+

include/linux/page-flags.h +7 -0

+static inline void __ClearPageKmem(struct page *page)
+{
+	VM_BUG_ON_PAGE(!PageKmem(page), page);
+	atomic_set(&page->_mapcount, -1);
+}
+
+/*
  * If network-based swap is enabled, sl*b must keep track of whether pages
  * were allocated from pfmemalloc reserves.
  */

kernel/fork.c +1 -1

index 2845623fb582..c23f8a17e99e 100644
@@ -169,7 +169,7 @@ static struct thread_info *alloc_thread_info_node(struct task_struct *tsk,
 
 static inline void free_thread_info(struct thread_info *ti)
 {
-	free_kmem_pages((unsigned long)ti, THREAD_SIZE_ORDER);
+	free_pages((unsigned long)ti, THREAD_SIZE_ORDER);
 }
 # else
 static struct kmem_cache *thread_info_cache;

mm/page_alloc.c +8 -18

index 48aaf7b9f253..88d85367c81e 100644
@@ -942,6 +942,10 @@ static bool free_pages_prepare(struct page *page, unsigned int order)
 
 	if (PageAnon(page))
 		page->mapping = NULL;
+	if (PageKmem(page)) {
+		memcg_kmem_uncharge_pages(page, order);
+		__ClearPageKmem(page);
+	}
 	bad += free_pages_check(page);
 	for (i = 1; i < (1 << order); i++) {
 		if (compound)

@@ -3434,6 +3438,8 @@ struct page *alloc_kmem_pages(gfp_t gfp_mask, unsigned int order)
 		return NULL;
 	page = alloc_pages(gfp_mask, order);
 	memcg_kmem_commit_charge(page, memcg, order);
+	if (page)
+		__SetPageKmem(page);
 	return page;
 }
 

@@ -3446,27 +3452,11 @@ struct page *alloc_kmem_pages_node(int nid, gfp_t gfp_mask, unsigned int order)
 		return NULL;
 	page = alloc_pages_node(nid, gfp_mask, order);
 	memcg_kmem_commit_charge(page, memcg, order);
+	if (page)
+		__SetPageKmem(page);
 	return page;
 }
 
-/*
- * __free_kmem_pages and free_kmem_pages will free pages allocated with
- * alloc_kmem_pages.
- */
-void __free_kmem_pages(struct page *page, unsigned int order)
-{
-	memcg_kmem_uncharge_pages(page, order);
-	__free_pages(page, order);
-}
-
-void free_kmem_pages(unsigned long addr, unsigned int order)
-{
-	if (addr != 0) {
-		VM_BUG_ON(!virt_addr_valid((void *)addr));
-		__free_kmem_pages(virt_to_page((void *)addr), order);
-	}
-}
-
 static void *make_alloc_exact(unsigned long addr, unsigned order, size_t size)
 {
 	if (addr) {

mm/slub.c +1 -1

index f614b5dc396b..f5248a7d9438 100644
@@ -3516,7 +3516,7 @@ void kfree(const void *x)
 	if (unlikely(!PageSlab(page))) {
 		BUG_ON(!PageCompound(page));
 		kfree_hook(x);
-		__free_kmem_pages(page, compound_order(page));
+		__free_pages(page, compound_order(page));
 		return;
 	}
 	slab_free(page->slab_cache, page, object, _RET_IP_);

mm/swap.c +0 -0

index 983f692a47fd..8d8d03118a18 100644

Vladimir Davydov wrote:

Quoted 12 lines

include/linux/page-flags.h +4 -0

index 416509e26d6d..a190719c2f46 100644
@@ -594,6 +594,28 @@ static inline void __ClearPageBalloon(struct page *page)
 }
 
 /*
+ * PageKmem() returns true if the page was allocated with alloc_kmem_pages().
+ */
+#define PAGE_KMEM_MAPCOUNT_VALUE (-512)
+

include/linux/page-flags.h static inline void __ClearPageBalloon(struct page *page) · line 610

Quoted 10 lines

include/linux/page-flags.h +10 -0

+static inline int PageKmem(struct page *page)
+{
+	return atomic_read(&page->_mapcount) == PAGE_KMEM_MAPCOUNT_VALUE;
+}
+
+static inline void __SetPageKmem(struct page *page)
+{
+	VM_BUG_ON_PAGE(atomic_read(&page->_mapcount) != -1, page);
+	atomic_set(&page->_mapcount, PAGE_KMEM_MAPCOUNT_VALUE);
+}

What do you think about several special mapcount values for various types of kmem?

It's helps user and administrators break down memory usage.

A nice equation is:
memory.usage_in_bytes = memory.stat[file + anon + unevictable + kmem]

Next, it's helpful to be able to breakdown kmem into:
kmem = stack + pgtable + slab + ...

On one hand (and the kernel I use internally) we can use separate per memcg counters for each kmem type. Then reconstitute memory.kmem as needed by adding them together. But using keeping a single kernel kmem counter is workable if there is a way to breakdown the memory charge to a container (e.g. by walking /proc/kpageflags-ish or per memcg memory.kpageflags-ish file).

Pipe buffers can be generated unrestrictedly by an unprivileged userspace process, so they shouldn't go unaccounted.

...

Quoted 10 lines · On Sat, 26 Sep 2015 13:45:54 +0300 Vladimir Davydov <[email protected]>…

Pipe buffers can be generated unrestrictedly by an unprivileged userspace process, so they shouldn't go unaccounted.

...

fs/pipe.c +0 -0

@@ -400,7 +400,7 @@ pipe_write(struct kiocb *iocb, struct iov_iter *from)
 			int copied;
 

fs/pipe.c pipe_write(struct kiocb *iocb, struct iov_iter *from) · line 406

Quoted 6 lines

fs/pipe.c +1 -1

 			if (!page) {
-				page = alloc_page(GFP_HIGHUSER);
+				page = alloc_kmem_pages(GFP_HIGHUSER, 0);
 				if (unlikely(!page)) {
 					ret = ret ? : -ENOMEM;
 					break;

This seems broken. We have a page buffer page which has a weird ->mapcount. Now it gets stolen (generic_pipe_buf_steal()) and spliced into pagecache. Then the page gets mmapped and MM starts playing with its ->_mapcount?