Re: [PATCH 1/5] mm: Refactor do_wp_page, extract the reuse case

["Kirill A. Shutemov" <kirill@shutemov.name>]

On Mon, Dec 01, 2014 at 12:34:30PM +0000, Shachar Raindel wrote:
> 
> 
> > -----Original Message-----
> > From: Kirill A. Shutemov [mailto:kirill@shutemov.name]
> > Sent: Monday, December 01, 2014 2:31 PM
> > 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; akpm@linux-foundation.org; torvalds@linux-
> > foundation.org; Haggai Eran; aarcange@redhat.com; pfeiner@google.com;
> > hannes@cmpxchg.org; Sagi Grimberg; walken@google.com
> > Subject: Re: [PATCH 1/5] mm: Refactor do_wp_page, extract the reuse case
> > 
> > On Mon, Dec 01, 2014 at 02:04:41PM +0200, Shachar Raindel wrote:
> > > When do_wp_page is ending, in several cases it needs to reuse the
> > > existing page. This is achieved by making the page table writable,
> > > and possibly updating the page-cache state.
> > >
> > > Currently, this logic was "called" by using a goto jump. This makes
> > > following the control flow of the function harder. It is also
> > > against the coding style guidelines for using goto.
> > >
> > > As the code can easily be refactored into a specialized function,
> > > refactor it out and simplify the code flow in do_wp_page.
> > >
> > > Signed-off-by: Shachar Raindel <raindel@mellanox.com>
> > > ---
> > >  mm/memory.c | 136 ++++++++++++++++++++++++++++++++++-----------------
> > ---------
> > >  1 file changed, 78 insertions(+), 58 deletions(-)
> > >
> > > diff --git a/mm/memory.c b/mm/memory.c
> > > index 3e50383..61334e9 100644
> > > --- a/mm/memory.c
> > > +++ b/mm/memory.c
> > > @@ -2020,6 +2020,75 @@ static int do_page_mkwrite(struct
> > vm_area_struct *vma, struct page *page,
> > >  }
> > >
> > >  /*
> > > + * Handle write page faults for pages that can be reused in the
> > current vma
> > > + *
> > > + * This can happen either due to the mapping being with the VM_SHARED
> > flag,
> > > + * or due to us being the last reference standing to the page. In
> > either
> > > + * 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,
> > > +			 unsigned long address, pte_t *page_table,
> > > +			 spinlock_t *ptl, pte_t orig_pte,
> > > +			 struct page *recycled_page, int dirty_page,
> > 
> > recycled_page? what's wrong with old_page?
> > 
> 
> You are reusing the page in this function, so I was feeling that naming it
> "old" is less indicative than "recycled". However, if you prefer "old", I
> am happy with it.

Sinse it's the only page in the scope, call it simply -- 'page'.

-- 
 Kirill A. Shutemov

--
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>