RE: [PATCH V5 0/4] Refactor do_wp_page, no functional change

[Shachar Raindel <raindel@mellanox.com>]

> -----Original Message-----
> From: Andrew Morton [mailto:akpm@linux-foundation.org]
> Sent: Tuesday, February 24, 2015 2:20 AM
> To: Shachar Raindel
> Cc: linux-mm@kvack.org; kirill.shutemov@linux.intel.com;
> mgorman@suse.de; riel@redhat.com; ak@linux.intel.com;
> matthew.r.wilcox@intel.com; dave.hansen@linux.intel.com; n-
> horiguchi@ah.jp.nec.com; torvalds@linux-foundation.org; Haggai Eran;
> aarcange@redhat.com; pfeiner@google.com; hannes@cmpxchg.org; Sagi
> Grimberg; walken@google.com
> 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@mellanox.com> 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@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>