Re: [BUG] kmemcg limit defeats __GFP_NOFAIL allocation

[Michal Hocko <mhocko@kernel.org>]

On Wed 04-09-19 23:19:31, Tetsuo Handa wrote:
> On 2019/09/04 20:25, Michal Hocko wrote:
> > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > index 9ec5e12486a7..05a4828edf9d 100644
> > --- a/mm/memcontrol.c
> > +++ b/mm/memcontrol.c
> > @@ -2820,7 +2820,8 @@ int __memcg_kmem_charge_memcg(struct page *page, gfp_t gfp, int order,
> >  		return ret;
> >  
> >  	if (!cgroup_subsys_on_dfl(memory_cgrp_subsys) &&
> > -	    !page_counter_try_charge(&memcg->kmem, nr_pages, &counter)) {
> > +	    !page_counter_try_charge(&memcg->kmem, nr_pages, &counter) &&
> > +	    !(gfp_mask & __GFP_NOFAIL)) {
> >  		cancel_charge(memcg, nr_pages);
> >  		return -ENOMEM;
> >  	}
> > 
> 
> With s/gfp_mask/gfp/ applied, I get no crash but got below warning.
> I don't know relevance with the patch.

Ohh, right. We are trying to uncharge something that hasn't been charged
because page_counter_try_charge has failed. So the fix needs to be more
involved. Sorry, I should have realized that.
---
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 9ec5e12486a7..e18108b2b786 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -2821,6 +2821,16 @@ int __memcg_kmem_charge_memcg(struct page *page, gfp_t gfp, int order,
 
 	if (!cgroup_subsys_on_dfl(memory_cgrp_subsys) &&
 	    !page_counter_try_charge(&memcg->kmem, nr_pages, &counter)) {
+
+		/*
+		 * Enforce __GFP_NOFAIL allocation because callers are not
+		 * prepared to see failures and likely do not have any failure
+		 * handling code.
+		 */
+		if (gfp & __GFP_NOFAIL) {
+			page_counter_charge(&memcg->kmem, nr_pages);
+			return 0;
+		}
 		cancel_charge(memcg, nr_pages);
 		return -ENOMEM;
 	}
-- 
Michal Hocko
SUSE Labs