> -----Original Message----- > From: Andrew Morton [mailto:akpm@xxxxxxxxxxxxxxxxxxxx] > Sent: Tuesday, February 24, 2015 2:20 AM > To: Shachar Raindel > Cc: linux-mm@xxxxxxxxx; kirill.shutemov@xxxxxxxxxxxxxxx; > mgorman@xxxxxxx; riel@xxxxxxxxxx; ak@xxxxxxxxxxxxxxx; > matthew.r.wilcox@xxxxxxxxx; dave.hansen@xxxxxxxxxxxxxxx; n- > horiguchi@xxxxxxxxxxxxx; torvalds@xxxxxxxxxxxxxxxxxxxx; Haggai Eran; > aarcange@xxxxxxxxxx; pfeiner@xxxxxxxxxx; hannes@xxxxxxxxxxx; Sagi > Grimberg; walken@xxxxxxxxxx > Subject: Re: [PATCH V5 0/4] Refactor do_wp_page, no functional change > > On Sun, 22 Feb 2015 15:42:14 +0200 Shachar Raindel > <raindel@xxxxxxxxxxxx> wrote: > > > Currently do_wp_page contains 265 code lines. It also contains 9 goto > > statements, of which 5 are targeting labels which are not cleanup > > related. This makes the function extremely difficult to > > understand. The following patches are an attempt at breaking the > > function to its basic components, and making it easier to understand. > > > > The patches are straight forward function extractions from > > do_wp_page. As we extract functions, we remove unneeded parameters and > > simplify the code as much as possible. However, the functionality is > > supposed to remain completely unchanged. The patches also attempt to > > document the functionality of each extracted function. In patch 2, we > > split the unlock logic to the contain logic relevant to specific needs > > of each use case, instead of having huge number of conditional > > decisions in a single unlock flow. > > gcc-4.4.4: > > text data bss dec hex filename > 40898 186 13344 54428 d49c mm/memory.o-before > 41422 186 13456 55064 d718 mm/memory.o-after > > gcc-4.8.2: > > text data bss dec hex filename > 35261 12118 13904 61283 ef63 mm/memory.o > 35646 12278 14032 61956 f204 mm/memory.o > My results (gcc version 4.8.2 20140120 (Red Hat 4.8.2-16)) differ: text data bss dec hex filename 29957 70 32 30059 756b memory.o.next-20150219 30061 70 32 30163 75d3 memory.o.next-20150219+1 30093 70 32 30195 75f3 memory.o.next-20150219+2 30165 70 32 30267 763b memory.o.next-20150219+3 30165 70 32 30267 763b memory.o.next-20150219+4 > The more recent compiler is more interesting but either way, that's a > somewhat disappointing increase in code size for refactoring of a > single function. > Seems like the majority of the size impact (104 bytes out of 208) originate from the first patch - "mm: Refactor do_wp_page, extract the reuse case" This is probably due to changing 3 gotos into returning a function call. As gcc cannot do tail call optimization there, it is forced to create 3 new call sites there. Adding an inline to wp_page_reuse reduced the total size to: text data bss dec hex filename 30109 70 32 30211 7603 memory.o > I had a brief poke around and couldn't find any obvious improvements > to make. IMHO, 152 bytes in code size is a small price to pay for code that is not spaghetti. The patch to add the strategic inline which saves 56 bytes on my GCC: diff --git a/mm/memory.c b/mm/memory.c index b246d22..9025285 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -1990,7 +1990,7 @@ static int do_page_mkwrite(struct vm_area_struct *vma, struct page *page, * case, all we need to do here is to mark the page as writable and update * any related book-keeping. */ -static int wp_page_reuse(struct mm_struct *mm, struct vm_area_struct *vma, +static inline int wp_page_reuse(struct mm_struct *mm, struct vm_area_struct *vma, unsigned long address, pte_t *page_table, spinlock_t *ptl, pte_t orig_pte, struct page *page, int page_mkwrite, Thanks, --Shachar -- 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