From 82369553d6d3bc67c54129a02e0bc0b5b88f3045 Mon Sep 17 00:00:00 2001 From: Hugh Dickins Date: Thu, 7 Feb 2008 00:14:22 -0800 Subject: [PATCH] memcgroup: fix hang with shmem/tmpfs The memcgroup regime relies upon a cgroup reclaiming pages from itself within add_to_page_cache: which may involve some waiting. Whereas shmem and tmpfs rely upon using add_to_page_cache while holding a spinlock: when it cannot wait. The consequence is that when a cgroup reaches its limit, shmem_getpage just hangs - unless there is outside memory pressure too, neither kswapd nor radix_tree_preload get it out of the retry loop. In most cases we can mem_cgroup_cache_charge the page waitably first, to attach the page_cgroup in advance, so add_to_page_cache will do no more than increment a count; then mem_cgroup_uncharge_page after (in both success and failure cases) to balance the books again. And where there used to be a congestion_wait for kswapd (recently made redundant by radix_tree_preload), use mem_cgroup_cache_charge with NULL page to go through a cycle of allocation and freeing, without accounting to any particular page, and without updating the statistics vector. This brings the cgroup below its limit so the next try usually succeeds. Signed-off-by: Hugh Dickins Cc: Balbir Singh Cc: KAMEZAWA Hiroyuki Cc: Mel Gorman Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- mm/memcontrol.c | 37 +++++++++++++++++++++---------------- mm/shmem.c | 28 +++++++++++++++++++++++++++- 2 files changed, 48 insertions(+), 17 deletions(-) diff --git a/mm/memcontrol.c b/mm/memcontrol.c index dbf571547c03..11b23f203d68 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -329,23 +329,26 @@ static int mem_cgroup_charge_common(struct page *page, struct mm_struct *mm, * with it */ retry: - lock_page_cgroup(page); - pc = page_get_page_cgroup(page); - /* - * The page_cgroup exists and the page has already been accounted - */ - if (pc) { - if (unlikely(!atomic_inc_not_zero(&pc->ref_cnt))) { - /* this page is under being uncharged ? */ - unlock_page_cgroup(page); - cpu_relax(); - goto retry; - } else { - unlock_page_cgroup(page); - goto done; + if (page) { + lock_page_cgroup(page); + pc = page_get_page_cgroup(page); + /* + * The page_cgroup exists and + * the page has already been accounted. + */ + if (pc) { + if (unlikely(!atomic_inc_not_zero(&pc->ref_cnt))) { + /* this page is under being uncharged ? */ + unlock_page_cgroup(page); + cpu_relax(); + goto retry; + } else { + unlock_page_cgroup(page); + goto done; + } } + unlock_page_cgroup(page); } - unlock_page_cgroup(page); pc = kzalloc(sizeof(struct page_cgroup), gfp_mask); if (pc == NULL) @@ -404,7 +407,7 @@ static int mem_cgroup_charge_common(struct page *page, struct mm_struct *mm, if (ctype == MEM_CGROUP_CHARGE_TYPE_CACHE) pc->flags |= PAGE_CGROUP_FLAG_CACHE; - if (page_cgroup_assign_new_page_cgroup(page, pc)) { + if (!page || page_cgroup_assign_new_page_cgroup(page, pc)) { /* * Another charge has been added to this page already. * We take lock_page_cgroup(page) again and read @@ -413,6 +416,8 @@ static int mem_cgroup_charge_common(struct page *page, struct mm_struct *mm, res_counter_uncharge(&mem->res, PAGE_SIZE); css_put(&mem->css); kfree(pc); + if (!page) + goto done; goto retry; } diff --git a/mm/shmem.c b/mm/shmem.c index 0f246c44a574..85bed948fafc 100644 --- a/mm/shmem.c +++ b/mm/shmem.c @@ -912,9 +912,13 @@ static int shmem_unuse_inode(struct shmem_inode_info *info, swp_entry_t entry, s error = 1; if (!inode) goto out; - error = radix_tree_preload(GFP_KERNEL); + /* Precharge page while we can wait, compensate afterwards */ + error = mem_cgroup_cache_charge(page, current->mm, GFP_KERNEL); if (error) goto out; + error = radix_tree_preload(GFP_KERNEL); + if (error) + goto uncharge; error = 1; spin_lock(&info->lock); @@ -947,6 +951,8 @@ static int shmem_unuse_inode(struct shmem_inode_info *info, swp_entry_t entry, s shmem_swp_unmap(ptr); spin_unlock(&info->lock); radix_tree_preload_end(); +uncharge: + mem_cgroup_uncharge_page(page); out: unlock_page(page); page_cache_release(page); @@ -1308,6 +1314,13 @@ static int shmem_getpage(struct inode *inode, unsigned long idx, spin_unlock(&info->lock); unlock_page(swappage); page_cache_release(swappage); + if (error == -ENOMEM) { + /* allow reclaim from this memory cgroup */ + error = mem_cgroup_cache_charge(NULL, + current->mm, gfp & ~__GFP_HIGHMEM); + if (error) + goto failed; + } goto repeat; } } else if (sgp == SGP_READ && !filepage) { @@ -1353,6 +1366,17 @@ static int shmem_getpage(struct inode *inode, unsigned long idx, goto failed; } + /* Precharge page while we can wait, compensate after */ + error = mem_cgroup_cache_charge(filepage, current->mm, + gfp & ~__GFP_HIGHMEM); + if (error) { + page_cache_release(filepage); + shmem_unacct_blocks(info->flags, 1); + shmem_free_blocks(inode, 1); + filepage = NULL; + goto failed; + } + spin_lock(&info->lock); entry = shmem_swp_alloc(info, idx, sgp); if (IS_ERR(entry)) @@ -1364,6 +1388,7 @@ static int shmem_getpage(struct inode *inode, unsigned long idx, if (error || swap.val || 0 != add_to_page_cache_lru( filepage, mapping, idx, GFP_NOWAIT)) { spin_unlock(&info->lock); + mem_cgroup_uncharge_page(filepage); page_cache_release(filepage); shmem_unacct_blocks(info->flags, 1); shmem_free_blocks(inode, 1); @@ -1372,6 +1397,7 @@ static int shmem_getpage(struct inode *inode, unsigned long idx, goto failed; goto repeat; } + mem_cgroup_uncharge_page(filepage); info->flags |= SHMEM_PAGEIN; } -- GitLab