introduce uncharged file mapped folios

+++ b/mm/filemap.c

mm/filemap.c +4 -3

@@ -960,15 +960,19 @@ int filemap_add_folio(struct address_space *mapping, struct folio *folio,
 {
 	void *shadow = NULL;
 	int ret;
+	bool charge_mem_cgroup = !test_bit(AS_UNCHARGED, &mapping->flags);
 
-	ret = mem_cgroup_charge(folio, NULL, gfp);
-	if (ret)
-		return ret;
+	if (charge_mem_cgroup) {
+		ret = mem_cgroup_charge(folio, NULL, gfp);
+		if (ret)

Quoted 16 lines · Boris Burkov

+++ b/mm/filemap.c

mm/filemap.c +6 -3

@@ -960,15 +960,19 @@ int filemap_add_folio(struct address_space *mapping, struct folio *folio,
 {
 	void *shadow = NULL;
 	int ret;
+	bool charge_mem_cgroup = !test_bit(AS_UNCHARGED, &mapping->flags);
 
-	ret = mem_cgroup_charge(folio, NULL, gfp);
-	if (ret)
-		return ret;
+	if (charge_mem_cgroup) {
+		ret = mem_cgroup_charge(folio, NULL, gfp);
+		if (ret)
+			return ret;
+	}
 

mm/filemap.c int filemap_add_folio(struct address_space *mapping, struct folio *folio, · line 976

Quoted 7 lines

mm/filemap.c +2 -1

 	__folio_set_locked(folio);
 	ret = __filemap_add_folio(mapping, folio, index, gfp, &shadow);
 	if (unlikely(ret)) {
-		mem_cgroup_uncharge(folio);
+		if (charge_mem_cgroup)
+			mem_cgroup_uncharge(folio);
 		__folio_clear_locked(folio);

This is unnecessarily complex; mem_cgroup_uncharge() is a no-op if the folio is not charged. Sure, it's a wasted function call, but this is a rare error path, so minimising size is more important than minimising time.

Btrfs currently tracks its metadata pages in the page cache, using a fake inode (fs_info->btree_inode) with offsets corresponding to where the metadata is stored in the filesystem's full logical address space.

A consequence of this is that when btrfs uses filemap_add_folio(), this usage is charged to the cgroup of whichever task happens to be running at the time. These folios don't belong to any particular user cgroup, so I don't think it makes much sense for them to be charged in that way. Some negative consequences as a result:
- A task can be holding some important btrfs locks, then need to lookup some metadata and go into reclaim, extending the duration it holds that lock for, and unfairly pushing its own reclaim pain onto other cgroups.
- If that cgroup goes into reclaim, it might reclaim these folios a different non-reclaiming cgroup might need soon. This is naturally offset by LRU reclaim, but still.

A very similar proposal to use the root cgroup was previously made by Qu, where he eventually proposed the idea of setting it per address_space. This makes good sense for the btrfs use case, as the uncharged behavior should apply to all use of the address_space, not select allocations. I.e., if someone adds another filemap_add_folio() call using btrfs's btree_inode, we would almost certainly want the uncharged behavior.

Link https://lore.kernel.org/linux-mm/b5fef5372ae454a7b6da4f2f75c427aeab6a07d6.1727498749.git.wqu@suse.com/ open here

Suggested-by Qu Wenruo <[email protected]>

Acked-by Shakeel Butt <[email protected]>

Tested-by [email protected]

Signed-off-by Boris Burkov <[email protected]>

---
 include/linux/pagemap.h |  1 +
 mm/filemap.c            | 12 ++++++++----
 2 files changed, 9 insertions(+), 4 deletions(-)

include/linux/pagemap.h +1 -0

index c9ba69e02e3e..06dc3fae8124 100644
@@ -211,6 +211,7 @@ enum mapping_flags {
 				   folio contents */
 	AS_INACCESSIBLE = 8,	/* Do not attempt direct R/W access to the mapping */
 	AS_WRITEBACK_MAY_DEADLOCK_ON_RECLAIM = 9,
+	AS_UNCHARGED = 10,	/* Do not charge usage to a cgroup */
 	/* Bits 16-25 are used for FOLIO_ORDER */
 	AS_FOLIO_ORDER_BITS = 5,
 	AS_FOLIO_ORDER_MIN = 16,

mm/filemap.c +8 -4

index e4a5a46db89b..5004a2cfa0cc 100644
@@ -960,15 +960,19 @@ int filemap_add_folio(struct address_space *mapping, struct folio *folio,
 {
 	void *shadow = NULL;
 	int ret;
+	bool charge_mem_cgroup = !test_bit(AS_UNCHARGED, &mapping->flags);
 
-	ret = mem_cgroup_charge(folio, NULL, gfp);
-	if (ret)
-		return ret;
+	if (charge_mem_cgroup) {
+		ret = mem_cgroup_charge(folio, NULL, gfp);
+		if (ret)
+			return ret;
+	}
 
 	__folio_set_locked(folio);
 	ret = __filemap_add_folio(mapping, folio, index, gfp, &shadow);
 	if (unlikely(ret)) {
-		mem_cgroup_uncharge(folio);
+		if (charge_mem_cgroup)
+			mem_cgroup_uncharge(folio);
 		__folio_clear_locked(folio);
 	} else {
 		/*

Signature · 1 line

2.50.1

The following diff resolves the issue for me:

include/linux/memcontrol.h +1 -1

index fae105a9cb46..c70e789201fc 100644
@@ -809,7 +809,7 @@ void mem_cgroup_scan_tasks(struct mem_cgroup *memcg,
 
 static inline unsigned short mem_cgroup_id(struct mem_cgroup *memcg)
 {
-	if (mem_cgroup_disabled())
+	if (mem_cgroup_disabled() || !memcg)
 		return 0;
 
 	return memcg->id.id;

However, it's mentioned in folio_memcg() that it can return NULL so this might be an existing bug which this patch just makes more obvious.

There's also workingset_eviction() which instead gets the memcg from lruvec. Doing that in lru_gen_eviction() also resolves the issue for me:

mm/workingset.c +4 -1

index 68a76a91111f..e805eadf0ec7 100644
@@ -243,6 +243,7 @@ static void *lru_gen_eviction(struct folio *folio)
 	int tier = lru_tier_from_refs(refs, workingset);
 	struct mem_cgroup *memcg = folio_memcg(folio);
 	struct pglist_data *pgdat = folio_pgdat(folio);
+	int memcgid;
 
 	BUILD_BUG_ON(LRU_GEN_WIDTH + LRU_REFS_WIDTH > BITS_PER_LONG - EVICTION_SHIFT);
 

@@ -254,7 +255,9 @@ static void *lru_gen_eviction(struct folio *folio)
 	hist = lru_hist_from_seq(min_seq);
 	atomic_long_add(delta, &lrugen->evicted[hist][type][tier]);
 
-	return pack_shadow(mem_cgroup_id(memcg), pgdat, token, workingset);
+	memcgid = mem_cgroup_id(lruvec_memcg(lruvec));
+
+	return pack_shadow(memcgid, pgdat, token, workingset);
 }
 
 /*

I don't really know what I'm doing here, though.

Hi,

Quoted 25 lines · Boris Burkov

Btrfs currently tracks its metadata pages in the page cache, using a fake inode (fs_info->btree_inode) with offsets corresponding to where the metadata is stored in the filesystem's full logical address space.

A consequence of this is that when btrfs uses filemap_add_folio(), this usage is charged to the cgroup of whichever task happens to be running at the time. These folios don't belong to any particular user cgroup, so I don't think it makes much sense for them to be charged in that way. Some negative consequences as a result:
- A task can be holding some important btrfs locks, then need to lookup some metadata and go into reclaim, extending the duration it holds that lock for, and unfairly pushing its own reclaim pain onto other cgroups.
- If that cgroup goes into reclaim, it might reclaim these folios a different non-reclaiming cgroup might need soon. This is naturally offset by LRU reclaim, but still.

A very similar proposal to use the root cgroup was previously made by Qu, where he eventually proposed the idea of setting it per address_space. This makes good sense for the btrfs use case, as the uncharged behavior should apply to all use of the address_space, not select allocations. I.e., if someone adds another filemap_add_folio() call using btrfs's btree_inode, we would almost certainly want the uncharged behavior.

Quoted 5 lines

Link https://lore.kernel.org/linux-mm/b5fef5372ae454a7b6da4f2f75c427aeab6a07d6.1727498749.git.wqu@suse.com/ open here

Suggested-by Qu Wenruo <[email protected]>

Acked-by Shakeel Butt <[email protected]>

Tested-by [email protected]

Signed-off-by Boris Burkov <[email protected]>

I bisected the following null-dereference to 3f31e0d9912d ("btrfs: set AS_UNCHARGED on the btree_inode") in mm-new but I believe it's a result of this patch:

Oops [#1]

 CPU: 4 UID: 0 PID: 87 Comm: kswapd0 Not tainted 6.17.0-rc2-next-20250820-00349-gd6ecef4f9566 #511 PREEMPTLAZY
 Hardware name: Banana Pi BPI-F3 (DT)
 epc : workingset_eviction (include/linux/memcontrol.h:815 mm/workingset.c:257 mm/workingset.c:394) 
 ra : __remove_mapping (mm/vmscan.c:805) 
 epc : ffffffff802e6de8 ra : ffffffff802b4114 sp : ffffffc6006c3670
  gp : ffffffff8227dad8 tp : ffffffd701a2cb00 t0 : ffffffff80027d00
  t1 : 0000000000000000 t2 : 0000000000000001 s0 : ffffffc6006c3680
  s1 : ffffffc50415a540 a0 : 0000000000000001 a1 : ffffffd700b70048
  a2 : 0000000000000000 a3 : 0000000000000000 a4 : 00000000000003f0
  a5 : ffffffd700b70430 a6 : 0000000000000000 a7 : ffffffd77ffd1dc0
  s2 : ffffffd705a483d8 s3 : ffffffd705a483e0 s4 : 0000000000000001
  s5 : 0000000000000000 s6 : 0000000000000000 s7 : 0000000000000001
  s8 : ffffffd705a483d8 s9 : ffffffc6006c3760 s10: ffffffc50415a548
  s11: ffffffff81e000e0 t3 : 0000000000000000 t4 : 0000000000000001
  t5 : 0000000000000003 t6 : 0000000000000003
 status: 0000000200000100 badaddr: 00000000000000d0 cause: 000000000000000d
 workingset_eviction (include/linux/memcontrol.h:815 mm/workingset.c:257 mm/workingset.c:394) 
 __remove_mapping (mm/vmscan.c:805) 
 shrink_folio_list (mm/vmscan.c:1545 (discriminator 2)) 
 evict_folios (mm/vmscan.c:4738) 
 try_to_shrink_lruvec (mm/vmscan.c:4901) 
 shrink_one (mm/vmscan.c:4947) 
 shrink_node (include/asm-generic/preempt.h:54 (discriminator 1) include/linux/rcupdate.h:93 (discriminator 1) include/linux/rcupdate.h:839 (discriminator 1) mm/vmscan.c:5010 (discriminator 1) mm/vmscan.c:5086 (discriminator 1) mm/vmscan.c:6073 (discriminator 1)) 
 balance_pgdat (mm/vmscan.c:6942 mm/vmscan.c:7116) 
 kswapd (mm/vmscan.c:7381) 
 kthread (kernel/kthread.c:463) 
 ret_from_fork_kernel (include/linux/entry-common.h:155 (discriminator 4) include/linux/entry-common.h:210 (discriminator 4) arch/riscv/kernel/process.c:216 (discriminator 4)) 
 ret_from_fork_kernel_asm (arch/riscv/kernel/entry.S:328) 
 Code: 0987 060a 6633 01c6 97ba b02f 01d7 0001 0013 0000 (5503) 0d08
 All code
 ========
    0:	060a0987          	.insn	4, 0x060a0987
    4:	01c66633          	or	a2,a2,t3
    8:	97ba                	.insn	2, 0x97ba
    a:	01d7b02f          	amoadd.d	zero,t4,(a5)
    e:	0001                	.insn	2, 0x0001
   10:	00000013          	addi	zero,zero,0
   14:*	0d085503          	lhu	a0,208(a6)		<-- trapping instruction

Code starting with the faulting instruction
===========================================

    0:	0d085503          	lhu	a0,208(a6)
 ---[ end trace 0000000000000000 ]---
 note: kswapd0[87] exited with irqs disabled
 note: kswapd0[87] exited with preempt_count 2

Quoted 48 lines

---
 include/linux/pagemap.h |  1 +
 mm/filemap.c            | 12 ++++++++----
 2 files changed, 9 insertions(+), 4 deletions(-)

include/linux/pagemap.h +1 -0

index c9ba69e02e3e..06dc3fae8124 100644
@@ -211,6 +211,7 @@ enum mapping_flags {
 				   folio contents */
 	AS_INACCESSIBLE = 8,	/* Do not attempt direct R/W access to the mapping */
 	AS_WRITEBACK_MAY_DEADLOCK_ON_RECLAIM = 9,
+	AS_UNCHARGED = 10,	/* Do not charge usage to a cgroup */
 	/* Bits 16-25 are used for FOLIO_ORDER */
 	AS_FOLIO_ORDER_BITS = 5,
 	AS_FOLIO_ORDER_MIN = 16,

mm/filemap.c +8 -4

index e4a5a46db89b..5004a2cfa0cc 100644
@@ -960,15 +960,19 @@ int filemap_add_folio(struct address_space *mapping, struct folio *folio,
 {
 	void *shadow = NULL;
 	int ret;
+	bool charge_mem_cgroup = !test_bit(AS_UNCHARGED, &mapping->flags);
 
-	ret = mem_cgroup_charge(folio, NULL, gfp);
-	if (ret)
-		return ret;
+	if (charge_mem_cgroup) {
+		ret = mem_cgroup_charge(folio, NULL, gfp);
+		if (ret)
+			return ret;
+	}
 
 	__folio_set_locked(folio);
 	ret = __filemap_add_folio(mapping, folio, index, gfp, &shadow);
 	if (unlikely(ret)) {
-		mem_cgroup_uncharge(folio);
+		if (charge_mem_cgroup)
+			mem_cgroup_uncharge(folio);
 		__folio_clear_locked(folio);
 	} else {
 		/*

Signature · 1 line

2.50.1

This means that not all folios will have a memcg attached also when memcg is enabled. In lru_gen_eviction() mem_cgroup_id() is called without a NULL check which then leads to the null-dereference.

The following diff resolves the issue for me:

include/linux/memcontrol.h +1 -1

index fae105a9cb46..c70e789201fc 100644
@@ -809,7 +809,7 @@ void mem_cgroup_scan_tasks(struct mem_cgroup *memcg,
 
 static inline unsigned short mem_cgroup_id(struct mem_cgroup *memcg)
 {
-	if (mem_cgroup_disabled())
+	if (mem_cgroup_disabled() || !memcg)
 		return 0;
 
 	return memcg->id.id;

However, it's mentioned in folio_memcg() that it can return NULL so this might be an existing bug which this patch just makes more obvious.

There's also workingset_eviction() which instead gets the memcg from lruvec. Doing that in lru_gen_eviction() also resolves the issue for me:

mm/workingset.c +4 -1

index 68a76a91111f..e805eadf0ec7 100644
@@ -243,6 +243,7 @@ static void *lru_gen_eviction(struct folio *folio)
 	int tier = lru_tier_from_refs(refs, workingset);
 	struct mem_cgroup *memcg = folio_memcg(folio);
 	struct pglist_data *pgdat = folio_pgdat(folio);
+	int memcgid;
 
 	BUILD_BUG_ON(LRU_GEN_WIDTH + LRU_REFS_WIDTH > BITS_PER_LONG - EVICTION_SHIFT);
 

@@ -254,7 +255,9 @@ static void *lru_gen_eviction(struct folio *folio)
 	hist = lru_hist_from_seq(min_seq);
 	atomic_long_add(delta, &lrugen->evicted[hist][type][tier]);
 
-	return pack_shadow(mem_cgroup_id(memcg), pgdat, token, workingset);
+	memcgid = mem_cgroup_id(lruvec_memcg(lruvec));
+
+	return pack_shadow(memcgid, pgdat, token, workingset);
 }
 
 /*

I don't really know what I'm doing here, though.

Regards,
Klara Modin

Quoted 38 lines · Klara Modin

Hi,

Quoted 25 lines · Boris Burkov

Btrfs currently tracks its metadata pages in the page cache, using a fake inode (fs_info->btree_inode) with offsets corresponding to where the metadata is stored in the filesystem's full logical address space.

A consequence of this is that when btrfs uses filemap_add_folio(), this usage is charged to the cgroup of whichever task happens to be running at the time. These folios don't belong to any particular user cgroup, so I don't think it makes much sense for them to be charged in that way. Some negative consequences as a result:
- A task can be holding some important btrfs locks, then need to lookup some metadata and go into reclaim, extending the duration it holds that lock for, and unfairly pushing its own reclaim pain onto other cgroups.
- If that cgroup goes into reclaim, it might reclaim these folios a different non-reclaiming cgroup might need soon. This is naturally offset by LRU reclaim, but still.

A very similar proposal to use the root cgroup was previously made by Qu, where he eventually proposed the idea of setting it per address_space. This makes good sense for the btrfs use case, as the uncharged behavior should apply to all use of the address_space, not select allocations. I.e., if someone adds another filemap_add_folio() call using btrfs's btree_inode, we would almost certainly want the uncharged behavior.

Quoted 5 lines

Link https://lore.kernel.org/linux-mm/b5fef5372ae454a7b6da4f2f75c427aeab6a07d6.1727498749.git.wqu@suse.com/ open here

Suggested-by Qu Wenruo <[email protected]>

Acked-by Shakeel Butt <[email protected]>

Tested-by [email protected]

Signed-off-by Boris Burkov <[email protected]>

I bisected the following null-dereference to 3f31e0d9912d ("btrfs: set AS_UNCHARGED on the btree_inode") in mm-new but I believe it's a result of this patch:

Quoted 51 lines

This means that not all folios will have a memcg attached also when memcg is enabled. In lru_gen_eviction() mem_cgroup_id() is called without a NULL check which then leads to the null-dereference.

The following diff resolves the issue for me:

include/linux/memcontrol.h +1 -1

index fae105a9cb46..c70e789201fc 100644
@@ -809,7 +809,7 @@ void mem_cgroup_scan_tasks(struct mem_cgroup *memcg,
 
 static inline unsigned short mem_cgroup_id(struct mem_cgroup *memcg)
 {
-	if (mem_cgroup_disabled())
+	if (mem_cgroup_disabled() || !memcg)
 		return 0;
 
 	return memcg->id.id;

However, it's mentioned in folio_memcg() that it can return NULL so this might be an existing bug which this patch just makes more obvious.

There's also workingset_eviction() which instead gets the memcg from lruvec. Doing that in lru_gen_eviction() also resolves the issue for me:

mm/workingset.c +4 -1

index 68a76a91111f..e805eadf0ec7 100644
@@ -243,6 +243,7 @@ static void *lru_gen_eviction(struct folio *folio)
 	int tier = lru_tier_from_refs(refs, workingset);
 	struct mem_cgroup *memcg = folio_memcg(folio);
 	struct pglist_data *pgdat = folio_pgdat(folio);
+	int memcgid;
 
 	BUILD_BUG_ON(LRU_GEN_WIDTH + LRU_REFS_WIDTH > BITS_PER_LONG - EVICTION_SHIFT);
 

@@ -254,7 +255,9 @@ static void *lru_gen_eviction(struct folio *folio)
 	hist = lru_hist_from_seq(min_seq);
 	atomic_long_add(delta, &lrugen->evicted[hist][type][tier]);
 
-	return pack_shadow(mem_cgroup_id(memcg), pgdat, token, workingset);
+	memcgid = mem_cgroup_id(lruvec_memcg(lruvec));
+
+	return pack_shadow(memcgid, pgdat, token, workingset);
 }
 
 /*

Quoted 1 line

I don't really know what I'm doing here, though.

Me neither, clearly :)

Thanks so much for the report and fix! I fear there might be some other paths that try to get memcg from lruvec or folio or whatever without checking it. I feel like in this exact case, I would want to go to the first sign of trouble and fix it at lruvec_memcg(). But then who knows what else we've missed.

May I ask what you were running to trigger this? My fstests run (clearly not exercising enough interesting memory paths) did not hit it.

This does make me wonder if the superior approach to the original patch isn't just to go back to the very first thing Qu did and account these to the root cgroup rather than do the whole uncharged thing.

Boris

Quoted 3 lines

Regards,
Klara Modin

Thanks so much for the report and fix! I fear there might be some other paths that try to get memcg from lruvec or folio or whatever without checking it. I feel like in this exact case, I would want to go to the first sign of trouble and fix it at lruvec_memcg(). But then who knows what else we've missed.

May I ask what you were running to trigger this? My fstests run (clearly not exercising enough interesting memory paths) did not hit it.

This does make me wonder if the superior approach to the original patch isn't just to go back to the very first thing Qu did and account these to the root cgroup rather than do the whole uncharged thing.

On Wed, Aug 20, 2025 at 03:52:22PM -0700, Boris Burkov wrote:
[...]

Quoted 6 lines

Thanks so much for the report and fix! I fear there might be some other paths that try to get memcg from lruvec or folio or whatever without checking it. I feel like in this exact case, I would want to go to the first sign of trouble and fix it at lruvec_memcg(). But then who knows what else we've missed.

lruvec_memcg() is not an issue but folio_memcg() can be. I found following instances of folio_memcg() which are problematic (on next-20250819):

mm/memcontrol.c:3246: css_get_many(&__folio_memcg(folio)->css, new_refs);

include/trace/events/writeback.h:269: __entry->page_cgroup_ino = cgroup_ino(folio_memcg(folio)->css.cgroup);

mm/workingset.c:244: struct mem_cgroup *memcg = folio_memcg(folio);

mm/huge_memory.c:4020: WARN_ON_ONCE(!mem_cgroup_disabled() && !folio_memcg(folio));

Quoted 1 line

Quoted 6 lines

May I ask what you were running to trigger this? My fstests run (clearly not exercising enough interesting memory paths) did not hit it.

This does make me wonder if the superior approach to the original patch isn't just to go back to the very first thing Qu did and account these to the root cgroup rather than do the whole uncharged thing.

I don't have any strong preference one way or the other.

Uncharged pages are tricky to track by their essential "uncharged" nature. To maintain good accounting, introduce a vmstat counter tracking all uncharged pages. Since this is only meaningful when cgroups are configured, only expose the counter when CONFIG_MEMCG is set.

Quoted 4 lines · Boris Burkov

Uncharged pages are tricky to track by their essential "uncharged" nature. To maintain good accounting, introduce a vmstat counter tracking all uncharged pages. Since this is only meaningful when cgroups are configured, only expose the counter when CONFIG_MEMCG is set.

I don't understand why this is needed. Maybe Shakeel had better reasoning that wasn't captured in the commit message.

If they're unaccounted, then you can get a good estimate of them just by subtracting the number of accounted pages from the number of file pages. Sure there's a small race between the two numbers being updated, so you migth be off by a bit.

Quoted 4 lines · Boris Burkov

Uncharged pages are tricky to track by their essential "uncharged" nature. To maintain good accounting, introduce a vmstat counter tracking all uncharged pages. Since this is only meaningful when cgroups are configured, only expose the counter when CONFIG_MEMCG is set.

I don't understand why this is needed. Maybe Shakeel had better reasoning that wasn't captured in the commit message.

If they're unaccounted, then you can get a good estimate of them just by subtracting the number of accounted pages from the number of file pages. Sure there's a small race between the two numbers being updated, so you migth be off by a bit.

My initial thinking was based on Qu's original proposal which was using root memcg where there will not be any difference between accounted file pages and system wide file pages. However with Boris's change, we can actually get the estimate, as you pointed out, by subtracting the number of accounted file pages from system wide number of file pages.

However I still think we should keep this new metric because of performance reason. To get accounted file pages, we need to read memory.stat of the root memcg which can be very expensive. Basically it may have to flush the rstat update trees on all the CPUs on the system. Since this new metric will be used to calculate system overhead, the high cost will limit how frequently a user can query the latest stat.

Quoted 6 lines · Shakeel Butt

My initial thinking was based on Qu's original proposal which was using root memcg where there will not be any difference between accounted file pages and system wide file pages. However with Boris's change, we can actually get the estimate, as you pointed out, by subtracting the number of accounted file pages from system wide number of file pages.

Quoted 6 lines

However I still think we should keep this new metric because of performance reason. To get accounted file pages, we need to read memory.stat of the root memcg which can be very expensive. Basically it may have to flush the rstat update trees on all the CPUs on the system. Since this new metric will be used to calculate system overhead, the high cost will limit how frequently a user can query the latest stat.

OK, but couldn't we make that argument for anything else? Like slab, say. Why's "file" memory different?

Quoted 2 lines · Matthew Wilcox

OK, but couldn't we make that argument for anything else? Like slab, say. Why's "file" memory different?

Good point and I think it does apply to other memory types too. I would call "file" memory to be more important as it is one of the largest consumer of DRAM on, at least, Meta infra. Slab needs a bit more thought. At the system level (i.e. /proc/meminfo), we account at the page (or slab) level while for memcg, we account per-object (plus obj_cgroup pointer).

Quoted 10 lines · Shakeel Butt

Quoted 2 lines · Matthew Wilcox

OK, but couldn't we make that argument for anything else? Like slab, say. Why's "file" memory different?

Good point and I think it does apply to other memory types too. I would call "file" memory to be more important as it is one of the largest consumer of DRAM on, at least, Meta infra. Slab needs a bit more thought. At the system level (i.e. /proc/meminfo), we account at the page (or slab) level while for memcg, we account per-object (plus obj_cgroup pointer).

That was supposed to be a reductio ad absurdum, not an invitation to add more counters.

Look, if this is information you really need, I think you should come up with a better way of collecting it than by adding new counters and new complexity to everything involved in GFP_ACCOUNT activities.

The unaccounted address_spaces are a very tiny percentage of file memory, at least as far as this patch set goes.

I don't think this
patch is justifiable on its face.

Commit a4055888629bc ("mm/memcg: warning on !memcg after readahead page charged") added the warning in folio_lruvec (older name was mem_cgroup_page_lruvec) for !memcg when charging of readahead pages were added to the kernel. Basically lru pages on a memcg enabled system were always expected to be charged to a memcg.

However a recent functionality to allow metadata of btrfs, which is in page cache, to be uncharged is added to the kernel. We can either change the condition to only check anon pages or file pages which does not have AS_UNCHARGED in their mapping. Instead of such complicated check, let's just remove the warning as it is not really helpful anymore.

Quoted 6 lines · Boris Burkov

Commit a4055888629bc ("mm/memcg: warning on !memcg after readahead page charged") added the warning in folio_lruvec (older name was mem_cgroup_page_lruvec) for !memcg when charging of readahead pages were added to the kernel. Basically lru pages on a memcg enabled system were always expected to be charged to a memcg.

Quoted 5 lines

However a recent functionality to allow metadata of btrfs, which is in page cache, to be uncharged is added to the kernel. We can either change the condition to only check anon pages or file pages which does not have AS_UNCHARGED in their mapping. Instead of such complicated check, let's just remove the warning as it is not really helpful anymore.

This has to go before patch 3 (and I'd put it before patch 1) in order to preserve bisectability..

commit message, but that's perfectly acceptable.