[Intel-gfx] [PATCH 02/13] kthread: finer-grained lockdep/cross-release completion
Daniel Vetter
daniel.vetter at ffwll.ch
Wed Nov 7 15:30:08 UTC 2018
When cross release was originally merged we've hit a bunch of lockdep
splats around the 2 kthread completions. In all cases they are because
totally independent uses of kthread are mixed up by lockdep into the
same locking class, creating artificial deadlocks.
Fix this by converting kthread code in the same way as e.g.
alloc_workqueue already works: Use macros for the public api so we can
have a callsite specific lockdep key, then pass that through the
entire callchain. Due to the many entry points this is slightly
tedious.
Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
Cc: Marta Lofstedt <marta.lofstedt at intel.com>
Cc: Byungchul Park <byungchul.park at lge.com>
Cc: Ingo Molnar <mingo at kernel.org>
Cc: Peter Zijlstra <peterz at infradead.org>
Cc: Tejun Heo <tj at kernel.org>
Cc: Kees Cook <keescook at chromium.org>
Cc: Thomas Gleixner <tglx at linutronix.de>
Cc: Shaohua Li <shli at fb.com>
Cc: Andrew Morton <akpm at linux-foundation.org>
Cc: Jens Axboe <axboe at kernel.dk>
Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
Cc: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
Cc: Jonathan Corbet <corbet at lwn.net>
Cc: Oleg Nesterov <oleg at redhat.com>
Cc: "Steven Rostedt (VMware)" <rostedt at goodmis.org>
Cc: Snild Dolkow <snild at sony.com>
References: https://bugs.freedesktop.org/show_bug.cgi?id=103950
Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
--
Peter acked the original patch but then cross-release got removed
before this one here landed.
-Daniel
---
include/linux/kthread.h | 48 ++++++++++++++++++++++++-----
kernel/kthread.c | 68 ++++++++++++++++++++++++++++-------------
2 files changed, 88 insertions(+), 28 deletions(-)
diff --git a/include/linux/kthread.h b/include/linux/kthread.h
index c1961761311d..7a9463f0be5c 100644
--- a/include/linux/kthread.h
+++ b/include/linux/kthread.h
@@ -6,10 +6,12 @@
#include <linux/sched.h>
#include <linux/cgroup.h>
-__printf(4, 5)
-struct task_struct *kthread_create_on_node(int (*threadfn)(void *data),
+__printf(6, 7)
+struct task_struct *_kthread_create_on_node(int (*threadfn)(void *data),
void *data,
int node,
+ struct lock_class_key *exited_key,
+ struct lock_class_key *parked_key,
const char namefmt[], ...);
/**
@@ -25,12 +27,27 @@ struct task_struct *kthread_create_on_node(int (*threadfn)(void *data),
*/
#define kthread_create(threadfn, data, namefmt, arg...) \
kthread_create_on_node(threadfn, data, NUMA_NO_NODE, namefmt, ##arg)
+#define kthread_create_on_node(threadfn, data, node, namefmt, arg...) \
+({ \
+ static struct lock_class_key __exited_key, __parked_key; \
+ _kthread_create_on_node(threadfn, data, node, &__exited_key, \
+ &__parked_key, namefmt, ##arg); \
+})
-struct task_struct *kthread_create_on_cpu(int (*threadfn)(void *data),
+struct task_struct *_kthread_create_on_cpu(int (*threadfn)(void *data),
void *data,
unsigned int cpu,
+ struct lock_class_key *exited_key,
+ struct lock_class_key *parked_key,
const char *namefmt);
+#define kthread_create_on_cpu(threadfn, data, cpu, namefmt) \
+({ \
+ static struct lock_class_key __exited_key, __parked_key; \
+ _kthread_create_on_cpu(threadfn, data, cpu, &__exited_key,\
+ &__parked_key, namefmt); \
+})
+
/**
* kthread_run - create and wake a thread.
@@ -171,13 +188,30 @@ extern void __kthread_init_worker(struct kthread_worker *worker,
int kthread_worker_fn(void *worker_ptr);
-__printf(2, 3)
+__printf(4, 5)
struct kthread_worker *
-kthread_create_worker(unsigned int flags, const char namefmt[], ...);
+_kthread_create_worker(unsigned int flags,
+ struct lock_class_key *exited_key,
+ struct lock_class_key *parked_key,
+ const char namefmt[], ...);
+#define kthread_create_worker(flags, namefmt...) \
+({ \
+ static struct lock_class_key __exited_key, __parked_key; \
+ _kthread_create_worker(flags, &__exited_key, &__parked_key, \
+ ##namefmt); \
+})
-__printf(3, 4) struct kthread_worker *
-kthread_create_worker_on_cpu(int cpu, unsigned int flags,
+__printf(5, 6) struct kthread_worker *
+_kthread_create_worker_on_cpu(int cpu, unsigned int flags,
+ struct lock_class_key *exited_key,
+ struct lock_class_key *parked_key,
const char namefmt[], ...);
+#define kthread_create_worker_on_cpu(cpu, flags, namefmt...) \
+({ \
+ static struct lock_class_key __exited_key, __parked_key; \
+ _kthread_create_worker_on_cpu(cpu, flags, &__exited_key, &__parked_key,\
+ ##namefmt); \
+})
bool kthread_queue_work(struct kthread_worker *worker,
struct kthread_work *work);
diff --git a/kernel/kthread.c b/kernel/kthread.c
index 087d18d771b5..64c0464a17dd 100644
--- a/kernel/kthread.c
+++ b/kernel/kthread.c
@@ -32,6 +32,7 @@ struct kthread_create_info
int (*threadfn)(void *data);
void *data;
int node;
+ struct lock_class_key *exited_key, *parked_key;
/* Result passed back to kthread_create() from kthreadd. */
struct task_struct *result;
@@ -229,8 +230,15 @@ static int kthread(void *_create)
}
self->data = data;
- init_completion(&self->exited);
- init_completion(&self->parked);
+ lockdep_init_map_crosslock(&self->exited.map.map,
+ "(kthread completion)->exited",
+ create->exited_key, 0);
+ init_completion_map(&self->exited, &self->exited.map.map);
+ lockdep_init_map_crosslock(&self->parked.map.map,
+ "(kthread completion)->parked",
+ create->parked_key, 0);
+ init_completion_map(&self->parked, &self->exited.map.map);
+
current->vfork_done = &self->exited;
/* OK, tell user we're spawned, wait for stop or wakeup */
@@ -280,9 +288,11 @@ static void create_kthread(struct kthread_create_info *create)
}
}
-static __printf(4, 0)
+static __printf(6, 0)
struct task_struct *__kthread_create_on_node(int (*threadfn)(void *data),
void *data, int node,
+ struct lock_class_key *exited_key,
+ struct lock_class_key *parked_key,
const char namefmt[],
va_list args)
{
@@ -297,6 +307,8 @@ struct task_struct *__kthread_create_on_node(int (*threadfn)(void *data),
create->data = data;
create->node = node;
create->done = &done;
+ create->exited_key = exited_key;
+ create->parked_key = parked_key;
spin_lock(&kthread_create_lock);
list_add_tail(&create->list, &kthread_create_list);
@@ -367,21 +379,24 @@ struct task_struct *__kthread_create_on_node(int (*threadfn)(void *data),
*
* Returns a task_struct or ERR_PTR(-ENOMEM) or ERR_PTR(-EINTR).
*/
-struct task_struct *kthread_create_on_node(int (*threadfn)(void *data),
- void *data, int node,
- const char namefmt[],
- ...)
+struct task_struct *_kthread_create_on_node(int (*threadfn)(void *data),
+ void *data, int node,
+ struct lock_class_key *exited_key,
+ struct lock_class_key *parked_key,
+ const char namefmt[],
+ ...)
{
struct task_struct *task;
va_list args;
va_start(args, namefmt);
- task = __kthread_create_on_node(threadfn, data, node, namefmt, args);
+ task = __kthread_create_on_node(threadfn, data, node,
+ exited_key, parked_key, namefmt, args);
va_end(args);
return task;
}
-EXPORT_SYMBOL(kthread_create_on_node);
+EXPORT_SYMBOL(_kthread_create_on_node);
static void __kthread_bind_mask(struct task_struct *p, const struct cpumask *mask, long state)
{
@@ -435,14 +450,16 @@ EXPORT_SYMBOL(kthread_bind);
* Description: This helper function creates and names a kernel thread
* The thread will be woken and put into park mode.
*/
-struct task_struct *kthread_create_on_cpu(int (*threadfn)(void *data),
+struct task_struct *_kthread_create_on_cpu(int (*threadfn)(void *data),
void *data, unsigned int cpu,
+ struct lock_class_key *exited_key,
+ struct lock_class_key *parked_key,
const char *namefmt)
{
struct task_struct *p;
- p = kthread_create_on_node(threadfn, data, cpu_to_node(cpu), namefmt,
- cpu);
+ p = _kthread_create_on_node(threadfn, data, cpu_to_node(cpu),
+ exited_key, parked_key, namefmt, cpu);
if (IS_ERR(p))
return p;
kthread_bind(p, cpu);
@@ -669,8 +686,10 @@ int kthread_worker_fn(void *worker_ptr)
}
EXPORT_SYMBOL_GPL(kthread_worker_fn);
-static __printf(3, 0) struct kthread_worker *
+static __printf(5, 0) struct kthread_worker *
__kthread_create_worker(int cpu, unsigned int flags,
+ struct lock_class_key *exited_key,
+ struct lock_class_key *parked_key,
const char namefmt[], va_list args)
{
struct kthread_worker *worker;
@@ -686,8 +705,8 @@ __kthread_create_worker(int cpu, unsigned int flags,
if (cpu >= 0)
node = cpu_to_node(cpu);
- task = __kthread_create_on_node(kthread_worker_fn, worker,
- node, namefmt, args);
+ task = __kthread_create_on_node(kthread_worker_fn, worker, node,
+ exited_key, parked_key, namefmt, args);
if (IS_ERR(task))
goto fail_task;
@@ -714,18 +733,22 @@ __kthread_create_worker(int cpu, unsigned int flags,
* when the worker was SIGKILLed.
*/
struct kthread_worker *
-kthread_create_worker(unsigned int flags, const char namefmt[], ...)
+_kthread_create_worker(unsigned int flags,
+ struct lock_class_key *exited_key,
+ struct lock_class_key *parked_key,
+ const char namefmt[], ...)
{
struct kthread_worker *worker;
va_list args;
va_start(args, namefmt);
- worker = __kthread_create_worker(-1, flags, namefmt, args);
+ worker = __kthread_create_worker(-1, flags, exited_key, parked_key,
+ namefmt, args);
va_end(args);
return worker;
}
-EXPORT_SYMBOL(kthread_create_worker);
+EXPORT_SYMBOL(_kthread_create_worker);
/**
* kthread_create_worker_on_cpu - create a kthread worker and bind it
@@ -745,19 +768,22 @@ EXPORT_SYMBOL(kthread_create_worker);
* when the worker was SIGKILLed.
*/
struct kthread_worker *
-kthread_create_worker_on_cpu(int cpu, unsigned int flags,
+_kthread_create_worker_on_cpu(int cpu, unsigned int flags,
+ struct lock_class_key *exited_key,
+ struct lock_class_key *parked_key,
const char namefmt[], ...)
{
struct kthread_worker *worker;
va_list args;
va_start(args, namefmt);
- worker = __kthread_create_worker(cpu, flags, namefmt, args);
+ worker = __kthread_create_worker(cpu, flags, exited_key, parked_key,
+ namefmt, args);
va_end(args);
return worker;
}
-EXPORT_SYMBOL(kthread_create_worker_on_cpu);
+EXPORT_SYMBOL(_kthread_create_worker_on_cpu);
/*
* Returns true when the work could not be queued at the moment.
--
2.19.1
More information about the Intel-gfx
mailing list