mm: net: memcg accounting for TCP rx zerocopy

PATCH Superseded Arjun Roy 3 participants 3 messages Mar 16, 2021 – Mar 22, 2021

Subsystems (7)

Networking Network drivers Memory management TCP +3 more subsystems

lore

Share & tools

lore

Arjun Roy

author

2021-03-16 01:30 UTC #0 lore raw

PATCH v2 1/1 mm: net: memcg accounting for TCP rx zerocopy

From: Arjun Roy <[email protected]>

TCP zerocopy receive is used by high performance network applications to further scale. For RX zerocopy, the memory containing the network data filled by the network driver is directly mapped into the address space of high performance applications. To keep the TLB cost low, these applications unmap the network memory in big batches. So, this memory can remain mapped for long time. This can cause a memory isolation issue as this memory becomes unaccounted after getting mapped into the application address space. This patch adds the memcg accounting for such memory.

Accounting the network memory comes with its own unique challenges. The high performance NIC drivers use page pooling to reuse the pages to eliminate/reduce expensive setup steps like IOMMU. These drivers keep an extra reference on the pages and thus we can not depend on the page reference for the uncharging. The page in the pool may keep a memcg pinned for arbitrary long time or may get used by other memcg.

This patch decouples the uncharging of the page from the refcnt and associates it with the map count i.e. the page gets uncharged when the last address space unmaps it. Now the question is, what if the driver drops its reference while the page is still mapped? That is fine as the address space also holds a reference to the page i.e. the reference count can not drop to zero before the map count.

Signed-off-by Arjun Roy <[email protected]>

Co-developed-by Shakeel Butt <[email protected]>

Signed-off-by Shakeel Butt <[email protected]>

Signed-off-by Eric Dumazet <[email protected]>

Signed-off-by Soheil Hassas Yeganeh <[email protected]>

---

Changelog since v1:
- Pages accounted for in this manner are now tracked via MEMCG_SOCK.
- v1 allowed for a brief period of double-charging, now we have a brief period of under-charging to avoid undue memory pressure.

 include/linux/memcontrol.h |  48 ++++++++++++-
 mm/memcontrol.c            | 138 +++++++++++++++++++++++++++++++++++++
 mm/rmap.c                  |   7 +-
 net/ipv4/tcp.c             |  79 +++++++++++++++++----
 4 files changed, 253 insertions(+), 19 deletions(-)

include/linux/memcontrol.h +45 -3

index eeb0b52203e9..158484018c8c 100644
@@ -339,11 +339,13 @@ extern struct mem_cgroup *root_mem_cgroup;
 
 enum page_memcg_data_flags {
 	/* page->memcg_data is a pointer to an objcgs vector */
-	MEMCG_DATA_OBJCGS = (1UL << 0),
+	MEMCG_DATA_OBJCGS	= (1UL << 0),
 	/* page has been accounted as a non-slab kernel page */
-	MEMCG_DATA_KMEM = (1UL << 1),
+	MEMCG_DATA_KMEM		= (1UL << 1),
+	/* page has been accounted as network memory */
+	MEMCG_DATA_SOCK		= (1UL << 2),
 	/* the next bit after the last actual flag */
-	__NR_MEMCG_DATA_FLAGS  = (1UL << 2),
+	__NR_MEMCG_DATA_FLAGS	= (1UL << 3),
 };
 
 #define MEMCG_DATA_FLAGS_MASK (__NR_MEMCG_DATA_FLAGS - 1)

@@ -434,6 +436,11 @@ static inline bool PageMemcgKmem(struct page *page)
 	return page->memcg_data & MEMCG_DATA_KMEM;
 }
 
+static inline bool PageMemcgSock(struct page *page)
+{
+	return page->memcg_data & MEMCG_DATA_SOCK;
+}
+
 #ifdef CONFIG_MEMCG_KMEM
 /*
  * page_objcgs - get the object cgroups vector associated with a page

@@ -1104,6 +1111,11 @@ static inline bool PageMemcgKmem(struct page *page)
 	return false;
 }
 
+static inline bool PageMemcgSock(struct page *page)
+{
+	return false;
+}
+
 static inline bool mem_cgroup_is_root(struct mem_cgroup *memcg)
 {
 	return true;

@@ -1570,6 +1582,15 @@ extern struct static_key_false memcg_sockets_enabled_key;
 #define mem_cgroup_sockets_enabled static_branch_unlikely(&memcg_sockets_enabled_key)
 void mem_cgroup_sk_alloc(struct sock *sk);
 void mem_cgroup_sk_free(struct sock *sk);
+
+void mem_cgroup_post_charge_sock_pages(struct mem_cgroup *memcg,
+				 unsigned int nr_pages);
+void mem_cgroup_uncharge_sock_page(struct page *page);
+int mem_cgroup_prepare_sock_pages(struct mem_cgroup *memcg, struct page **pages,
+			       u8 *page_prepared, unsigned int nr_pages);
+int mem_cgroup_unprepare_sock_pages(struct mem_cgroup *memcg, struct page **pages,
+				 u8 *page_prepared, unsigned int nr_pages);
+
 static inline bool mem_cgroup_under_socket_pressure(struct mem_cgroup *memcg)
 {
 	if (!cgroup_subsys_on_dfl(memory_cgrp_subsys) && memcg->tcpmem_pressure)

@@ -1598,6 +1619,27 @@ static inline void memcg_set_shrinker_bit(struct mem_cgroup *memcg,
 					  int nid, int shrinker_id)
 {
 }
+
+static inline void mem_cgroup_post_charge_sock_pages(struct mem_cgroup *memcg,
+					       unsigned int nr_pages)
+{
+}
+
+static inline void mem_cgroup_uncharge_sock_page(struct page *page)
+{
+}
+
+static inline int mem_cgroup_prepare_sock_pages(struct mem_cgroup *memcg, struct page **pages,
+			       u8 *page_prepared, unsigned int nr_pages)
+{
+	return 0;
+}
+
+static inline int mem_cgroup_unprepare_sock_pages(struct mem_cgroup *memcg, struct page **pages,
+				 u8 *page_prepared, unsigned int nr_pages)
+{
+	return 0;
+}
 #endif
 
 #ifdef CONFIG_MEMCG_KMEM

mm/memcontrol.c +138 -0

144 lines not on this page · raw message

mm/rmap.c +5 -2

index 08c56aaf72eb..7b112b088426 100644
@@ -1276,6 +1276,9 @@ static void page_remove_file_rmap(struct page *page, bool compound)
 
 	if (unlikely(PageMlocked(page)))
 		clear_page_mlock(page);
+
+	if (unlikely(PageMemcgSock(page)))
+		mem_cgroup_uncharge_sock_page(page);
 }
 
 static void page_remove_anon_compound_rmap(struct page *page)

@@ -1331,7 +1334,7 @@ static void page_remove_anon_compound_rmap(struct page *page)
  */
 void page_remove_rmap(struct page *page, bool compound)
 {
-	lock_page_memcg(page);
+	struct mem_cgroup *memcg = lock_page_memcg(page);
 
 	if (!PageAnon(page)) {
 		page_remove_file_rmap(page, compound);

@@ -1370,7 +1373,7 @@ void page_remove_rmap(struct page *page, bool compound)
 	 * faster for those pages still in swapcache.
 	 */
 out:
-	unlock_page_memcg(page);
+	__unlock_page_memcg(memcg);
 }
 
 /*

net/ipv4/tcp.c +65 ~~-14~~

197 lines not on this page · raw message

Signature · 1 line

2.31.0.rc2.261.g7f71774620-goog

Andrew Morton

maintainer

2021-03-18 03:21 UTC #1 lore raw

Quoted 19 lines · On Mon, 15 Mar 2021 18:30:03 -0700 Arjun Roy <[email protected]> wrote:

From: Arjun Roy <[email protected]>
TCP zerocopy receive is used by high performance network applications to further scale. For RX zerocopy, the memory containing the network data filled by the network driver is directly mapped into the address space of high performance applications. To keep the TLB cost low, these applications unmap the network memory in big batches. So, this memory can remain mapped for long time. This can cause a memory isolation issue as this memory becomes unaccounted after getting mapped into the application address space. This patch adds the memcg accounting for such memory.
Accounting the network memory comes with its own unique challenges. The high performance NIC drivers use page pooling to reuse the pages to eliminate/reduce expensive setup steps like IOMMU. These drivers keep an extra reference on the pages and thus we can not depend on the page reference for the uncharging. The page in the pool may keep a memcg pinned for arbitrary long time or may get used by other memcg.

Quoted 6 lines

This patch decouples the uncharging of the page from the refcnt and associates it with the map count i.e. the page gets uncharged when the last address space unmaps it. Now the question is, what if the driver drops its reference while the page is still mapped? That is fine as the address space also holds a reference to the page i.e. the reference count can not drop to zero before the map count.

What tree were you hoping to get this merged through? I'd suggest net
- it's more likely to get tested over there.

mm/memcontrol.c

Quoted 5 lines

...
mm/memcontrol.c +0 -0

These changes could be inside #ifdef CONFIG_NET. Although I expect MEMCG=y&&NET=n is pretty damn rare.

Arjun Roy

2021-03-22 21:19 UTC #2 lore raw

Quoted 29 lines · Andrew Morton

Quoted 19 lines · On Mon, 15 Mar 2021 18:30:03 -0700 Arjun Roy <[email protected]> wrote:
From: Arjun Roy <[email protected]>
TCP zerocopy receive is used by high performance network applications to further scale. For RX zerocopy, the memory containing the network data filled by the network driver is directly mapped into the address space of high performance applications. To keep the TLB cost low, these applications unmap the network memory in big batches. So, this memory can remain mapped for long time. This can cause a memory isolation issue as this memory becomes unaccounted after getting mapped into the application address space. This patch adds the memcg accounting for such memory.
Accounting the network memory comes with its own unique challenges. The high performance NIC drivers use page pooling to reuse the pages to eliminate/reduce expensive setup steps like IOMMU. These drivers keep an extra reference on the pages and thus we can not depend on the page reference for the uncharging. The page in the pool may keep a memcg pinned for arbitrary long time or may get used by other memcg.
Quoted 6 lines
This patch decouples the uncharging of the page from the refcnt and associates it with the map count i.e. the page gets uncharged when the last address space unmaps it. Now the question is, what if the driver drops its reference while the page is still mapped? That is fine as the address space also holds a reference to the page i.e. the reference count can not drop to zero before the map count.

Quoted 3 lines

What tree were you hoping to get this merged through? I'd suggest net
- it's more likely to get tested over there.

That was one part I wasn't quite sure about - the v3 patchset makes things less clear even, since while v1/v2 are mostly mm heavy v3 would have some significant changes in both subsystems.

I'm open to whichever is the "right" way to go, but am not currently certain which would be.

Thanks,
-Arjun

Quoted 9 lines

mm/memcontrol.c
Quoted 5 lines
...
mm/memcontrol.c +0 -0
These changes could be inside #ifdef CONFIG_NET. Although I expect MEMCG=y&&NET=n is pretty damn rare.