Commit ea4452de authored by Qi Zheng's avatar Qi Zheng Committed by Andrew Morton

mm: fix unexpected changes to {failslab|fail_page_alloc}.attr

When we specify __GFP_NOWARN, we only expect that no warnings will be
issued for current caller.  But in the __should_failslab() and
__should_fail_alloc_page(), the local GFP flags alter the global
{failslab|fail_page_alloc}.attr, which is persistent and shared by all
tasks.  This is not what we expected, let's fix it.

[akpm@linux-foundation.org: unexport should_fail_ex()]
Link: https://lkml.kernel.org/r/20221118100011.2634-1-zhengqi.arch@bytedance.com
Fixes: 3f913fc5 ("mm: fix missing handler for __GFP_NOWARN")
Signed-off-by: default avatarQi Zheng <zhengqi.arch@bytedance.com>
Reported-by: default avatarDmitry Vyukov <dvyukov@google.com>
Reviewed-by: default avatarAkinobu Mita <akinobu.mita@gmail.com>
Reviewed-by: default avatarJason Gunthorpe <jgg@nvidia.com>
Cc: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: <stable@vger.kernel.org>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
parent de1ccfb6
...@@ -20,7 +20,6 @@ struct fault_attr { ...@@ -20,7 +20,6 @@ struct fault_attr {
atomic_t space; atomic_t space;
unsigned long verbose; unsigned long verbose;
bool task_filter; bool task_filter;
bool no_warn;
unsigned long stacktrace_depth; unsigned long stacktrace_depth;
unsigned long require_start; unsigned long require_start;
unsigned long require_end; unsigned long require_end;
...@@ -32,6 +31,10 @@ struct fault_attr { ...@@ -32,6 +31,10 @@ struct fault_attr {
struct dentry *dname; struct dentry *dname;
}; };
enum fault_flags {
FAULT_NOWARN = 1 << 0,
};
#define FAULT_ATTR_INITIALIZER { \ #define FAULT_ATTR_INITIALIZER { \
.interval = 1, \ .interval = 1, \
.times = ATOMIC_INIT(1), \ .times = ATOMIC_INIT(1), \
...@@ -40,11 +43,11 @@ struct fault_attr { ...@@ -40,11 +43,11 @@ struct fault_attr {
.ratelimit_state = RATELIMIT_STATE_INIT_DISABLED, \ .ratelimit_state = RATELIMIT_STATE_INIT_DISABLED, \
.verbose = 2, \ .verbose = 2, \
.dname = NULL, \ .dname = NULL, \
.no_warn = false, \
} }
#define DECLARE_FAULT_ATTR(name) struct fault_attr name = FAULT_ATTR_INITIALIZER #define DECLARE_FAULT_ATTR(name) struct fault_attr name = FAULT_ATTR_INITIALIZER
int setup_fault_attr(struct fault_attr *attr, char *str); int setup_fault_attr(struct fault_attr *attr, char *str);
bool should_fail_ex(struct fault_attr *attr, ssize_t size, int flags);
bool should_fail(struct fault_attr *attr, ssize_t size); bool should_fail(struct fault_attr *attr, ssize_t size);
#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS #ifdef CONFIG_FAULT_INJECTION_DEBUG_FS
......
...@@ -41,9 +41,6 @@ EXPORT_SYMBOL_GPL(setup_fault_attr); ...@@ -41,9 +41,6 @@ EXPORT_SYMBOL_GPL(setup_fault_attr);
static void fail_dump(struct fault_attr *attr) static void fail_dump(struct fault_attr *attr)
{ {
if (attr->no_warn)
return;
if (attr->verbose > 0 && __ratelimit(&attr->ratelimit_state)) { if (attr->verbose > 0 && __ratelimit(&attr->ratelimit_state)) {
printk(KERN_NOTICE "FAULT_INJECTION: forcing a failure.\n" printk(KERN_NOTICE "FAULT_INJECTION: forcing a failure.\n"
"name %pd, interval %lu, probability %lu, " "name %pd, interval %lu, probability %lu, "
...@@ -103,7 +100,7 @@ static inline bool fail_stacktrace(struct fault_attr *attr) ...@@ -103,7 +100,7 @@ static inline bool fail_stacktrace(struct fault_attr *attr)
* http://www.nongnu.org/failmalloc/ * http://www.nongnu.org/failmalloc/
*/ */
bool should_fail(struct fault_attr *attr, ssize_t size) bool should_fail_ex(struct fault_attr *attr, ssize_t size, int flags)
{ {
if (in_task()) { if (in_task()) {
unsigned int fail_nth = READ_ONCE(current->fail_nth); unsigned int fail_nth = READ_ONCE(current->fail_nth);
...@@ -146,13 +143,19 @@ bool should_fail(struct fault_attr *attr, ssize_t size) ...@@ -146,13 +143,19 @@ bool should_fail(struct fault_attr *attr, ssize_t size)
return false; return false;
fail: fail:
fail_dump(attr); if (!(flags & FAULT_NOWARN))
fail_dump(attr);
if (atomic_read(&attr->times) != -1) if (atomic_read(&attr->times) != -1)
atomic_dec_not_zero(&attr->times); atomic_dec_not_zero(&attr->times);
return true; return true;
} }
bool should_fail(struct fault_attr *attr, ssize_t size)
{
return should_fail_ex(attr, size, 0);
}
EXPORT_SYMBOL_GPL(should_fail); EXPORT_SYMBOL_GPL(should_fail);
#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS #ifdef CONFIG_FAULT_INJECTION_DEBUG_FS
......
...@@ -16,6 +16,8 @@ static struct { ...@@ -16,6 +16,8 @@ static struct {
bool __should_failslab(struct kmem_cache *s, gfp_t gfpflags) bool __should_failslab(struct kmem_cache *s, gfp_t gfpflags)
{ {
int flags = 0;
/* No fault-injection for bootstrap cache */ /* No fault-injection for bootstrap cache */
if (unlikely(s == kmem_cache)) if (unlikely(s == kmem_cache))
return false; return false;
...@@ -30,10 +32,16 @@ bool __should_failslab(struct kmem_cache *s, gfp_t gfpflags) ...@@ -30,10 +32,16 @@ bool __should_failslab(struct kmem_cache *s, gfp_t gfpflags)
if (failslab.cache_filter && !(s->flags & SLAB_FAILSLAB)) if (failslab.cache_filter && !(s->flags & SLAB_FAILSLAB))
return false; return false;
/*
* In some cases, it expects to specify __GFP_NOWARN
* to avoid printing any information(not just a warning),
* thus avoiding deadlocks. See commit 6b9dbedbe349 for
* details.
*/
if (gfpflags & __GFP_NOWARN) if (gfpflags & __GFP_NOWARN)
failslab.attr.no_warn = true; flags |= FAULT_NOWARN;
return should_fail(&failslab.attr, s->object_size); return should_fail_ex(&failslab.attr, s->object_size, flags);
} }
static int __init setup_failslab(char *str) static int __init setup_failslab(char *str)
......
...@@ -3887,6 +3887,8 @@ __setup("fail_page_alloc=", setup_fail_page_alloc); ...@@ -3887,6 +3887,8 @@ __setup("fail_page_alloc=", setup_fail_page_alloc);
static bool __should_fail_alloc_page(gfp_t gfp_mask, unsigned int order) static bool __should_fail_alloc_page(gfp_t gfp_mask, unsigned int order)
{ {
int flags = 0;
if (order < fail_page_alloc.min_order) if (order < fail_page_alloc.min_order)
return false; return false;
if (gfp_mask & __GFP_NOFAIL) if (gfp_mask & __GFP_NOFAIL)
...@@ -3897,10 +3899,11 @@ static bool __should_fail_alloc_page(gfp_t gfp_mask, unsigned int order) ...@@ -3897,10 +3899,11 @@ static bool __should_fail_alloc_page(gfp_t gfp_mask, unsigned int order)
(gfp_mask & __GFP_DIRECT_RECLAIM)) (gfp_mask & __GFP_DIRECT_RECLAIM))
return false; return false;
/* See comment in __should_failslab() */
if (gfp_mask & __GFP_NOWARN) if (gfp_mask & __GFP_NOWARN)
fail_page_alloc.attr.no_warn = true; flags |= FAULT_NOWARN;
return should_fail(&fail_page_alloc.attr, 1 << order); return should_fail_ex(&fail_page_alloc.attr, 1 << order, flags);
} }
#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS #ifdef CONFIG_FAULT_INJECTION_DEBUG_FS
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment