| Message ID | 20250212020627.3461237-1-shengken.lin@amlogic.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm: Remove redundant PageMemcgKmem(page) call | expand |
On Wed, Feb 12, 2025 at 10:06:27AM +0800, Shengken Lin wrote: > The function of PageMemcgKmem(page) is the same as that of > folio_memcg_kmem(folio). > > Since __memcg_kmem_uncharge_page already uses folio_memcg_kmem to check > if a folio has been uncharged, calling PageMemcgKmem(page) before > __memcg_kmem_uncharge_page is redundant. But it saves a function call. NAK.
On Wed, 12 Feb 2025 04:28PM +0000, Matthew Wilcox wrote: >On Wed, Feb 12, 2025 at 10:06:27AM +0800, Shengken Lin wrote: >> The function of PageMemcgKmem(page) is the same as that of >> folio_memcg_kmem(folio). >> >> Since __memcg_kmem_uncharge_page already uses folio_memcg_kmem to check >> if a folio has been uncharged, calling PageMemcgKmem(page) before >> __memcg_kmem_uncharge_page is redundant. > >But it saves a function call. NAK. Hi Matthew Wilcox, If it can save a function call, then adding PageMemcgKmem(page) in include/linux/memcontrol.h would make the code more consistent. If this is considered more reasonable, I can submit a new patch. --- a/include/linux/memcontrol.h +++ b/include/linux/memcontrol.h @@ -1707,7 +1707,7 @@ static inline int memcg_kmem_charge_page(struct page *page, gfp_t gfp, static inline void memcg_kmem_uncharge_page(struct page *page, int order) { - if (memcg_kmem_online()) + if (memcg_kmem_online() && PageMemcgKmem(page)) __memcg_kmem_uncharge_page(page, order); } -- Best regards, Shengken Lin
On Thu, Feb 20, 2025 at 05:47:56PM +0800, Shengken.Lin@amlogic.com wrote: > If it can save a function call, then adding PageMemcgKmem(page) > in include/linux/memcontrol.h would make the code more consistent. The code is fine the way it is.
diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 579789600a3c..5a76760de495 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -1055,7 +1055,7 @@ __always_inline bool free_pages_prepare(struct page *page, trace_mm_page_free(page, order); kmsan_free_page(page, order); - if (memcg_kmem_online() && PageMemcgKmem(page)) + if (memcg_kmem_online()) __memcg_kmem_uncharge_page(page, order); /*
The function of PageMemcgKmem(page) is the same as that of folio_memcg_kmem(folio). Since __memcg_kmem_uncharge_page already uses folio_memcg_kmem to check if a folio has been uncharged, calling PageMemcgKmem(page) before __memcg_kmem_uncharge_page is redundant. Signed-off-by: Shengken Lin <shengken.lin@amlogic.com> --- mm/page_alloc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.34.1