Messages in this thread | | | Subject | Re: [PATCH] mm/filemap: Adding missing mem_cgroup_uncharge() to __add_to_page_cache_locked() | From | Waiman Long <> | Date | Mon, 25 Jan 2021 09:09:26 -0500 |
| |
On 1/24/21 11:36 PM, Matthew Wilcox wrote: > On Sun, Jan 24, 2021 at 11:24:41PM -0500, Waiman Long wrote: >> diff --git a/mm/filemap.c b/mm/filemap.c >> index 5c9d564317a5..aa0e0fb04670 100644 >> --- a/mm/filemap.c >> +++ b/mm/filemap.c >> @@ -835,6 +835,7 @@ noinline int __add_to_page_cache_locked(struct page *page, >> XA_STATE(xas, &mapping->i_pages, offset); >> int huge = PageHuge(page); >> int error; >> + bool charged = false; > I don't think we need this extra bool. > >> @@ -896,6 +898,8 @@ noinline int __add_to_page_cache_locked(struct page *page, >> >> if (xas_error(&xas)) { >> error = xas_error(&xas); >> + if (charged) >> + mem_cgroup_uncharge(page); >> goto error; >> } > Better: > > - goto error; > + goto uncharge; > ... > +uncharge: > + if (!huge) > + mem_cgroup_uncharge(page); > error: > ... > That was my original plan. After finding out there was a potentially conflicting patch in linux-next:
commit 7a02fa97b897 ("secretmem: add memcg accounting")
@@ -839,7 +840,7 @@ noinline int __add_to_page_cache_locked(struct page *page, page->mapping = mapping; page->index = offset;
- if (!huge) { + if (!huge && !page_is_secretmem(page)) { error = mem_cgroup_charge(page, current->mm, gfp); if (error) goto error;
Adding a boolean is an easy way out without conflicting it.
Cheers, Longman
| |