Re: [PATCH v5 20/37] mm: fix non-compound multi-order memory accounting in __free_pages
From: Matthew Wilcox <willy@infradead.org>
Date: 2024-03-13 15:04:51
Also in:
cgroups, linux-arch, linux-doc, linux-fsdevel, linux-iommu, linux-mm, lkml
On Wed, Mar 06, 2024 at 10:24:18AM -0800, Suren Baghdasaryan wrote:
When a non-compound multi-order page is freed, it is possible that a speculative reference keeps the page pinned. In this case we free all pages except for the first page, which will be freed later by the last put_page(). However put_page() ignores the order of the page being freed, treating it as a 0-order page. This creates a memory accounting imbalance because the pages freed in __free_pages() do not have their own alloc_tag and their memory was accounted to the first page. To fix this the first page should adjust its allocation size counter when "tail" pages are freed.
It's not "ignored". It's not available! Better wording: However the page passed to put_page() is indisinguishable from an order-0 page, so it cannot do the accounting, just as it cannot free the subsequent pages. Do the accounting here, where we free the pages. (I'm sure further improvements are possible)
+static inline void pgalloc_tag_sub_bytes(struct alloc_tag *tag, unsigned int order)
+{
+ if (mem_alloc_profiling_enabled() && tag)
+ this_cpu_sub(tag->counters->bytes, PAGE_SIZE << order);
+}
This is a terribly named function. And it's not even good for what we
want to use it for.
static inline void pgalloc_tag_sub_pages(struct alloc_tag *tag, unsigned int nr)
{
if (mem_alloc_profiling_enabled() && tag)
this_cpu_sub(tag->counters->bytes, PAGE_SIZE * nr);
}
quoted hunk ↗ jump to hunk
+++ b/mm/page_alloc.c@@ -4697,12 +4697,21 @@ void __free_pages(struct page *page, unsigned int order) { /* get PageHead before we drop reference */ int head = PageHead(page); + struct alloc_tag *tag = pgalloc_tag_get(page); if (put_page_testzero(page)) free_the_page(page, order); else if (!head) - while (order-- > 0) + while (order-- > 0) { free_the_page(page + (1 << order), order); + /* + * non-compound multi-order page accounts all allocations + * to the first page (just like compound one), therefore + * we need to adjust the allocation size of the first + * page as its order is ignored when put_page() frees it. + */ + pgalloc_tag_sub_bytes(tag, order);
- else if (!head
+ else if (!head) {
+ pgalloc_tag_sub_pages(1 << order - 1);
while (order-- > 0)
free_the_page(page + (1 << order), order);
+ }
It doesn't need a comment, it's obvious what you're doing.