David Rientjes wrote:
> On Sun, 23 Aug 2015, Tetsuo Handa wrote:
>
> > >From 08a638e04351386ab03cd1223988ac7940d4d3aa Mon Sep 17 00:00:00 2001
> > From: Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx>
> > Date: Sat, 1 Aug 2015 22:46:12 +0900
> > Subject: [PATCH 2/2] mm: Fix potentially scheduling in GFP_ATOMIC
> > allocations.
> >
> > Currently, if somebody does GFP_ATOMIC | __GFP_NOFAIL allocation,
> > wait_iff_congested() might be called via __alloc_pages_high_priority()
> > before reaching
> >
> > if (!wait) {
> > WARN_ON_ONCE(gfp_mask & __GFP_NOFAIL);
> > goto nopage;
> > }
> >
> > because gfp_to_alloc_flags() includes ALLOC_NO_WATERMARKS if TIF_MEMDIE
> > was set.
> >
> > We need to check for __GFP_WAIT flag at __alloc_pages_high_priority()
> > in order to make sure that we won't schedule.
> >
>
> I've brought the GFP_ATOMIC | __GFP_NOFAIL combination up before, which
> resulted in the WARN_ON_ONCE() that you cited. We don't support such a
> combination. Fixing up the documentation in any places you feel it is
> deficient would be the best.
>
The purpose of this check is to warn about unassured combination, isn't it?
Then, I think this check should be done like
----------
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 5b5240b..7358225 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -3046,15 +3046,8 @@ retry:
}
/* Atomic allocations - we can't balance anything */
- if (!wait) {
- /*
- * All existing users of the deprecated __GFP_NOFAIL are
- * blockable, so warn of any new users that actually allow this
- * type of allocation to fail.
- */
- WARN_ON_ONCE(gfp_mask & __GFP_NOFAIL);
+ if (!wait)
goto nopage;
- }
/* Avoid recursion of direct reclaim */
if (current->flags & PF_MEMALLOC)
@@ -3183,6 +3176,12 @@ __alloc_pages_nodemask(gfp_t gfp_mask, unsigned int order,
lockdep_trace_alloc(gfp_mask);
+ /*
+ * All existing users of the __GFP_NOFAIL have __GFP_WAIT.
+ * __GFP_NOFAIL allocations without __GFP_WAIT is unassured.
+ */
+ WARN_ON_ONCE((gfp_mask & (__GFP_NOFAIL | __GFP_WAIT)) == __GFP_NOFAIL);
+
might_sleep_if(gfp_mask & __GFP_WAIT);
if (should_fail_alloc_page(gfp_mask, order))
----------
because such allocation requests can succeed at fast path or at
/* This is the last chance, in general, before the goto nopage. */
. If unconditional WARN_ON_ONCE() is too wasteful, maybe we can do like
#ifdef CONFIG_DEBUG_SOMETHING
WARN_ON_ONCE((gfp_mask & (__GFP_NOFAIL | __GFP_WAIT)) == __GFP_NOFAIL);
#endif
.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>