| Message ID | 20241104210602.374975-3-willy@infradead.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Introduce acctmem | expand |
On 04.11.24 22:05, Matthew Wilcox (Oracle) wrote: > The last argument to split_page_memcg() is now always 0, so remove it, > effectively reverting commit b8791381d7ed. > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> > --- > include/linux/memcontrol.h | 4 ++-- > mm/memcontrol.c | 26 ++++++++++++++------------ > mm/page_alloc.c | 4 ++-- > 3 files changed, 18 insertions(+), 16 deletions(-) > > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h > index 5502aa8e138e..a787080f814f 100644 > --- a/include/linux/memcontrol.h > +++ b/include/linux/memcontrol.h > @@ -1044,7 +1044,7 @@ static inline void memcg_memory_event_mm(struct mm_struct *mm, > rcu_read_unlock(); > } > > -void split_page_memcg(struct page *head, int old_order, int new_order); > +void split_page_memcg(struct page *first, int order); > > #else /* CONFIG_MEMCG */ > > @@ -1463,7 +1463,7 @@ void count_memcg_event_mm(struct mm_struct *mm, enum vm_event_item idx) > { > } > > -static inline void split_page_memcg(struct page *head, int old_order, int new_order) > +static inline void split_page_memcg(struct page *first, int order) > { > } > #endif /* CONFIG_MEMCG */ > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 5e44d6e7591e..506439a5dcfe 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -3034,25 +3034,27 @@ void __memcg_slab_free_hook(struct kmem_cache *s, struct slab *slab, > } > > /* > - * Because folio_memcg(head) is not set on tails, set it now. > + * The memcg data is only set on the first page, now transfer it to all the > + * other pages. > */ > -void split_page_memcg(struct page *head, int old_order, int new_order) > +void split_page_memcg(struct page *first, int order) > { > - struct folio *folio = page_folio(head); > + unsigned long memcg_data = first->memcg_data; > + struct obj_cgroup *objcg; > int i; > - unsigned int old_nr = 1 << old_order; > - unsigned int new_nr = 1 << new_order; > + unsigned int nr = 1 << order; > > - if (mem_cgroup_disabled() || !folio_memcg_charged(folio)) > + if (!memcg_data) > return; > > - for (i = new_nr; i < old_nr; i += new_nr) > - folio_page(folio, i)->memcg_data = folio->memcg_data; > + VM_BUG_ON_PAGE((memcg_data & OBJEXTS_FLAGS_MASK) != MEMCG_DATA_KMEM, > + first); > + objcg = (void *)(memcg_data & ~OBJEXTS_FLAGS_MASK); I'm not sure if dropping the if (mem_cgroup_disabled() || !folio_memcg_charged(folio)) end adding that OBJEXTS_FLAGS_MASK magic here is the right thing to do. It also isn't really part of a revert of b8791381d7ed.
On 4 Nov 2024, at 16:05, Matthew Wilcox (Oracle) wrote: > The last argument to split_page_memcg() is now always 0, so remove it, > effectively reverting commit b8791381d7ed. You forgot to cc me about this. > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> > --- > include/linux/memcontrol.h | 4 ++-- > mm/memcontrol.c | 26 ++++++++++++++------------ > mm/page_alloc.c | 4 ++-- > 3 files changed, 18 insertions(+), 16 deletions(-) > > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h > index 5502aa8e138e..a787080f814f 100644 > --- a/include/linux/memcontrol.h > +++ b/include/linux/memcontrol.h > @@ -1044,7 +1044,7 @@ static inline void memcg_memory_event_mm(struct mm_struct *mm, > rcu_read_unlock(); > } > > -void split_page_memcg(struct page *head, int old_order, int new_order); > +void split_page_memcg(struct page *first, int order); > > #else /* CONFIG_MEMCG */ > > @@ -1463,7 +1463,7 @@ void count_memcg_event_mm(struct mm_struct *mm, enum vm_event_item idx) > { > } > > -static inline void split_page_memcg(struct page *head, int old_order, int new_order) > +static inline void split_page_memcg(struct page *first, int order) > { > } > #endif /* CONFIG_MEMCG */ > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 5e44d6e7591e..506439a5dcfe 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -3034,25 +3034,27 @@ void __memcg_slab_free_hook(struct kmem_cache *s, struct slab *slab, > } > > /* > - * Because folio_memcg(head) is not set on tails, set it now. > + * The memcg data is only set on the first page, now transfer it to all the > + * other pages. > */ > -void split_page_memcg(struct page *head, int old_order, int new_order) > +void split_page_memcg(struct page *first, int order) So split_page_memcg() only handles kmem pages, it is better to rename it to split_kmem_page_memcg() to avoid confusion. Especially if this patchset is merged before my folio_split() patchset and I did not notice it, my patchset would cause memcg issues, since I was still using split_page_memcg() during folio split. -- Best Regards, Yan, Zi
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h index 5502aa8e138e..a787080f814f 100644 --- a/include/linux/memcontrol.h +++ b/include/linux/memcontrol.h @@ -1044,7 +1044,7 @@ static inline void memcg_memory_event_mm(struct mm_struct *mm, rcu_read_unlock(); } -void split_page_memcg(struct page *head, int old_order, int new_order); +void split_page_memcg(struct page *first, int order); #else /* CONFIG_MEMCG */ @@ -1463,7 +1463,7 @@ void count_memcg_event_mm(struct mm_struct *mm, enum vm_event_item idx) { } -static inline void split_page_memcg(struct page *head, int old_order, int new_order) +static inline void split_page_memcg(struct page *first, int order) { } #endif /* CONFIG_MEMCG */ diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 5e44d6e7591e..506439a5dcfe 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -3034,25 +3034,27 @@ void __memcg_slab_free_hook(struct kmem_cache *s, struct slab *slab, } /* - * Because folio_memcg(head) is not set on tails, set it now. + * The memcg data is only set on the first page, now transfer it to all the + * other pages. */ -void split_page_memcg(struct page *head, int old_order, int new_order) +void split_page_memcg(struct page *first, int order) { - struct folio *folio = page_folio(head); + unsigned long memcg_data = first->memcg_data; + struct obj_cgroup *objcg; int i; - unsigned int old_nr = 1 << old_order; - unsigned int new_nr = 1 << new_order; + unsigned int nr = 1 << order; - if (mem_cgroup_disabled() || !folio_memcg_charged(folio)) + if (!memcg_data) return; - for (i = new_nr; i < old_nr; i += new_nr) - folio_page(folio, i)->memcg_data = folio->memcg_data; + VM_BUG_ON_PAGE((memcg_data & OBJEXTS_FLAGS_MASK) != MEMCG_DATA_KMEM, + first); + objcg = (void *)(memcg_data & ~OBJEXTS_FLAGS_MASK); - if (folio_memcg_kmem(folio)) - obj_cgroup_get_many(__folio_objcg(folio), old_nr / new_nr - 1); - else - css_get_many(&folio_memcg(folio)->css, old_nr / new_nr - 1); + for (i = 1; i < nr; i++) + first[i].memcg_data = memcg_data; + + obj_cgroup_get_many(objcg, nr - 1); } unsigned long mem_cgroup_usage(struct mem_cgroup *memcg, bool swap) diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 47048b39b8ca..5523654c9759 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -2823,7 +2823,7 @@ void split_page(struct page *page, unsigned int order) set_page_refcounted(page + i); split_page_owner(page, order, 0); pgalloc_tag_split(page_folio(page), order, 0); - split_page_memcg(page, order, 0); + split_page_memcg(page, order); } EXPORT_SYMBOL_GPL(split_page); @@ -5020,7 +5020,7 @@ static void *make_alloc_exact(unsigned long addr, unsigned int order, split_page_owner(page, order, 0); pgalloc_tag_split(page_folio(page), order, 0); - split_page_memcg(page, order, 0); + split_page_memcg(page, order); while (page < --last) set_page_refcounted(last);
The last argument to split_page_memcg() is now always 0, so remove it, effectively reverting commit b8791381d7ed. Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> --- include/linux/memcontrol.h | 4 ++-- mm/memcontrol.c | 26 ++++++++++++++------------ mm/page_alloc.c | 4 ++-- 3 files changed, 18 insertions(+), 16 deletions(-)