Messages in this thread Patch in this message | | | Date | Wed, 13 Mar 2024 15:04:17 +0000 | From | Matthew Wilcox <> | Subject | Re: [PATCH v5 20/37] mm: fix non-compound multi-order memory accounting in __free_pages |
| |
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); }
> +++ 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.
| |