Re: [PATCH 04/15] memcg: when do_swap's do_wp_page fails

[Balbir Singh <balbir@linux.vnet.ibm.com>]

* Hugh Dickins <hugh@veritas.com> [2008-02-25 23:38:02]:

> Don't uncharge when do_swap_page's call to do_wp_page fails: the page which
> was charged for is there in the pagetable, and will be correctly uncharged
> when that area is unmapped - it was only its COWing which failed.
> 
> And while we're here, remove earlier XXX comment: yes, OR in do_wp_page's
> return value (maybe VM_FAULT_WRITE) with do_swap_page's there; but if it
> fails, mask out success bits, which might confuse some arches e.g. sparc.
> 
> Signed-off-by: Hugh Dickins <hugh@veritas.com>
> ---
> 
>  mm/memory.c |    9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> --- memcg03/mm/memory.c	2008-02-25 14:05:43.000000000 +0000
> +++ memcg04/mm/memory.c	2008-02-25 14:05:47.000000000 +0000
> @@ -2093,12 +2093,9 @@ static int do_swap_page(struct mm_struct
>  	unlock_page(page);
> 
>  	if (write_access) {
> -		/* XXX: We could OR the do_wp_page code with this one? */
> -		if (do_wp_page(mm, vma, address,
> -				page_table, pmd, ptl, pte) & VM_FAULT_OOM) {
> -			mem_cgroup_uncharge_page(page);
> -			ret = VM_FAULT_OOM;
> -		}
> +		ret |= do_wp_page(mm, vma, address, page_table, pmd, ptl, pte);
> +		if (ret & VM_FAULT_ERROR)
> +			ret &= VM_FAULT_ERROR;
>  		goto out;
>  	}
>

Looks good to me. Do you think we could add some of the description
from above as a comment in the code? People would not have to look at
the git log to understand why we did not uncharge.

Otherwise, it looks very good

Acked-by: Balbir Singh <balbir@linux.vnet.ibm.com>
 
-- 
	Warm Regards,
	Balbir Singh
	Linux Technology Center
	IBM, ISTL

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