lkml.org 
[lkml]   [2012]   [Nov]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH -v2 -mm] memcg: do not trigger OOM from add_to_page_cache_locked
On Wed, Nov 28, 2012 at 05:04:47PM +0100, Michal Hocko wrote:
> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> index 095d2b4..5abe441 100644
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -57,13 +57,14 @@ extern int mem_cgroup_newpage_charge(struct page *page, struct mm_struct *mm,
> gfp_t gfp_mask);
> /* for swap handling */
> extern int mem_cgroup_try_charge_swapin(struct mm_struct *mm,
> - struct page *page, gfp_t mask, struct mem_cgroup **memcgp);
> + struct page *page, gfp_t mask, struct mem_cgroup **memcgp,
> + bool oom);

Ok, now I feel almost bad for asking, but why the public interface,
too? You only ever pass "true" in there and this is unlikely to
change anytime soon, no?

> @@ -3754,7 +3753,8 @@ int mem_cgroup_newpage_charge(struct page *page,
> static int __mem_cgroup_try_charge_swapin(struct mm_struct *mm,
> struct page *page,
> gfp_t mask,
> - struct mem_cgroup **memcgp)
> + struct mem_cgroup **memcgp,
> + bool oom)
> {
> struct mem_cgroup *memcg;
> struct page_cgroup *pc;
> @@ -3776,20 +3776,21 @@ static int __mem_cgroup_try_charge_swapin(struct mm_struct *mm,
> if (!memcg)
> goto charge_cur_mm;
> *memcgp = memcg;
> - ret = __mem_cgroup_try_charge(NULL, mask, 1, memcgp, true);
> + ret = __mem_cgroup_try_charge(NULL, mask, 1, memcgp, oom);
> css_put(&memcg->css);
> if (ret == -EINTR)
> ret = 0;
> return ret;
> charge_cur_mm:
> - ret = __mem_cgroup_try_charge(mm, mask, 1, memcgp, true);
> + ret = __mem_cgroup_try_charge(mm, mask, 1, memcgp, oom);
> if (ret == -EINTR)
> ret = 0;
> return ret;
> }

Only this one is needed...

> @@ -3851,7 +3852,7 @@ void mem_cgroup_commit_charge_swapin(struct page *page,
> }
>
> int mem_cgroup_cache_charge(struct page *page, struct mm_struct *mm,
> - gfp_t gfp_mask)
> + gfp_t gfp_mask, bool oom)
> {
> struct mem_cgroup *memcg = NULL;
> enum charge_type type = MEM_CGROUP_CHARGE_TYPE_CACHE;
> @@ -3863,10 +3864,10 @@ int mem_cgroup_cache_charge(struct page *page, struct mm_struct *mm,
> return 0;
>
> if (!PageSwapCache(page))
> - ret = mem_cgroup_charge_common(page, mm, gfp_mask, type);
> + ret = mem_cgroup_charge_common(page, mm, gfp_mask, type, oom);
> else { /* page is swapcache/shmem */
> ret = __mem_cgroup_try_charge_swapin(mm, page,
> - gfp_mask, &memcg);
> + gfp_mask, &memcg, oom);
> if (!ret)
> __mem_cgroup_commit_charge_swapin(page, memcg, type);
> }

...for this site.

> diff --git a/mm/memory.c b/mm/memory.c
> index 6891d3b..afad903 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -2991,7 +2991,7 @@ static int do_swap_page(struct mm_struct *mm, struct vm_area_struct *vma,
> }
> }
>
> - if (mem_cgroup_try_charge_swapin(mm, page, GFP_KERNEL, &ptr)) {
> + if (mem_cgroup_try_charge_swapin(mm, page, GFP_KERNEL, &ptr, true)) {
> ret = VM_FAULT_OOM;
> goto out_page;
> }

Can not happen for shmem, the fault handler uses vma->vm_ops->fault.

> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 2f8e429..8ec511e 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -828,7 +828,7 @@ static int unuse_pte(struct vm_area_struct *vma, pmd_t *pmd,
> int ret = 1;
>
> if (mem_cgroup_try_charge_swapin(vma->vm_mm, page,
> - GFP_KERNEL, &memcg)) {
> + GFP_KERNEL, &memcg, true)) {
> ret = -ENOMEM;
> goto out_nolock;
> }

Can not happen for shmem, uses shmem_unuse() instead.


\
 
 \ /
  Last update: 2012-11-28 18:01    [W:0.147 / U:2.000 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site