Re: [PATCH 3/5] mm: refactor do_wp_page, extract the page copy flow

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

On Mon, Dec 01, 2014 at 02:04:43PM +0200, Shachar Raindel wrote:
> In some cases, do_wp_page had to copy the page suffering a write fault
> to a new location. If the function logic decided that to do this, it
> was done by jumping with a "goto" operation to the relevant code
> block. This made the code really hard to understand. It is also
> against the kernel coding style guidelines.
> 
> This patch extracts the page copy and page table update logic to a
> separate function. It also clean up the naming, from "gotten" to
> "wp_page_copy", and adds few comments.
> 
> Signed-off-by: Shachar Raindel <raindel@mellanox.com>
> ---
>  mm/memory.c | 238 +++++++++++++++++++++++++++++++++---------------------------
>  1 file changed, 131 insertions(+), 107 deletions(-)
> 
> diff --git a/mm/memory.c b/mm/memory.c
> index dd3bb13..436012d 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -2123,6 +2123,132 @@ static int wp_page_unlock(struct mm_struct *mm, struct vm_area_struct *vma,
>  }
>  
>  /*
> + * Handle the case of a page which we actually need to copy to a new page.
> + *
> + * High level logic flow:
> + *
> + * - Drop the PTL, allocate a page, copy the content.
> + * - Handle book keeping and accounting - cgroups, mmu-notifiers, etc.
> + * - Regain the PTL. If the pte changed, bail out and release the allocated page
> + * - If the pte is still the way we remember it, update the page table and all
> + *   relevant references. This includes dropping the reference the page-table
> + *   held to the old page, as well as updating the rmap.
> + * - In any case, unlock the PTL and drop the reference we took to the old page.
> + */
> +static int wp_page_copy(struct mm_struct *mm, struct vm_area_struct *vma,
> +			unsigned long address, pte_t *page_table, pmd_t *pmd,
> +			spinlock_t *ptl, pte_t orig_pte, struct page *old_page)
> +	__releases(ptl)
> +{
> +	struct page *new_page = NULL;
> +	pte_t entry;
> +	int page_copied = 0;
> +	const unsigned long mmun_start = address & PAGE_MASK;	/* For mmu_notifiers */
> +	const unsigned long mmun_end = mmun_start + PAGE_SIZE;	/* For mmu_notifiers */
> +	struct mem_cgroup *memcg;
> +
> +	pte_unmap_unlock(page_table, ptl);

Move ptl unlock to caller. No need in __releases(ptl) and shorter list of
argument.

Otherwise:

Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>

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