Linux cgroups development
 help / color / mirror / Atom feed
* [PATCH] mm/hugetlb_cgroup: call page_counter_set_max() outside VM_BUG_ON()
@ 2026-08-17 10:34 Narek Jilavyan
  2026-08-17 11:15 ` Muchun Song
  0 siblings, 1 reply; 2+ messages in thread
From: Narek Jilavyan @ 2026-08-17 10:34 UTC (permalink / raw)
  To: Muchun Song, Oscar Salvador, Andrew Morton
  Cc: David Hildenbrand, Shakeel Butt, linux-mm, cgroups, linux-kernel,
	Narek Jilavyan

hugetlb_cgroup_css_alloc() rounds the counter limit down to a multiple of
the huge page size and then applies it inside an assertion:

	VM_BUG_ON(page_counter_set_max(fault, limit));
	VM_BUG_ON(page_counter_set_max(rsvd, limit));

With CONFIG_DEBUG_VM=n, VM_BUG_ON(cond) is BUILD_BUG_ON_INVALID(cond),
i.e. ((void)(sizeof((__force long)(cond)))), whose operand is never
evaluated.  page_counter_set_max() is not a predicate - it performs
xchg(&counter->max, nr_pages) - so on every non-debug kernel the limit is
never applied and the counters keep page_counter_init()'s
PAGE_COUNTER_MAX.

That is user-visible, because hugetlb_cgroup_read_u64_max() recomputes
the same rounded value and uses equality as its "unlimited" sentinel.
PAGE_COUNTER_MAX is LONG_MAX / PAGE_SIZE = 2251799813685247, which is
odd, so round_down() really does change it and the two sides disagree.
With CONFIG_DEBUG_VM=n:

	$ cat /sys/fs/cgroup/t/hugetlb.2MB.max
	9223372036854771712

and with this patch:

	$ cat /sys/fs/cgroup/t/hugetlb.2MB.max
	max

A debug option should not change cgroup output.

Call the function, then assert the result, as v6.12 did.  Use
VM_WARN_ON_ONCE() rather than restoring VM_BUG_ON(): the two are
identical under CONFIG_DEBUG_VM=n, and checkpatch asks that new code not
use BUG() variants.

Fixes: 0e2759afcaf9 ("page_counter: track failcnt only for legacy cgroups")
Signed-off-by: Narek Jilavyan <njilav@gmail.com>
---
 mm/hugetlb_cgroup.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/mm/hugetlb_cgroup.c b/mm/hugetlb_cgroup.c
index e0083de1ca..ecb6e0b781 100644
--- a/mm/hugetlb_cgroup.c
+++ b/mm/hugetlb_cgroup.c
@@ -97,6 +97,7 @@ static void hugetlb_cgroup_init(struct hugetlb_cgroup *h_cgroup,
 		struct page_counter *fault, *fault_parent = NULL;
 		struct page_counter *rsvd, *rsvd_parent = NULL;
 		unsigned long limit;
+		int ret;
 
 		if (parent_h_cgroup) {
 			fault_parent = hugetlb_cgroup_counter_from_cgroup(
@@ -118,8 +119,10 @@ static void hugetlb_cgroup_init(struct hugetlb_cgroup *h_cgroup,
 		limit = round_down(PAGE_COUNTER_MAX,
 				   pages_per_huge_page(&hstates[idx]));
 
-		VM_BUG_ON(page_counter_set_max(fault, limit));
-		VM_BUG_ON(page_counter_set_max(rsvd, limit));
+		ret = page_counter_set_max(fault, limit);
+		VM_WARN_ON_ONCE(ret);
+		ret = page_counter_set_max(rsvd, limit);
+		VM_WARN_ON_ONCE(ret);
 	}
 }
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] mm/hugetlb_cgroup: call page_counter_set_max() outside VM_BUG_ON()
  2026-08-17 10:34 [PATCH] mm/hugetlb_cgroup: call page_counter_set_max() outside VM_BUG_ON() Narek Jilavyan
@ 2026-08-17 11:15 ` Muchun Song
  0 siblings, 0 replies; 2+ messages in thread
From: Muchun Song @ 2026-08-17 11:15 UTC (permalink / raw)
  To: Narek Jilavyan
  Cc: Oscar Salvador, Andrew Morton, David Hildenbrand, Shakeel Butt,
	linux-mm, cgroups, linux-kernel



> On Aug 17, 2026, at 18:34, Narek Jilavyan <njilav@gmail.com> wrote:
> 
> hugetlb_cgroup_css_alloc() rounds the counter limit down to a multiple of
> the huge page size and then applies it inside an assertion:
> 
> VM_BUG_ON(page_counter_set_max(fault, limit));
> VM_BUG_ON(page_counter_set_max(rsvd, limit));
> 
> With CONFIG_DEBUG_VM=n, VM_BUG_ON(cond) is BUILD_BUG_ON_INVALID(cond),
> i.e. ((void)(sizeof((__force long)(cond)))), whose operand is never
> evaluated.  page_counter_set_max() is not a predicate - it performs
> xchg(&counter->max, nr_pages) - so on every non-debug kernel the limit is
> never applied and the counters keep page_counter_init()'s
> PAGE_COUNTER_MAX.
> 
> That is user-visible, because hugetlb_cgroup_read_u64_max() recomputes
> the same rounded value and uses equality as its "unlimited" sentinel.
> PAGE_COUNTER_MAX is LONG_MAX / PAGE_SIZE = 2251799813685247, which is
> odd, so round_down() really does change it and the two sides disagree.
> With CONFIG_DEBUG_VM=n:
> 
> $ cat /sys/fs/cgroup/t/hugetlb.2MB.max
> 9223372036854771712
> 
> and with this patch:
> 
> $ cat /sys/fs/cgroup/t/hugetlb.2MB.max
> max
> 
> A debug option should not change cgroup output.
> 
> Call the function, then assert the result, as v6.12 did.  Use
> VM_WARN_ON_ONCE() rather than restoring VM_BUG_ON(): the two are
> identical under CONFIG_DEBUG_VM=n, and checkpatch asks that new code not
> use BUG() variants.
> 
> Fixes: 0e2759afcaf9 ("page_counter: track failcnt only for legacy cgroups")
> Signed-off-by: Narek Jilavyan <njilav@gmail.com>

Reviewed-by: Muchun Song <muchun.song@linux.dev>

Thanks.


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-08-17 11:16 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-17 10:34 [PATCH] mm/hugetlb_cgroup: call page_counter_set_max() outside VM_BUG_ON() Narek Jilavyan
2026-08-17 11:15 ` Muchun Song

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox