[PATCH 25/30] dept: Track timeout waits separately with a new Kconfig
Gwan-gyeong Mun
gwan-gyeong.mun at intel.com
Mon Nov 21 09:54:57 UTC 2022
From: Byungchul Park <byungchul.park at lge.com>
Waits with valid timeouts don't actually cause deadlocks. However, Dept
has been reporting the cases as well because it's worth informing the
circular dependency for some cases where, for example, timeout is used
to avoid a deadlock but not meant to be expired.
However, yes, there are also a lot of, even more, cases where timeout
is used for its clear purpose and meant to be expired.
Let Dept report these as an information rather than shouting DEADLOCK.
Plus, introduced CONFIG_DEPT_AGGRESSIVE_TIMEOUT_WAIT Kconfig to make it
optional so that any reports involving waits with timeouts can be turned
on/off depending on the purpose.
Signed-off-by: Byungchul Park <byungchul.park at lge.com>
Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun at intel.com>
---
include/linux/completion.h | 2 +-
include/linux/dept.h | 19 +++++++----
include/linux/dept_page.h | 4 +--
include/linux/dept_sdt.h | 15 +++++++--
include/linux/mutex.h | 2 +-
include/linux/rwlock.h | 4 +--
include/linux/rwsem.h | 2 +-
include/linux/seqlock.h | 2 +-
include/linux/spinlock.h | 2 +-
kernel/dependency/dept.c | 66 +++++++++++++++++++++++++++++++-------
lib/Kconfig.debug | 10 ++++++
11 files changed, 99 insertions(+), 29 deletions(-)
diff --git a/include/linux/completion.h b/include/linux/completion.h
index 9eaa90539661..72adefb4774f 100644
--- a/include/linux/completion.h
+++ b/include/linux/completion.h
@@ -36,7 +36,7 @@ struct completion {
#define dept_wfc_wait(m, ip) \
do { \
dept_ask_event(m); \
- dept_wait(m, 1UL, ip, __func__, 0, true); \
+ dept_wait(m, 1UL, ip, __func__, 0, true, false); \
} while (0)
#define dept_wfc_complete(m, ip) dept_event(m, 1UL, ip, __func__)
#define dept_wfc_enter(m, ip) dept_ecxt_enter(m, 1UL, ip, "completion_context_enter", "complete", 0)
diff --git a/include/linux/dept.h b/include/linux/dept.h
index dab75180941e..3b99cbb79c39 100644
--- a/include/linux/dept.h
+++ b/include/linux/dept.h
@@ -218,6 +218,11 @@ struct dept_wait {
* spin or sleep
*/
bool sleep;
+
+ /*
+ * whether a timeout is set
+ */
+ bool timeout;
};
struct dept_dep {
@@ -470,6 +475,7 @@ struct dept_task {
unsigned long stage_w_f;
const char *stage_w_fn;
int stage_ne;
+ bool stage_timeout;
/*
* the number of missing ecxts
@@ -508,6 +514,7 @@ struct dept_task {
.stage_w_f = 0UL, \
.stage_w_fn = NULL, \
.stage_ne = 0, \
+ .stage_timeout = false, \
.missing_ecxt = 0, \
.hardirqs_enabled = false, \
.softirqs_enabled = false, \
@@ -526,8 +533,8 @@ extern void dept_map_reinit(struct dept_map *m);
extern void dept_map_nocheck(struct dept_map *m);
extern void dept_map_copy(struct dept_map *to, struct dept_map *from);
-extern void dept_wait(struct dept_map *m, unsigned long w_f, unsigned long ip, const char *w_fn, int ne, bool sleep);
-extern void dept_stage_wait(struct dept_map *m, unsigned long w_f, const char *w_fn, int ne);
+extern void dept_wait(struct dept_map *m, unsigned long w_f, unsigned long ip, const char *w_fn, int ne, bool sleep, bool timeout);
+extern void dept_stage_wait(struct dept_map *m, unsigned long w_f, const char *w_fn, int ne, bool timeout);
extern void dept_ask_event_wait_commit(unsigned long ip);
extern void dept_clean_stage(void);
extern void dept_ecxt_enter(struct dept_map *m, unsigned long e_f, unsigned long ip, const char *c_fn, const char *e_fn, int ne);
@@ -536,7 +543,7 @@ extern void dept_event(struct dept_map *m, unsigned long e_f, unsigned long ip,
extern void dept_ecxt_exit(struct dept_map *m, unsigned long e_f, unsigned long ip);
extern void dept_split_map_each_init(struct dept_map_each *me);
extern void dept_split_map_common_init(struct dept_map_common *mc, struct dept_key *k, const char *n);
-extern void dept_wait_split_map(struct dept_map_each *me, struct dept_map_common *mc, unsigned long ip, const char *w_fn, int ne, bool sleep);
+extern void dept_wait_split_map(struct dept_map_each *me, struct dept_map_common *mc, unsigned long ip, const char *w_fn, int ne, bool sleep, bool timeout);
extern void dept_event_split_map(struct dept_map_each *me, struct dept_map_common *mc, unsigned long ip, const char *e_fn);
extern void dept_ask_event_split_map(struct dept_map_each *me, struct dept_map_common *mc);
extern void dept_kernel_enter(void);
@@ -583,8 +590,8 @@ struct dept_task { };
#define dept_map_nocheck(m) do { } while (0)
#define dept_map_copy(t, f) do { } while (0)
-#define dept_wait(m, w_f, ip, w_fn, ne, s) do { (void)(w_fn); } while (0)
-#define dept_stage_wait(m, w_f, w_fn, ne) do { (void)(w_fn); } while (0)
+#define dept_wait(m, w_f, ip, w_fn, ne, s, t) do { (void)(w_fn); } while (0)
+#define dept_stage_wait(m, w_f, w_fn, ne, t) do { (void)(w_fn); } while (0)
#define dept_ask_event_wait_commit(ip) do { } while (0)
#define dept_clean_stage() do { } while (0)
#define dept_ecxt_enter(m, e_f, ip, c_fn, e_fn, ne) do { (void)(c_fn); (void)(e_fn); } while (0)
@@ -593,7 +600,7 @@ struct dept_task { };
#define dept_ecxt_exit(m, e_f, ip) do { } while (0)
#define dept_split_map_each_init(me) do { } while (0)
#define dept_split_map_common_init(mc, k, n) do { (void)(n); (void)(k); } while (0)
-#define dept_wait_split_map(me, mc, ip, w_fn, ne, s) do { } while (0)
+#define dept_wait_split_map(me, mc, ip, w_fn, ne, s, t) do { } while (0)
#define dept_event_split_map(me, mc, ip, e_fn) do { } while (0)
#define dept_ask_event_split_map(me, mc) do { } while (0)
#define dept_kernel_enter() do { } while (0)
diff --git a/include/linux/dept_page.h b/include/linux/dept_page.h
index 5a2a9899548d..b30f46cf7954 100644
--- a/include/linux/dept_page.h
+++ b/include/linux/dept_page.h
@@ -22,7 +22,7 @@ do { \
struct dept_map_each *me = get_pglocked_me(&(f)->page, &cookie);\
\
if (likely(me)) \
- dept_wait_split_map(me, &pglocked_mc, _RET_IP_, __func__, 0, true);\
+ dept_wait_split_map(me, &pglocked_mc, _RET_IP_, __func__, 0, true, false);\
\
put_pglocked_me(cookie); \
} while (0)
@@ -55,7 +55,7 @@ do { \
struct dept_map_each *me = get_pgwriteback_me(&(f)->page, &cookie);\
\
if (likely(me)) \
- dept_wait_split_map(me, &pgwriteback_mc, _RET_IP_, __func__, 0, true);\
+ dept_wait_split_map(me, &pgwriteback_mc, _RET_IP_, __func__, 0, true, false);\
\
put_pgwriteback_me(cookie); \
} while (0)
diff --git a/include/linux/dept_sdt.h b/include/linux/dept_sdt.h
index 14a17200f137..74c48b0d8311 100644
--- a/include/linux/dept_sdt.h
+++ b/include/linux/dept_sdt.h
@@ -29,20 +29,27 @@
#define sdt_wait(m) \
do { \
dept_ask_event(m); \
- dept_wait(m, 1UL, _THIS_IP_, "wait", 0, true); \
+ dept_wait(m, 1UL, _THIS_IP_, "wait", 0, true, false); \
+ } while (0)
+
+#define sdt_wait_timeout(m) \
+ do { \
+ dept_ask_event(m); \
+ dept_wait(m, 1UL, _THIS_IP_, "wait", 0, true, true); \
} while (0)
#define sdt_wait_spin(m) \
do { \
dept_ask_event(m); \
- dept_wait(m, 1UL, _THIS_IP_, "wait", 0, false); \
+ dept_wait(m, 1UL, _THIS_IP_, "wait", 0, false, false); \
} while (0)
/*
* This will be committed in __schedule() when it actually gets to
* __schedule(). Both dept_ask_event() and dept_wait() will be performed
* on the commit in __schedule().
*/
-#define sdt_wait_prepare(m) dept_stage_wait(m, 1UL, "wait", 0)
+#define sdt_wait_prepare(m) dept_stage_wait(m, 1UL, "wait", 0, false)
+#define sdt_wait_prepare_timeout(m) dept_stage_wait(m, 1UL, "wait", 0, true)
#define sdt_wait_finish() dept_clean_stage()
#define sdt_ecxt_enter(m) dept_ecxt_enter(m, 1UL, _THIS_IP_, "start", "event", 0)
#define sdt_event(m) dept_event(m, 1UL, _THIS_IP_, "event")
@@ -53,8 +60,10 @@
#define sdt_map_init(m) do { } while (0)
#define sdt_map_init_key(m, k) do { (void)(k); } while (0)
#define sdt_wait(m) do { } while (0)
+#define sdt_wait_timeout(m) do { } while (0)
#define sdt_wait_spin(m) do { } while (0)
#define sdt_wait_prepare(m) do { } while (0)
+#define sdt_wait_prepare_timeout(m) do { } while (0)
#define sdt_wait_finish() do { } while (0)
#define sdt_ecxt_enter(m) do { } while (0)
#define sdt_event(m) do { } while (0)
diff --git a/include/linux/mutex.h b/include/linux/mutex.h
index e98a912b3d4c..719d870ae761 100644
--- a/include/linux/mutex.h
+++ b/include/linux/mutex.h
@@ -84,7 +84,7 @@ do { \
} else if (n) { \
dept_ecxt_enter_nokeep(m); \
} else { \
- dept_wait(m, 1UL, ip, __func__, ne, true); \
+ dept_wait(m, 1UL, ip, __func__, ne, true, false); \
dept_ecxt_enter(m, 1UL, ip, __func__, e_fn, ne); \
} \
} while (0)
diff --git a/include/linux/rwlock.h b/include/linux/rwlock.h
index 7f4c6e65f2f4..8c24a33c254a 100644
--- a/include/linux/rwlock.h
+++ b/include/linux/rwlock.h
@@ -40,7 +40,7 @@ do { \
} else if (n) { \
dept_ecxt_enter_nokeep(m); \
} else { \
- dept_wait(m, DEPT_EVT_RWLOCK_RW, ip, __func__, ne, s); \
+ dept_wait(m, DEPT_EVT_RWLOCK_RW, ip, __func__, ne, s, false);\
dept_ecxt_enter(m, DEPT_EVT_RWLOCK_W, ip, __func__, e_fn, ne);\
} \
} while (0)
@@ -51,7 +51,7 @@ do { \
} else if (n) { \
dept_ecxt_enter_nokeep(m); \
} else { \
- dept_wait(m, (q) ? DEPT_EVT_RWLOCK_RW : DEPT_EVT_RWLOCK_W, ip, __func__, ne, s);\
+ dept_wait(m, (q) ? DEPT_EVT_RWLOCK_RW : DEPT_EVT_RWLOCK_W, ip, __func__, ne, s, false);\
dept_ecxt_enter(m, DEPT_EVT_RWLOCK_R, ip, __func__, e_fn, ne);\
} \
} while (0)
diff --git a/include/linux/rwsem.h b/include/linux/rwsem.h
index fd86dfd599da..c49c4ba1a392 100644
--- a/include/linux/rwsem.h
+++ b/include/linux/rwsem.h
@@ -41,7 +41,7 @@ do { \
} else if (n) { \
dept_ecxt_enter_nokeep(m); \
} else { \
- dept_wait(m, 1UL, ip, __func__, ne, true); \
+ dept_wait(m, 1UL, ip, __func__, ne, true, false); \
dept_ecxt_enter(m, 1UL, ip, __func__, e_fn, ne); \
} \
} while (0)
diff --git a/include/linux/seqlock.h b/include/linux/seqlock.h
index 36c242c30f95..038f53fddfb2 100644
--- a/include/linux/seqlock.h
+++ b/include/linux/seqlock.h
@@ -24,7 +24,7 @@
#ifdef CONFIG_DEPT
#define dept_seq_wait(m, ip) \
- dept_wait(m, 1UL, ip, __func__, 0, false)
+ dept_wait(m, 1UL, ip, __func__, 0, false, false)
#define dept_seq_writebegin(m, ip) \
dept_ecxt_enter(m, 1UL, ip, __func__, "write_seqcount_end", 0)
#define dept_seq_writeend(m, ip) \
diff --git a/include/linux/spinlock.h b/include/linux/spinlock.h
index 775a6c076f16..4a1786e7a857 100644
--- a/include/linux/spinlock.h
+++ b/include/linux/spinlock.h
@@ -104,7 +104,7 @@ do { \
} else if (n) { \
dept_ecxt_enter_nokeep(m); \
} else { \
- dept_wait(m, 1UL, ip, __func__, ne, s); \
+ dept_wait(m, 1UL, ip, __func__, ne, s, false); \
dept_ecxt_enter(m, 1UL, ip, __func__, e_fn, ne); \
} \
} while (0)
diff --git a/kernel/dependency/dept.c b/kernel/dependency/dept.c
index 055356d5a6e4..1692c4d8df25 100644
--- a/kernel/dependency/dept.c
+++ b/kernel/dependency/dept.c
@@ -740,6 +740,8 @@ static void print_diagram(struct dept_dep *d)
if (!irqf) {
print_spc(spc, "[S] %s(%s:%d)\n", c_fn, fc->name, fc->sub);
print_spc(spc, "[W] %s(%s:%d)\n", w_fn, tc->name, tc->sub);
+ if (w->timeout)
+ print_spc(spc, "--------------- >8 timeout ---------------\n");
print_spc(spc, "[E] %s(%s:%d)\n", e_fn, fc->name, fc->sub);
}
}
@@ -791,6 +793,24 @@ static void print_dep(struct dept_dep *d)
static void save_current_stack(int skip);
+static bool is_timeout_wait_circle(struct dept_class *c)
+{
+ struct dept_class *fc = c->bfs_parent;
+ struct dept_class *tc = c;
+
+ do {
+ struct dept_dep *d = lookup_dep(fc, tc);
+
+ if (d->wait->timeout)
+ return true;
+
+ tc = fc;
+ fc = fc->bfs_parent;
+ } while (tc != c);
+
+ return false;
+}
+
/*
* Print all classes in a circle.
*/
@@ -813,10 +833,14 @@ static void print_circle(struct dept_class *c)
pr_warn("summary\n");
pr_warn("---------------------------------------------------\n");
- if (fc == tc)
+ if (is_timeout_wait_circle(c)) {
+ pr_warn("NOT A DEADLOCK BUT A CIRCULAR DEPENDENCY\n");
+ pr_warn("CHECK IF THE TIMEOUT IS INTENDED\n\n");
+ } else if (fc == tc) {
pr_warn("*** AA DEADLOCK ***\n\n");
- else
+ } else {
pr_warn("*** DEADLOCK ***\n\n");
+ }
i = 0;
do {
@@ -1560,7 +1584,7 @@ static void add_dep(struct dept_ecxt *e, struct dept_wait *w)
static atomic_t wgen = ATOMIC_INIT(1);
static void add_wait(struct dept_class *c, unsigned long ip,
- const char *w_fn, int ne, bool sleep)
+ const char *w_fn, int ne, bool sleep, bool timeout)
{
struct dept_task *dt = dept_task();
struct dept_wait *w;
@@ -1577,6 +1601,7 @@ static void add_wait(struct dept_class *c, unsigned long ip,
w->wait_fn = w_fn;
w->wait_stack = get_current_stack();
w->sleep = sleep;
+ w->timeout = timeout;
cxt = cur_cxt();
if (cxt == DEPT_CXT_HIRQ || cxt == DEPT_CXT_SIRQ)
@@ -2243,7 +2268,7 @@ static struct dept_class *check_new_class(struct dept_key *local,
static void __dept_wait(struct dept_map *m, unsigned long w_f,
unsigned long ip, const char *w_fn, int ne,
- bool sleep)
+ bool sleep, bool timeout)
{
int e;
@@ -2266,7 +2291,7 @@ static void __dept_wait(struct dept_map *m, unsigned long w_f,
if (!c)
continue;
- add_wait(c, ip, w_fn, ne, sleep);
+ add_wait(c, ip, w_fn, ne, sleep, timeout);
}
}
@@ -2286,7 +2311,7 @@ static inline struct dept_map *staged_map(struct dept_task *dt)
}
void dept_wait(struct dept_map *m, unsigned long w_f, unsigned long ip,
- const char *w_fn, int ne, bool sleep)
+ const char *w_fn, int ne, bool sleep, bool timeout)
{
struct dept_task *dt = dept_task();
unsigned long flags;
@@ -2294,6 +2319,11 @@ void dept_wait(struct dept_map *m, unsigned long w_f, unsigned long ip,
if (unlikely(!dept_working()))
return;
+#if !defined(CONFIG_DEPT_AGGRESSIVE_TIMEOUT_WAIT)
+ if (timeout)
+ return;
+#endif
+
if (dt->recursive)
return;
@@ -2314,14 +2344,14 @@ void dept_wait(struct dept_map *m, unsigned long w_f, unsigned long ip,
if (sleep)
unstage_map(dt);
- __dept_wait(m, w_f, ip, w_fn, ne, sleep);
+ __dept_wait(m, w_f, ip, w_fn, ne, sleep, timeout);
dept_exit(flags);
}
EXPORT_SYMBOL_GPL(dept_wait);
void dept_stage_wait(struct dept_map *m, unsigned long w_f,
- const char *w_fn, int ne)
+ const char *w_fn, int ne, bool timeout)
{
struct dept_task *dt = dept_task();
unsigned long flags;
@@ -2329,6 +2359,11 @@ void dept_stage_wait(struct dept_map *m, unsigned long w_f,
if (unlikely(!dept_working()))
return;
+#if !defined(CONFIG_DEPT_AGGRESSIVE_TIMEOUT_WAIT)
+ if (timeout)
+ return;
+#endif
+
if (m->nocheck)
return;
@@ -2342,6 +2377,7 @@ void dept_stage_wait(struct dept_map *m, unsigned long w_f,
dt->stage_w_f = w_f;
dt->stage_w_fn = w_fn;
dt->stage_ne = ne;
+ dt->stage_timeout = timeout;
/*
* Disable the map just in case real sleep won't happen. This
@@ -2371,6 +2407,7 @@ void dept_clean_stage(void)
dt->stage_w_f = 0UL;
dt->stage_w_fn = NULL;
dt->stage_ne = 0;
+ dt->stage_timeout = false;
dept_exit_recursive(flags);
}
@@ -2388,6 +2425,7 @@ void dept_ask_event_wait_commit(unsigned long ip)
unsigned long w_f;
const char *w_fn;
int ne;
+ bool timeout;
if (unlikely(!dept_working()))
return;
@@ -2425,6 +2463,7 @@ void dept_ask_event_wait_commit(unsigned long ip)
w_f = dt->stage_w_f;
w_fn = dt->stage_w_fn;
ne = dt->stage_ne;
+ timeout = dt->stage_timeout;
/*
* Avoid zero wgen.
@@ -2432,7 +2471,7 @@ void dept_ask_event_wait_commit(unsigned long ip)
wg = atomic_inc_return(&wgen) ?: atomic_inc_return(&wgen);
WRITE_ONCE(m->wgen, wg);
- __dept_wait(m, w_f, ip, w_fn, ne, true);
+ __dept_wait(m, w_f, ip, w_fn, ne, true, timeout);
exit:
dept_exit(flags);
}
@@ -2665,7 +2704,7 @@ EXPORT_SYMBOL_GPL(dept_split_map_common_init);
void dept_wait_split_map(struct dept_map_each *me,
struct dept_map_common *mc,
unsigned long ip, const char *w_fn, int ne,
- bool sleep)
+ bool sleep, bool timeout)
{
struct dept_task *dt = dept_task();
struct dept_class *c;
@@ -2675,6 +2714,11 @@ void dept_wait_split_map(struct dept_map_each *me,
if (unlikely(!dept_working()))
return;
+#if !defined(CONFIG_DEPT_AGGRESSIVE_TIMEOUT_WAIT)
+ if (timeout)
+ return;
+#endif
+
if (dt->recursive)
return;
@@ -2699,7 +2743,7 @@ void dept_wait_split_map(struct dept_map_each *me,
k = mc->keys ?: &mc->keys_local;
c = check_new_class(&mc->keys_local, k, 0, 0UL, mc->name);
if (c)
- add_wait(c, ip, w_fn, ne, sleep);
+ add_wait(c, ip, w_fn, ne, sleep, timeout);
dept_exit(flags);
}
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 75de2a04da1b..eaa2077d1625 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1255,6 +1255,16 @@ config DEPT
noting, to mitigate the impact by the false positives, multi
reporting has been supported.
+config DEPT_AGGRESSIVE_TIMEOUT_WAIT
+ bool "Aggressively track even timeout waits"
+ depends on DEPT
+ default n
+ help
+ Timeout wait doesn't contribute to a deadlock. However,
+ informing a circular dependency might be helpful for cases
+ that timeout is used to avoid a deadlock. Say N if you'd like
+ to avoid verbose reports.
+
config LOCK_DEBUGGING_SUPPORT
bool
depends on TRACE_IRQFLAGS_SUPPORT && STACKTRACE_SUPPORT && LOCKDEP_SUPPORT
--
2.37.1
More information about the Intel-gfx-trybot
mailing list