On Fri, Jun 13, 2014 at 8:36 AM, Chen Yucong <slaoub@xxxxxxxxx> wrote: > shrink_page_list() has too many arguments that have already reached ten. > Some of those arguments and temporary variables introduces extra 80 bytes > on the stack. This patch wraps five parameters into shrink_result and removes > some temporary variables, thus making the relative functions to consume fewer > stack space. I think it's better to put them into struct scan_control. Reset them before calling shrinker or take a snapshot to get delta. > > Before mm/vmscan.c is changed: > text data bss dec hex filename > 6876698 957224 966656 8800578 864942 vmlinux-3.15 > > After mm/vmscan.c is changed: > text data bss dec hex filename > 6876506 957224 966656 8800386 864882 vmlinux-3.15 > > > scripts/checkstack.pl can be used for checking the change of the target function stack. > > Before mm/vmscan.c is changed: > > 0xffffffff810af103 shrink_inactive_list []: 152 > 0xffffffff810af43d shrink_inactive_list []: 152 > ------------------------------------------------------------- > 0xffffffff810aede8 reclaim_clean_pages_from_list []: 184 > 0xffffffff810aeef8 reclaim_clean_pages_from_list []: 184 > ------------------------------------------------------------- > 0xffffffff810ae582 shrink_page_list []: 232 > 0xffffffff810aedb5 shrink_page_list []: 232 > > After mm/vmscan.c is changed:: > > 0xffffffff810af078 shrink_inactive_list []: 120 > 0xffffffff810af36d shrink_inactive_list []: 120 > ------------------------------------------------------------- > 0xffffffff810aed6c reclaim_clean_pages_from_list []: 152 > 0xffffffff810aee68 reclaim_clean_pages_from_list []: 152 > -------------------------------------------------------------------------------------- > 0xffffffff810ae586 shrink_page_list []: 184 ---> sub $0xb8,%rsp > 0xffffffff810aed36 shrink_page_list []: 184 ---> add $0xb8,%rsp > > Via the above figures, we can find that the difference value of the stack is 32 for > shrink_inactive_list and reclaim_clean_pages_from_list, and this value is 48(232-184) > for shrink_page_list. From the hierarchy of functions called, the total difference > value is 80(32+48) for this change. > > Changes since v1: https://lkml.org/lkml/2014/6/12/159 > * Rename arg_container to shrink_result > * Change the the way of initializing shrink_result object. > > Signed-off-by: Chen Yucong <slaoub@xxxxxxxxx> > --- > mm/vmscan.c | 62 ++++++++++++++++++++++++++--------------------------------- > 1 file changed, 27 insertions(+), 35 deletions(-) > > diff --git a/mm/vmscan.c b/mm/vmscan.c > index a8ffe4e..3f28e39 100644 > --- a/mm/vmscan.c > +++ b/mm/vmscan.c > @@ -791,28 +791,31 @@ static void page_check_dirty_writeback(struct page *page, > } > > /* > + * Callers pass a prezeroed shrink_result into the shrink functions to gather > + * statistics about how many pages of particular states were processed > + */ > +struct shrink_result { > + unsigned long nr_dirty; > + unsigned long nr_unqueued_dirty; > + unsigned long nr_congested; > + unsigned long nr_writeback; > + unsigned long nr_immediate; > +}; > + > +/* > * shrink_page_list() returns the number of reclaimed pages > */ > static unsigned long shrink_page_list(struct list_head *page_list, > struct zone *zone, > struct scan_control *sc, > enum ttu_flags ttu_flags, > - unsigned long *ret_nr_dirty, > - unsigned long *ret_nr_unqueued_dirty, > - unsigned long *ret_nr_congested, > - unsigned long *ret_nr_writeback, > - unsigned long *ret_nr_immediate, > + struct shrink_result *sr, > bool force_reclaim) > { > LIST_HEAD(ret_pages); > LIST_HEAD(free_pages); > int pgactivate = 0; > - unsigned long nr_unqueued_dirty = 0; > - unsigned long nr_dirty = 0; > - unsigned long nr_congested = 0; > unsigned long nr_reclaimed = 0; > - unsigned long nr_writeback = 0; > - unsigned long nr_immediate = 0; > > cond_resched(); > > @@ -858,10 +861,10 @@ static unsigned long shrink_page_list(struct list_head *page_list, > */ > page_check_dirty_writeback(page, &dirty, &writeback); > if (dirty || writeback) > - nr_dirty++; > + sr->nr_dirty++; > > if (dirty && !writeback) > - nr_unqueued_dirty++; > + sr->nr_unqueued_dirty++; > > /* > * Treat this page as congested if the underlying BDI is or if > @@ -872,7 +875,7 @@ static unsigned long shrink_page_list(struct list_head *page_list, > mapping = page_mapping(page); > if ((mapping && bdi_write_congested(mapping->backing_dev_info)) || > (writeback && PageReclaim(page))) > - nr_congested++; > + sr->nr_congested++; > > /* > * If a page at the tail of the LRU is under writeback, there > @@ -916,7 +919,7 @@ static unsigned long shrink_page_list(struct list_head *page_list, > if (current_is_kswapd() && > PageReclaim(page) && > zone_is_reclaim_writeback(zone)) { > - nr_immediate++; > + sr->nr_immediate++; > goto keep_locked; > > /* Case 2 above */ > @@ -934,7 +937,7 @@ static unsigned long shrink_page_list(struct list_head *page_list, > * and it's also appropriate in global reclaim. > */ > SetPageReclaim(page); > - nr_writeback++; > + sr->nr_writeback++; > > goto keep_locked; > > @@ -1132,11 +1135,6 @@ keep: > list_splice(&ret_pages, page_list); > count_vm_events(PGACTIVATE, pgactivate); > mem_cgroup_uncharge_end(); > - *ret_nr_dirty += nr_dirty; > - *ret_nr_congested += nr_congested; > - *ret_nr_unqueued_dirty += nr_unqueued_dirty; > - *ret_nr_writeback += nr_writeback; > - *ret_nr_immediate += nr_immediate; > return nr_reclaimed; > } > > @@ -1148,7 +1146,8 @@ unsigned long reclaim_clean_pages_from_list(struct zone *zone, > .priority = DEF_PRIORITY, > .may_unmap = 1, > }; > - unsigned long ret, dummy1, dummy2, dummy3, dummy4, dummy5; > + unsigned long ret; > + struct shrink_result dummy = { }; > struct page *page, *next; > LIST_HEAD(clean_pages); > > @@ -1161,8 +1160,7 @@ unsigned long reclaim_clean_pages_from_list(struct zone *zone, > } > > ret = shrink_page_list(&clean_pages, zone, &sc, > - TTU_UNMAP|TTU_IGNORE_ACCESS, > - &dummy1, &dummy2, &dummy3, &dummy4, &dummy5, true); > + TTU_UNMAP|TTU_IGNORE_ACCESS, &dummy, true); > list_splice(&clean_pages, page_list); > mod_zone_page_state(zone, NR_ISOLATED_FILE, -ret); > return ret; > @@ -1469,11 +1467,7 @@ shrink_inactive_list(unsigned long nr_to_scan, struct lruvec *lruvec, > unsigned long nr_scanned; > unsigned long nr_reclaimed = 0; > unsigned long nr_taken; > - unsigned long nr_dirty = 0; > - unsigned long nr_congested = 0; > - unsigned long nr_unqueued_dirty = 0; > - unsigned long nr_writeback = 0; > - unsigned long nr_immediate = 0; > + struct shrink_result sr = { }; > isolate_mode_t isolate_mode = 0; > int file = is_file_lru(lru); > struct zone *zone = lruvec_zone(lruvec); > @@ -1515,9 +1509,7 @@ shrink_inactive_list(unsigned long nr_to_scan, struct lruvec *lruvec, > return 0; > > nr_reclaimed = shrink_page_list(&page_list, zone, sc, TTU_UNMAP, > - &nr_dirty, &nr_unqueued_dirty, &nr_congested, > - &nr_writeback, &nr_immediate, > - false); > + &sr, false); > > spin_lock_irq(&zone->lru_lock); > > @@ -1554,7 +1546,7 @@ shrink_inactive_list(unsigned long nr_to_scan, struct lruvec *lruvec, > * of pages under pages flagged for immediate reclaim and stall if any > * are encountered in the nr_immediate check below. > */ > - if (nr_writeback && nr_writeback == nr_taken) > + if (sr.nr_writeback && sr.nr_writeback == nr_taken) > zone_set_flag(zone, ZONE_WRITEBACK); > > /* > @@ -1566,7 +1558,7 @@ shrink_inactive_list(unsigned long nr_to_scan, struct lruvec *lruvec, > * Tag a zone as congested if all the dirty pages scanned were > * backed by a congested BDI and wait_iff_congested will stall. > */ > - if (nr_dirty && nr_dirty == nr_congested) > + if (sr.nr_dirty && sr.nr_dirty == sr.nr_congested) > zone_set_flag(zone, ZONE_CONGESTED); > > /* > @@ -1576,7 +1568,7 @@ shrink_inactive_list(unsigned long nr_to_scan, struct lruvec *lruvec, > * pages from reclaim context. It will forcibly stall in the > * next check. > */ > - if (nr_unqueued_dirty == nr_taken) > + if (sr.nr_unqueued_dirty == nr_taken) > zone_set_flag(zone, ZONE_TAIL_LRU_DIRTY); > > /* > @@ -1585,7 +1577,7 @@ shrink_inactive_list(unsigned long nr_to_scan, struct lruvec *lruvec, > * implies that pages are cycling through the LRU faster than > * they are written so also forcibly stall. > */ > - if ((nr_unqueued_dirty == nr_taken || nr_immediate) && > + if ((sr.nr_unqueued_dirty == nr_taken || sr.nr_immediate) && > current_may_throttle()) > congestion_wait(BLK_RW_ASYNC, HZ/10); > } > -- > 1.7.10.4 > > -- > To unsubscribe, send a message with 'unsubscribe linux-mm' in > the body to majordomo@xxxxxxxxx. For more info on Linux MM, > see: http://www.linux-mm.org/ . > Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a> -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>