Message ID | 20210409122959.82264-10-songmuchun@bytedance.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | Use obj_cgroup APIs to charge the LRU pages | expand |
On Fri, Apr 09, 2021 at 08:29:50PM +0800, Muchun Song wrote: > The noinline_for_stack is introduced by commit 666356297ec4 ("vmscan: > set up pagevec as late as possible in shrink_inactive_list()"), its > purpose is to delay the allocation of pagevec as late as possible to > save stack memory. But the commit 2bcf88796381 ("mm: take pagevecs off > reclaim stack") replace pagevecs by lists of pages_to_free. So we do > not need noinline_for_stack, just remove it (let the compiler decide > whether to inline). > > Signed-off-by: Muchun Song <songmuchun@bytedance.com> Good catch. Acked-by: Johannes Weiner <hannes@cmpxchg.org> Since this patch is somewhat independent of the rest of the series, you may want to put it in the very beginning, or even submit it separately, to keep the main series as compact as possible. Reviewers can be more hesitant to get involved with larger series ;)
On Fri, Apr 09, 2021 at 08:29:50PM +0800, Muchun Song wrote: > The noinline_for_stack is introduced by commit 666356297ec4 ("vmscan: > set up pagevec as late as possible in shrink_inactive_list()"), its > purpose is to delay the allocation of pagevec as late as possible to > save stack memory. But the commit 2bcf88796381 ("mm: take pagevecs off > reclaim stack") replace pagevecs by lists of pages_to_free. So we do > not need noinline_for_stack, just remove it (let the compiler decide > whether to inline). > > Signed-off-by: Muchun Song <songmuchun@bytedance.com> Acked-by: Roman Gushchin <guro@fb.com> > --- > mm/vmscan.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/mm/vmscan.c b/mm/vmscan.c > index 64bf07cc20f2..e40b21298d77 100644 > --- a/mm/vmscan.c > +++ b/mm/vmscan.c > @@ -2015,8 +2015,8 @@ static int too_many_isolated(struct pglist_data *pgdat, int file, > * > * Returns the number of pages moved to the given lruvec. > */ > -static unsigned noinline_for_stack move_pages_to_lru(struct lruvec *lruvec, > - struct list_head *list) > +static unsigned int move_pages_to_lru(struct lruvec *lruvec, > + struct list_head *list) > { > int nr_pages, nr_moved = 0; > LIST_HEAD(pages_to_free); > @@ -2096,7 +2096,7 @@ static int current_may_throttle(void) > * shrink_inactive_list() is a helper for shrink_node(). It returns the number > * of reclaimed pages > */ > -static noinline_for_stack unsigned long > +static unsigned long > shrink_inactive_list(unsigned long nr_to_scan, struct lruvec *lruvec, > struct scan_control *sc, enum lru_list lru) > { > -- > 2.11.0 >
On Sat, Apr 10, 2021 at 2:31 AM Johannes Weiner <hannes@cmpxchg.org> wrote: > > On Fri, Apr 09, 2021 at 08:29:50PM +0800, Muchun Song wrote: > > The noinline_for_stack is introduced by commit 666356297ec4 ("vmscan: > > set up pagevec as late as possible in shrink_inactive_list()"), its > > purpose is to delay the allocation of pagevec as late as possible to > > save stack memory. But the commit 2bcf88796381 ("mm: take pagevecs off > > reclaim stack") replace pagevecs by lists of pages_to_free. So we do > > not need noinline_for_stack, just remove it (let the compiler decide > > whether to inline). > > > > Signed-off-by: Muchun Song <songmuchun@bytedance.com> > > Good catch. > > Acked-by: Johannes Weiner <hannes@cmpxchg.org> > > Since this patch is somewhat independent of the rest of the series, > you may want to put it in the very beginning, or even submit it > separately, to keep the main series as compact as possible. Reviewers > can be more hesitant to get involved with larger series ;) OK. I will gather all the cleanup patches into a separate series. Thanks for your suggestion.
On Fri, Apr 9, 2021 at 9:34 PM Muchun Song <songmuchun@bytedance.com> wrote: > > On Sat, Apr 10, 2021 at 2:31 AM Johannes Weiner <hannes@cmpxchg.org> wrote: > > > > On Fri, Apr 09, 2021 at 08:29:50PM +0800, Muchun Song wrote: > > > The noinline_for_stack is introduced by commit 666356297ec4 ("vmscan: > > > set up pagevec as late as possible in shrink_inactive_list()"), its > > > purpose is to delay the allocation of pagevec as late as possible to > > > save stack memory. But the commit 2bcf88796381 ("mm: take pagevecs off > > > reclaim stack") replace pagevecs by lists of pages_to_free. So we do > > > not need noinline_for_stack, just remove it (let the compiler decide > > > whether to inline). > > > > > > Signed-off-by: Muchun Song <songmuchun@bytedance.com> > > > > Good catch. > > > > Acked-by: Johannes Weiner <hannes@cmpxchg.org> > > > > Since this patch is somewhat independent of the rest of the series, > > you may want to put it in the very beginning, or even submit it > > separately, to keep the main series as compact as possible. Reviewers > > can be more hesitant to get involved with larger series ;) > > OK. I will gather all the cleanup patches into a separate series. > Thanks for your suggestion. That would be best. For this patch: Reviewed-by: Shakeel Butt <shakeelb@google.com>
diff --git a/mm/vmscan.c b/mm/vmscan.c index 64bf07cc20f2..e40b21298d77 100644 --- a/mm/vmscan.c +++ b/mm/vmscan.c @@ -2015,8 +2015,8 @@ static int too_many_isolated(struct pglist_data *pgdat, int file, * * Returns the number of pages moved to the given lruvec. */ -static unsigned noinline_for_stack move_pages_to_lru(struct lruvec *lruvec, - struct list_head *list) +static unsigned int move_pages_to_lru(struct lruvec *lruvec, + struct list_head *list) { int nr_pages, nr_moved = 0; LIST_HEAD(pages_to_free); @@ -2096,7 +2096,7 @@ static int current_may_throttle(void) * shrink_inactive_list() is a helper for shrink_node(). It returns the number * of reclaimed pages */ -static noinline_for_stack unsigned long +static unsigned long shrink_inactive_list(unsigned long nr_to_scan, struct lruvec *lruvec, struct scan_control *sc, enum lru_list lru) {
The noinline_for_stack is introduced by commit 666356297ec4 ("vmscan: set up pagevec as late as possible in shrink_inactive_list()"), its purpose is to delay the allocation of pagevec as late as possible to save stack memory. But the commit 2bcf88796381 ("mm: take pagevecs off reclaim stack") replace pagevecs by lists of pages_to_free. So we do not need noinline_for_stack, just remove it (let the compiler decide whether to inline). Signed-off-by: Muchun Song <songmuchun@bytedance.com> --- mm/vmscan.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)