| Message ID | 20210204163055.56080-1-songmuchun@bytedance.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] mm: memcontrol: replace the loop with a list_for_each_entry() | expand |
On Fri, Feb 05, 2021 at 12:30:55AM +0800, Muchun Song wrote: > The rule of list walk has gone since: > > commit a9d5adeeb4b2 ("mm/memcontrol: allow to uncharge page without using page->lru field") > > So remove the strange comment and replace the loop with a > list_for_each_entry(). > > There is only one caller of the uncharge_list(). So just fold it into > mem_cgroup_uncharge_list() and remove it. > > Signed-off-by: Muchun Song <songmuchun@bytedance.com> Acked-by: Johannes Weiner <hannes@cmpxchg.org> Thank you, Muchun!
On Fri, Feb 05, 2021 at 12:30:55AM +0800, Muchun Song wrote: > The rule of list walk has gone since: > > commit a9d5adeeb4b2 ("mm/memcontrol: allow to uncharge page without using page->lru field") > > So remove the strange comment and replace the loop with a > list_for_each_entry(). > > There is only one caller of the uncharge_list(). So just fold it into > mem_cgroup_uncharge_list() and remove it. > > Signed-off-by: Muchun Song <songmuchun@bytedance.com> > --- > v2: > - Fold uncharge_list() to mem_cgroup_uncharge_list(). > > mm/memcontrol.c | 35 ++++++++--------------------------- > 1 file changed, 8 insertions(+), 27 deletions(-) Nice cleanup! Acked-by: Roman Gushchin <guro@fb.com> Thanks.
Hi: On 2021/2/5 0:30, Muchun Song wrote: > The rule of list walk has gone since: > > commit a9d5adeeb4b2 ("mm/memcontrol: allow to uncharge page without using page->lru field") > > So remove the strange comment and replace the loop with a > list_for_each_entry(). > > There is only one caller of the uncharge_list(). So just fold it into > mem_cgroup_uncharge_list() and remove it. > > Signed-off-by: Muchun Song <songmuchun@bytedance.com> LGTM. Thanks. Reviewed-by: Miaohe Lin <linmiaohe@huawei.com> > --- > v2: > - Fold uncharge_list() to mem_cgroup_uncharge_list(). > > mm/memcontrol.c | 35 ++++++++--------------------------- > 1 file changed, 8 insertions(+), 27 deletions(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index ed5cc78a8dbf..8c035846c7a4 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -6862,31 +6862,6 @@ static void uncharge_page(struct page *page, struct uncharge_gather *ug) > css_put(&ug->memcg->css); > } > > -static void uncharge_list(struct list_head *page_list) > -{ > - struct uncharge_gather ug; > - struct list_head *next; > - > - uncharge_gather_clear(&ug); > - > - /* > - * Note that the list can be a single page->lru; hence the > - * do-while loop instead of a simple list_for_each_entry(). > - */ > - next = page_list->next; > - do { > - struct page *page; > - > - page = list_entry(next, struct page, lru); > - next = page->lru.next; > - > - uncharge_page(page, &ug); > - } while (next != page_list); > - > - if (ug.memcg) > - uncharge_batch(&ug); > -} > - > /** > * mem_cgroup_uncharge - uncharge a page > * @page: page to uncharge > @@ -6918,11 +6893,17 @@ void mem_cgroup_uncharge(struct page *page) > */ > void mem_cgroup_uncharge_list(struct list_head *page_list) > { > + struct uncharge_gather ug; > + struct page *page; > + > if (mem_cgroup_disabled()) > return; > > - if (!list_empty(page_list)) > - uncharge_list(page_list); > + uncharge_gather_clear(&ug); > + list_for_each_entry(page, page_list, lru) > + uncharge_page(page, &ug); > + if (ug.memcg) > + uncharge_batch(&ug); > } > > /** >
On Fri 05-02-21 00:30:55, Muchun Song wrote: > The rule of list walk has gone since: > > commit a9d5adeeb4b2 ("mm/memcontrol: allow to uncharge page without using page->lru field") > > So remove the strange comment and replace the loop with a > list_for_each_entry(). > > There is only one caller of the uncharge_list(). So just fold it into > mem_cgroup_uncharge_list() and remove it. > > Signed-off-by: Muchun Song <songmuchun@bytedance.com> Acked-by: Michal Hocko <mhocko@suse.com> Thanks! > --- > v2: > - Fold uncharge_list() to mem_cgroup_uncharge_list(). > > mm/memcontrol.c | 35 ++++++++--------------------------- > 1 file changed, 8 insertions(+), 27 deletions(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index ed5cc78a8dbf..8c035846c7a4 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -6862,31 +6862,6 @@ static void uncharge_page(struct page *page, struct uncharge_gather *ug) > css_put(&ug->memcg->css); > } > > -static void uncharge_list(struct list_head *page_list) > -{ > - struct uncharge_gather ug; > - struct list_head *next; > - > - uncharge_gather_clear(&ug); > - > - /* > - * Note that the list can be a single page->lru; hence the > - * do-while loop instead of a simple list_for_each_entry(). > - */ > - next = page_list->next; > - do { > - struct page *page; > - > - page = list_entry(next, struct page, lru); > - next = page->lru.next; > - > - uncharge_page(page, &ug); > - } while (next != page_list); > - > - if (ug.memcg) > - uncharge_batch(&ug); > -} > - > /** > * mem_cgroup_uncharge - uncharge a page > * @page: page to uncharge > @@ -6918,11 +6893,17 @@ void mem_cgroup_uncharge(struct page *page) > */ > void mem_cgroup_uncharge_list(struct list_head *page_list) > { > + struct uncharge_gather ug; > + struct page *page; > + > if (mem_cgroup_disabled()) > return; > > - if (!list_empty(page_list)) > - uncharge_list(page_list); > + uncharge_gather_clear(&ug); > + list_for_each_entry(page, page_list, lru) > + uncharge_page(page, &ug); > + if (ug.memcg) > + uncharge_batch(&ug); > } > > /** > -- > 2.11.0
On Thu, Feb 4, 2021 at 8:39 AM Muchun Song <songmuchun@bytedance.com> wrote: > > The rule of list walk has gone since: > > commit a9d5adeeb4b2 ("mm/memcontrol: allow to uncharge page without using page->lru field") > > So remove the strange comment and replace the loop with a > list_for_each_entry(). > > There is only one caller of the uncharge_list(). So just fold it into > mem_cgroup_uncharge_list() and remove it. > > Signed-off-by: Muchun Song <songmuchun@bytedance.com> Reviewed-by: Shakeel Butt <shakeelb@google.com>
diff --git a/mm/memcontrol.c b/mm/memcontrol.c index ed5cc78a8dbf..8c035846c7a4 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -6862,31 +6862,6 @@ static void uncharge_page(struct page *page, struct uncharge_gather *ug) css_put(&ug->memcg->css); } -static void uncharge_list(struct list_head *page_list) -{ - struct uncharge_gather ug; - struct list_head *next; - - uncharge_gather_clear(&ug); - - /* - * Note that the list can be a single page->lru; hence the - * do-while loop instead of a simple list_for_each_entry(). - */ - next = page_list->next; - do { - struct page *page; - - page = list_entry(next, struct page, lru); - next = page->lru.next; - - uncharge_page(page, &ug); - } while (next != page_list); - - if (ug.memcg) - uncharge_batch(&ug); -} - /** * mem_cgroup_uncharge - uncharge a page * @page: page to uncharge @@ -6918,11 +6893,17 @@ void mem_cgroup_uncharge(struct page *page) */ void mem_cgroup_uncharge_list(struct list_head *page_list) { + struct uncharge_gather ug; + struct page *page; + if (mem_cgroup_disabled()) return; - if (!list_empty(page_list)) - uncharge_list(page_list); + uncharge_gather_clear(&ug); + list_for_each_entry(page, page_list, lru) + uncharge_page(page, &ug); + if (ug.memcg) + uncharge_batch(&ug); } /**
The rule of list walk has gone since: commit a9d5adeeb4b2 ("mm/memcontrol: allow to uncharge page without using page->lru field") So remove the strange comment and replace the loop with a list_for_each_entry(). There is only one caller of the uncharge_list(). So just fold it into mem_cgroup_uncharge_list() and remove it. Signed-off-by: Muchun Song <songmuchun@bytedance.com> --- v2: - Fold uncharge_list() to mem_cgroup_uncharge_list(). mm/memcontrol.c | 35 ++++++++--------------------------- 1 file changed, 8 insertions(+), 27 deletions(-)