mm/vmscan.c: wrap five parameters into arg_container in shrink_page_list()
Subsystems (3)
PATCH 1/1 mm/vmscan.c: wrap five parameters into arg_container in shrink_page_list()
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 arg_container and removes some temporary variables, thus making shrink_page_list() to consume fewer stack space.
Before mm/vmscan.c is modified:
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
--- mm/vmscan.c | 64 +++++++++++++++++++++++++++-------------------------------- 1 file changed, 29 insertions(+), 35 deletions(-)
mm/vmscan.c +29 -35
@@ -790,6 +790,14 @@ static void page_check_dirty_writeback(struct page *page, mapping->a_ops->is_dirty_writeback (page, dirty, writeback); } +struct arg_container { +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 */
@@ -797,22 +805,13 @@ 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 arg_container *ac, 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 +857,10 @@ static unsigned long shrink_page_list(struct list_head *page_list,*/ page_check_dirty_writeback (page, &dirty, &writeback);if (dirty || writeback) - nr_dirty++; + ac->nr_dirty++;if (dirty && !writeback) - nr_unqueued_dirty++; + ac->nr_unqueued_dirty++;/* * Treat this page as congested if the underlying BDI is or if
@@ -872,7 +871,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++; + ac->nr_congested++;/* * If a page at the tail of the LRU is under writeback, there
@@ -916,7 +915,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++; + ac->nr_immediate++; goto keep_locked;/* Case 2 above */
@@ -934,7 +933,7 @@ static unsigned long shrink_page_list(struct list_head *page_list,* and it's also appropriate in global reclaim. */ SetPageReclaim (page); - nr_writeback++; + ac->nr_writeback++; goto keep_locked;
@@ -1132,11 +1131,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 +1142,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 arg_container dummy;struct page *page, *next;LIST_HEAD (clean_pages);
@@ -1161,8 +1156,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 +1463,13 @@ 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 arg_container ac = { + .nr_dirty =0 , + .nr_congested =0 , + .nr_unqueued_dirty =0 , + .nr_writeback =0 , + .nr_immediate =0 , + };isolate_mode_t isolate_mode =0 ;int file =is_file_lru (lru);struct zone *zone =lruvec_zone (lruvec);
@@ -1515,9 +1511,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); + &ac, false);spin_lock_irq (&zone->lru_lock);
@@ -1554,7 +1548,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 (ac.nr_writeback && ac.nr_writeback == nr_taken)zone_set_flag (zone,ZONE_WRITEBACK );/*
@@ -1566,7 +1560,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 (ac.nr_dirty && ac.nr_dirty == ac.nr_congested)zone_set_flag (zone,ZONE_CONGESTED );/*
@@ -1576,7 +1570,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 (ac.nr_unqueued_dirty == nr_taken)zone_set_flag (zone,ZONE_TAIL_LRU_DIRTY );/*
@@ -1585,7 +1579,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 ((ac.nr_unqueued_dirty == nr_taken || ac.nr_immediate) &¤t_may_throttle ())congestion_wait (BLK_RW_ASYNC ,HZ /10 ); }
Signature · 7 lines
1.7.10.4 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 6 lines · Chen Yucong
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
arg_containerand removes some temporary variables, thus makingshrink_page_list()to consume fewer stack space.
Won't the container with those arguments now live on the stack, using up the same space that the variables used to take?
Quoted 195 lines
Before
mm/vmscan.cis modified:
text data bss dec hex filename
6876698 957224 966656 8800578 864942 vmlinux-3.15After
mm/vmscan.cis changed:
text data bss dec hex filename
6876506 957224 966656 8800386 864882 vmlinux-3.15Signed-off-by Chen Yucong <[email protected]>--- mm/vmscan.c | 64 +++++++++++++++++++++++++++-------------------------------- 1 file changed, 29 insertions(+), 35 deletions(-)mm/vmscan.c +29
-35@@ -790,6 +790,14 @@ static void page_check_dirty_writeback(struct page *page, mapping->a_ops->is_dirty_writeback (page, dirty, writeback); } +struct arg_container { +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 */ @@ -797,22 +805,13 @@ 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 arg_container *ac, 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 +857,10 @@ static unsigned long shrink_page_list(struct list_head *page_list,*/ page_check_dirty_writeback (page, &dirty, &writeback);if (dirty || writeback) - nr_dirty++; + ac->nr_dirty++;if (dirty && !writeback) - nr_unqueued_dirty++; + ac->nr_unqueued_dirty++;/* * Treat this page as congested if the underlying BDI is or if @@ -872,7 +871,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++; + ac->nr_congested++;/* * If a page at the tail of the LRU is under writeback, there @@ -916,7 +915,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++; + ac->nr_immediate++; goto keep_locked;/* Case 2 above */ @@ -934,7 +933,7 @@ static unsigned long shrink_page_list(struct list_head *page_list,* and it's also appropriate in global reclaim. */ SetPageReclaim (page); - nr_writeback++; + ac->nr_writeback++; goto keep_locked;@@ -1132,11 +1131,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 +1142,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 arg_container dummy;struct page *page, *next;LIST_HEAD (clean_pages);@@ -1161,8 +1156,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 +1463,13 @@ 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 arg_container ac = { + .nr_dirty =0 , + .nr_congested =0 , + .nr_unqueued_dirty =0 , + .nr_writeback =0 , + .nr_immediate =0 , + };isolate_mode_t isolate_mode =0 ;int file =is_file_lru (lru);struct zone *zone =lruvec_zone (lruvec);@@ -1515,9 +1511,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); + &ac, false);spin_lock_irq (&zone->lru_lock);@@ -1554,7 +1548,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 (ac.nr_writeback && ac.nr_writeback == nr_taken)zone_set_flag (zone,ZONE_WRITEBACK );/* @@ -1566,7 +1560,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 (ac.nr_dirty && ac.nr_dirty == ac.nr_congested)zone_set_flag (zone,ZONE_CONGESTED );/* @@ -1576,7 +1570,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 (ac.nr_unqueued_dirty == nr_taken)zone_set_flag (zone,ZONE_TAIL_LRU_DIRTY );/* @@ -1585,7 +1579,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 ((ac.nr_unqueued_dirty == nr_taken || ac.nr_immediate) &¤t_may_throttle ())congestion_wait (BLK_RW_ASYNC ,HZ /10 ); }
Signature · 7 lines
All rights reversed -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 6 lines · Rik van Riel
Quoted 1 line
shrink_page_list()has too many arguments that have already reachedten.
Quoted 1 line
Some of those arguments and temporary variables introduces extra 80
bytes
Quoted 2 lines
on the stack.
Quoted 8 lines
Quoted 1 line
This patch wraps five parameters into
arg_containerand removes sometemporary
Quoted 1 line
variables, thus making
shrink_page_list()to consume fewer stackspace.
Won't the container with those arguments now live on the stack, using up the same space that the variables used to take?
Of course, the container with those arguments live on the stack.
One of the key reason for introducing this patch is to avoid passing five pointer arguments to shrink_page_list().
The arg_container also uses up the same space that the variables used to take.
If the those arguments is wrapped to arg_container, we just need to pass one pointer to shrink_page_list instead of five.
thx!
cyc
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Andrew Morton
maintainerQuoted 7 lines · On Thu, 12 Jun 2014 17:36:35 +0800 Chen Yucong <[email protected]> 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
arg_containerand removes some temporary variables, thus makingshrink_page_list()to consume fewer stack space.
Quoted 7 lines
Before
mm/vmscan.cis modified:
text data bss dec hex filename
6876698 957224 966656 8800578 864942 vmlinux-3.15After
mm/vmscan.cis changed:
text data bss dec hex filename
6876506 957224 966656 8800386 864882 vmlinux-3.15
Code size reduction is a good sign.
Quoted 1 line
1 file changed, 29 insertions(+), 35 deletions(-)
We can look at the frame pointer alterations. Requires CONFIG_FRAME_POINTER. There's also scripts/checkstack.pl.
Without:
shrink_page_list:
pushq %rbp # movq %rsp, %rbp #, pushq %r15 # pushq %r14 # pushq %r13 # pushq %r12 # pushq %rbx # subq $184, %rsp #,
With:
shrink_page_list:
pushq %rbp # movq %rsp, %rbp #, pushq %r15 # pushq %r14 # pushq %r13 # pushq %r12 # pushq %rbx # subq $136, %rsp #,
So we've saved approx 184-136=48 bytes of stack in shrink_page_list(). shrink_inactive_list() stack space is unchanged.
Please do this sort of analysis yourself and include it in the changelogs.
mm/vmscan.c static void page_check_dirty_writeback(struct page *page, · line 799
Quoted 13 lines
mm/vmscan.c +7
-0@@ -790,6 +790,14 @@ static void page_check_dirty_writeback(struct page *page, mapping->a_ops->is_dirty_writeback (page, dirty, writeback); } +struct arg_container { +unsigned long nr_dirty; +unsigned long nr_unqueued_dirty; +unsigned long nr_congested; +unsigned long nr_writeback; +unsigned long nr_immediate; +};
This name is dreadful. Let's give it a nice, meaningful name and document it appropriately. So it all looks like a part of the vmscan code and not some hack which was bolted onto the side to save a bit of stack.
Something like
/*
* 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 {
...mm/vmscan.c unsigned long reclaim_clean_pages_from_list(struct zone *zone, · line 1146
Quoted 13 lines
mm/vmscan.c +2
-1/* * shrink_page_list() returns the number of reclaimed pages */ @@ -1148,7 +1142,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 arg_container dummy;
If we're not going to use this then we can make it static and save more stack. That will have some runtime cost as different CPUs fight over ownership of cachelines but I doubt if it will be significant.
If we leave it on the stack then this code will send kmemcheck berzerk with all the used-uninitialized errors. Presumably that it already the case. Perhaps `dummy' should be initialized if kmemcheck is in operation, dunno.
Quoted 12 lines
mm/vmscan.c +0
-2struct page *page, *next;LIST_HEAD (clean_pages);@@ -1469,11 +1463,13 @@ 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 ;
mm/vmscan.c shrink_inactive_list(unsigned long nr_to_scan, struct lruvec *lruvec, · line 1472
Quoted 10 lines
mm/vmscan.c +7
-3-unsigned long nr_unqueued_dirty =0 ; -unsigned long nr_writeback =0 ; -unsigned long nr_immediate =0 ; +struct arg_container ac = { + .nr_dirty =0 , + .nr_congested =0 , + .nr_unqueued_dirty =0 , + .nr_writeback =0 , + .nr_immediate =0 , + };
This:
struct arg_container ac = { };mm/vmscan.c shrink_inactive_list(unsigned long nr_to_scan, struct lruvec *lruvec,
Quoted 6 lines
mm/vmscan.c +0
-0isolate_mode_t isolate_mode =0 ;int file =is_file_lru (lru);struct zone *zone =lruvec_zone (lruvec);
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>