2022-03-04 07:31:54

by Byungchul Park

[permalink] [raw]
Subject: [PATCH v4 00/24] DEPT(Dependency Tracker)

Hi Linus and folks,

I've been developing a tool for detecting deadlock possibilities by
tracking wait/event rather than lock(?) acquisition order to try to
cover all synchonization machanisms. It's done on v5.17-rc1 tag.

https://github.com/lgebyungchulpark/linux-dept/commits/dept1.14_on_v5.17-rc1

Benifit:

0. Works with all lock primitives.
1. Works with wait_for_completion()/complete().
2. Works with 'wait' on PG_locked.
3. Works with 'wait' on PG_writeback.
4. Works with swait/wakeup.
5. Works with waitqueue.
6. Multiple reports are allowed.
7. Deduplication control on multiple reports.
8. Withstand false positives thanks to 6.
9. Easy to tag any wait/event.

Future work:

0. To make it more stable.
1. To separates Dept from Lockdep.
2. To improves performance in terms of time and space.
3. To use Dept as a dependency engine for Lockdep.
4. To add any missing tags of wait/event in the kernel.
5. To deduplicate stack trace.

How to interpret reports:

1. E(event) in each context cannot be triggered because of the
W(wait) that cannot be woken.
2. The stack trace helping find the problematic code is located
in each conext's detail.

Thanks,
Byungchul

---

Changes from v3:

1. Dept shouldn't create dependencies between different depths
of a class that were indicated by *_lock_nested(). Dept
normally doesn't but it does once another lock class comes
in. So fixed it. (feedback from Hyeonggon)
2. Dept considered a wait as a real wait once getting to
__schedule() even if it has been set to TASK_RUNNING by wake
up sources in advance. Fixed it so that Dept doesn't consider
the case as a real wait. (feedback from Jan Kara)
3. Stop tracking dependencies with a map once the event
associated with the map has been handled. Dept will start to
work with the map again, on the next sleep.

Changes from v2:

1. Disable Dept on bit_wait_table[] in sched/wait_bit.c
reporting a lot of false positives, which is my fault.
Wait/event for bit_wait_table[] should've been tagged in a
higher layer for better work, which is a future work.
(feedback from Jan Kara)
2. Disable Dept on crypto_larval's completion to prevent a false
positive.

Changes from v1:

1. Fix coding style and typo. (feedback from Steven)
2. Distinguish each work context from another in workqueue.
3. Skip checking lock acquisition with nest_lock, which is about
correct lock usage that should be checked by Lockdep.

Changes from RFC:

1. Prevent adding a wait tag at prepare_to_wait() but __schedule().
(feedback from Linus and Matthew)
2. Use try version at lockdep_acquire_cpus_lock() annotation.
3. Distinguish each syscall context from another.

Byungchul Park (24):
llist: Move llist_{head,node} definition to types.h
dept: Implement Dept(Dependency Tracker)
dept: Embed Dept data in Lockdep
dept: Add a API for skipping dependency check temporarily
dept: Apply Dept to spinlock
dept: Apply Dept to mutex families
dept: Apply Dept to rwlock
dept: Apply Dept to wait_for_completion()/complete()
dept: Apply Dept to seqlock
dept: Apply Dept to rwsem
dept: Add proc knobs to show stats and dependency graph
dept: Introduce split map concept and new APIs for them
dept: Apply Dept to wait/event of PG_{locked,writeback}
dept: Apply SDT to swait
dept: Apply SDT to wait(waitqueue)
locking/lockdep, cpu/hotplus: Use a weaker annotation in AP thread
dept: Distinguish each syscall context from another
dept: Distinguish each work from another
dept: Disable Dept within the wait_bit layer by default
dept: Add nocheck version of init_completion()
dept: Disable Dept on struct crypto_larval's completion for now
dept: Don't create dependencies between different depths in any case
dept: Let it work with real sleeps in __schedule()
dept: Disable Dept on that map once it's been handled until next turn

crypto/api.c | 7 +-
include/linux/completion.h | 50 +-
include/linux/dept.h | 535 +++++++
include/linux/dept_page.h | 78 ++
include/linux/dept_sdt.h | 62 +
include/linux/hardirq.h | 3 +
include/linux/irqflags.h | 33 +-
include/linux/llist.h | 8 -
include/linux/lockdep.h | 158 ++-
include/linux/lockdep_types.h | 3 +
include/linux/mutex.h | 33 +
include/linux/page-flags.h | 45 +-
include/linux/pagemap.h | 7 +-
include/linux/percpu-rwsem.h | 10 +-
include/linux/rtmutex.h | 7 +
include/linux/rwlock.h | 52 +
include/linux/rwlock_api_smp.h | 8 +-
include/linux/rwlock_types.h | 7 +
include/linux/rwsem.h | 33 +
include/linux/sched.h | 7 +
include/linux/seqlock.h | 59 +-
include/linux/spinlock.h | 26 +
include/linux/spinlock_types_raw.h | 13 +
include/linux/swait.h | 4 +
include/linux/types.h | 8 +
include/linux/wait.h | 6 +-
init/init_task.c | 2 +
init/main.c | 4 +
kernel/Makefile | 1 +
kernel/cpu.c | 2 +-
kernel/dependency/Makefile | 4 +
kernel/dependency/dept.c | 2716 ++++++++++++++++++++++++++++++++++++
kernel/dependency/dept_hash.h | 10 +
kernel/dependency/dept_internal.h | 26 +
kernel/dependency/dept_object.h | 13 +
kernel/dependency/dept_proc.c | 92 ++
kernel/entry/common.c | 3 +
kernel/exit.c | 1 +
kernel/fork.c | 2 +
kernel/locking/lockdep.c | 12 +-
kernel/module.c | 2 +
kernel/sched/completion.c | 12 +-
kernel/sched/core.c | 8 +
kernel/sched/swait.c | 10 +
kernel/sched/wait.c | 16 +
kernel/sched/wait_bit.c | 5 +-
kernel/softirq.c | 6 +-
kernel/trace/trace_preemptirq.c | 19 +-
kernel/workqueue.c | 3 +
lib/Kconfig.debug | 21 +
mm/filemap.c | 68 +
mm/page_ext.c | 5 +
52 files changed, 4266 insertions(+), 59 deletions(-)
create mode 100644 include/linux/dept.h
create mode 100644 include/linux/dept_page.h
create mode 100644 include/linux/dept_sdt.h
create mode 100644 kernel/dependency/Makefile
create mode 100644 kernel/dependency/dept.c
create mode 100644 kernel/dependency/dept_hash.h
create mode 100644 kernel/dependency/dept_internal.h
create mode 100644 kernel/dependency/dept_object.h
create mode 100644 kernel/dependency/dept_proc.c

--
1.9.1


2022-03-04 07:35:05

by Byungchul Park

[permalink] [raw]
Subject: [PATCH v4 10/24] dept: Apply Dept to rwsem

Makes Dept able to track dependencies by rwsem.

Signed-off-by: Byungchul Park <[email protected]>
---
include/linux/lockdep.h | 24 ++++++++++++++++++++----
include/linux/percpu-rwsem.h | 10 +++++++++-
include/linux/rwsem.h | 33 +++++++++++++++++++++++++++++++++
3 files changed, 62 insertions(+), 5 deletions(-)

diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index b93a707..37af50c 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -646,10 +646,26 @@ static inline void print_irqtrace_events(struct task_struct *curr)
dept_mutex_unlock(&(l)->dmap, i); \
} while (0)

-#define rwsem_acquire(l, s, t, i) lock_acquire_exclusive(l, s, t, NULL, i)
-#define rwsem_acquire_nest(l, s, t, n, i) lock_acquire_exclusive(l, s, t, n, i)
-#define rwsem_acquire_read(l, s, t, i) lock_acquire_shared(l, s, t, NULL, i)
-#define rwsem_release(l, i) lock_release(l, i)
+#define rwsem_acquire(l, s, t, i) \
+do { \
+ lock_acquire_exclusive(l, s, t, NULL, i); \
+ dept_rwsem_lock(&(l)->dmap, s, t, NULL, "up_write", i); \
+} while (0)
+#define rwsem_acquire_nest(l, s, t, n, i) \
+do { \
+ lock_acquire_exclusive(l, s, t, n, i); \
+ dept_rwsem_lock(&(l)->dmap, s, t, (n) ? &(n)->dmap : NULL, "up_write", i);\
+} while (0)
+#define rwsem_acquire_read(l, s, t, i) \
+do { \
+ lock_acquire_shared(l, s, t, NULL, i); \
+ dept_rwsem_lock(&(l)->dmap, s, t, NULL, "up_read", i); \
+} while (0)
+#define rwsem_release(l, i) \
+do { \
+ lock_release(l, i); \
+ dept_rwsem_unlock(&(l)->dmap, i); \
+} while (0)

#define lock_map_acquire(l) lock_acquire_exclusive(l, 0, 0, NULL, _THIS_IP_)
#define lock_map_acquire_read(l) lock_acquire_shared_recursive(l, 0, 0, NULL, _THIS_IP_)
diff --git a/include/linux/percpu-rwsem.h b/include/linux/percpu-rwsem.h
index 5fda40f..ac2b1a5 100644
--- a/include/linux/percpu-rwsem.h
+++ b/include/linux/percpu-rwsem.h
@@ -20,8 +20,16 @@ struct percpu_rw_semaphore {
#endif
};

+#ifdef CONFIG_DEPT
+#define __PERCPU_RWSEM_DMAP_INIT(lockname) .dmap = { .name = #lockname, .skip_cnt = ATOMIC_INIT(0) }
+#else
+#define __PERCPU_RWSEM_DMAP_INIT(lockname)
+#endif
+
#ifdef CONFIG_DEBUG_LOCK_ALLOC
-#define __PERCPU_RWSEM_DEP_MAP_INIT(lockname) .dep_map = { .name = #lockname },
+#define __PERCPU_RWSEM_DEP_MAP_INIT(lockname) .dep_map = { \
+ .name = #lockname, \
+ __PERCPU_RWSEM_DMAP_INIT(lockname) },
#else
#define __PERCPU_RWSEM_DEP_MAP_INIT(lockname)
#endif
diff --git a/include/linux/rwsem.h b/include/linux/rwsem.h
index f934876..dc7977a 100644
--- a/include/linux/rwsem.h
+++ b/include/linux/rwsem.h
@@ -16,11 +16,18 @@
#include <linux/atomic.h>
#include <linux/err.h>

+#ifdef CONFIG_DEPT
+# define RWSEM_DMAP_INIT(lockname) .dmap = { .name = #lockname, .skip_cnt = ATOMIC_INIT(0) },
+#else
+# define RWSEM_DMAP_INIT(lockname)
+#endif
+
#ifdef CONFIG_DEBUG_LOCK_ALLOC
# define __RWSEM_DEP_MAP_INIT(lockname) \
.dep_map = { \
.name = #lockname, \
.wait_type_inner = LD_WAIT_SLEEP, \
+ RWSEM_DMAP_INIT(lockname) \
},
#else
# define __RWSEM_DEP_MAP_INIT(lockname)
@@ -32,6 +39,32 @@
#include <linux/osq_lock.h>
#endif

+#ifdef CONFIG_DEPT
+#define dept_rwsem_lock(m, ne, t, n, e_fn, ip) \
+do { \
+ if (t) { \
+ dept_ecxt_enter(m, 1UL, ip, __func__, e_fn, ne); \
+ dept_ask_event(m); \
+ } else if (n) { \
+ dept_skip(m); \
+ } else { \
+ dept_wait(m, 1UL, ip, __func__, ne); \
+ dept_ecxt_enter(m, 1UL, ip, __func__, e_fn, ne); \
+ dept_ask_event(m); \
+ } \
+} while (0)
+#define dept_rwsem_unlock(m, ip) \
+do { \
+ if (!dept_unskip_if_skipped(m)) { \
+ dept_event(m, 1UL, ip, __func__); \
+ dept_ecxt_exit(m, ip); \
+ } \
+} while (0)
+#else
+#define dept_rwsem_lock(m, ne, t, n, e_fn, ip) do { } while (0)
+#define dept_rwsem_unlock(m, ip) do { } while (0)
+#endif
+
/*
* For an uncontended rwsem, count and owner are the only fields a task
* needs to touch when acquiring the rwsem. So they are put next to each
--
1.9.1

2022-03-04 07:51:07

by Byungchul Park

[permalink] [raw]
Subject: [PATCH v4 17/24] dept: Distinguish each syscall context from another

It enters kernel mode on each syscall and each syscall handling should
be considered independently from the point of view of Dept. Otherwise,
Dept may wrongly track dependencies across different syscalls.

That might be a real dependency from user mode. However, now that Dept
just started to work, conservatively let Dept not track dependencies
across different syscalls.

Signed-off-by: Byungchul Park <[email protected]>
---
include/linux/dept.h | 39 ++++++++++++++++------------
kernel/dependency/dept.c | 67 ++++++++++++++++++++++++------------------------
kernel/entry/common.c | 3 +++
3 files changed, 60 insertions(+), 49 deletions(-)

diff --git a/include/linux/dept.h b/include/linux/dept.h
index e2d4aea..1a1c307 100644
--- a/include/linux/dept.h
+++ b/include/linux/dept.h
@@ -25,11 +25,16 @@
#define DEPT_MAX_SUBCLASSES_USR (DEPT_MAX_SUBCLASSES / DEPT_MAX_SUBCLASSES_EVT)
#define DEPT_MAX_SUBCLASSES_CACHE 2

-#define DEPT_SIRQ 0
-#define DEPT_HIRQ 1
-#define DEPT_IRQS_NR 2
-#define DEPT_SIRQF (1UL << DEPT_SIRQ)
-#define DEPT_HIRQF (1UL << DEPT_HIRQ)
+enum {
+ DEPT_CXT_SIRQ = 0,
+ DEPT_CXT_HIRQ,
+ DEPT_CXT_IRQS_NR,
+ DEPT_CXT_PROCESS = DEPT_CXT_IRQS_NR,
+ DEPT_CXTS_NR
+};
+
+#define DEPT_SIRQF (1UL << DEPT_CXT_SIRQ)
+#define DEPT_HIRQF (1UL << DEPT_CXT_HIRQ)

struct dept_ecxt;
struct dept_iecxt {
@@ -95,8 +100,8 @@ struct dept_class {
/*
* for tracking IRQ dependencies
*/
- struct dept_iecxt iecxt[DEPT_IRQS_NR];
- struct dept_iwait iwait[DEPT_IRQS_NR];
+ struct dept_iecxt iecxt[DEPT_CXT_IRQS_NR];
+ struct dept_iwait iwait[DEPT_CXT_IRQS_NR];
};

struct dept_stack {
@@ -150,8 +155,8 @@ struct dept_ecxt {
/*
* where the IRQ-enabled happened
*/
- unsigned long enirq_ip[DEPT_IRQS_NR];
- struct dept_stack *enirq_stack[DEPT_IRQS_NR];
+ unsigned long enirq_ip[DEPT_CXT_IRQS_NR];
+ struct dept_stack *enirq_stack[DEPT_CXT_IRQS_NR];

/*
* where the event context started
@@ -194,8 +199,8 @@ struct dept_wait {
/*
* where the IRQ wait happened
*/
- unsigned long irq_ip[DEPT_IRQS_NR];
- struct dept_stack *irq_stack[DEPT_IRQS_NR];
+ unsigned long irq_ip[DEPT_CXT_IRQS_NR];
+ struct dept_stack *irq_stack[DEPT_CXT_IRQS_NR];

/*
* where the wait happened
@@ -400,19 +405,19 @@ struct dept_task {
int wait_hist_pos;

/*
- * sequential id to identify each IRQ context
+ * sequential id to identify each context
*/
- unsigned int irq_id[DEPT_IRQS_NR];
+ unsigned int cxt_id[DEPT_CXTS_NR];

/*
* for tracking IRQ-enabled points with cross-event
*/
- unsigned int wgen_enirq[DEPT_IRQS_NR];
+ unsigned int wgen_enirq[DEPT_CXT_IRQS_NR];

/*
* for keeping up-to-date IRQ-enabled points
*/
- unsigned long enirq_ip[DEPT_IRQS_NR];
+ unsigned long enirq_ip[DEPT_CXT_IRQS_NR];

/*
* current effective IRQ-enabled flag
@@ -448,7 +453,7 @@ struct dept_task {
.dept_task.wait_hist = { { .wait = NULL, } }, \
.dept_task.ecxt_held_pos = 0, \
.dept_task.wait_hist_pos = 0, \
- .dept_task.irq_id = { 0 }, \
+ .dept_task.cxt_id = { 0 }, \
.dept_task.wgen_enirq = { 0 }, \
.dept_task.enirq_ip = { 0 }, \
.dept_task.recursive = 0, \
@@ -480,6 +485,7 @@ struct dept_task {
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);
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);

/*
* for users who want to manage external keys
@@ -520,6 +526,7 @@ struct dept_task {
#define dept_wait_split_map(me, mc, ip, w_fn, ne) 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)
#define dept_key_init(k) do { (void)(k); } while (0)
#define dept_key_destroy(k) do { (void)(k); } while (0)
#endif
diff --git a/kernel/dependency/dept.c b/kernel/dependency/dept.c
index 6a47149..8f962ae 100644
--- a/kernel/dependency/dept.c
+++ b/kernel/dependency/dept.c
@@ -212,9 +212,9 @@ static inline struct dept_class *dep_tc(struct dept_dep *d)

static inline const char *irq_str(int irq)
{
- if (irq == DEPT_SIRQ)
+ if (irq == DEPT_CXT_SIRQ)
return "softirq";
- if (irq == DEPT_HIRQ)
+ if (irq == DEPT_CXT_HIRQ)
return "hardirq";
return "(unknown)";
}
@@ -374,7 +374,7 @@ static void initialize_class(struct dept_class *c)
{
int i;

- for (i = 0; i < DEPT_IRQS_NR; i++) {
+ for (i = 0; i < DEPT_CXT_IRQS_NR; i++) {
struct dept_iecxt *ie = &c->iecxt[i];
struct dept_iwait *iw = &c->iwait[i];

@@ -399,7 +399,7 @@ static void initialize_ecxt(struct dept_ecxt *e)
{
int i;

- for (i = 0; i < DEPT_IRQS_NR; i++) {
+ for (i = 0; i < DEPT_CXT_IRQS_NR; i++) {
e->enirq_stack[i] = NULL;
e->enirq_ip[i] = 0UL;
}
@@ -414,7 +414,7 @@ static void initialize_wait(struct dept_wait *w)
{
int i;

- for (i = 0; i < DEPT_IRQS_NR; i++) {
+ for (i = 0; i < DEPT_CXT_IRQS_NR; i++) {
w->irq_stack[i] = NULL;
w->irq_ip[i] = 0UL;
}
@@ -453,7 +453,7 @@ static void destroy_ecxt(struct dept_ecxt *e)
{
int i;

- for (i = 0; i < DEPT_IRQS_NR; i++)
+ for (i = 0; i < DEPT_CXT_IRQS_NR; i++)
if (e->enirq_stack[i])
put_stack(e->enirq_stack[i]);
if (e->class)
@@ -469,7 +469,7 @@ static void destroy_wait(struct dept_wait *w)
{
int i;

- for (i = 0; i < DEPT_IRQS_NR; i++)
+ for (i = 0; i < DEPT_CXT_IRQS_NR; i++)
if (w->irq_stack[i])
put_stack(w->irq_stack[i]);
if (w->class)
@@ -614,7 +614,7 @@ static void print_diagram(struct dept_dep *d)
const char *c_fn = e->ecxt_fn ?: "(unknown)";

irqf = e->enirqf & w->irqf;
- for_each_set_bit(irq, &irqf, DEPT_IRQS_NR) {
+ for_each_set_bit(irq, &irqf, DEPT_CXT_IRQS_NR) {
if (!firstline)
pr_warn("\nor\n\n");
firstline = false;
@@ -645,7 +645,7 @@ static void print_dep(struct dept_dep *d)
const char *c_fn = e->ecxt_fn ?: "(unknown)";

irqf = e->enirqf & w->irqf;
- for_each_set_bit(irq, &irqf, DEPT_IRQS_NR) {
+ for_each_set_bit(irq, &irqf, DEPT_CXT_IRQS_NR) {
pr_warn("%s has been enabled:\n", irq_str(irq));
print_ip_stack(e->enirq_ip[irq], e->enirq_stack[irq]);
pr_warn("\n");
@@ -871,7 +871,7 @@ static void bfs(struct dept_class *c, bfs_f *cb, void *in, void **out)
*/

static inline unsigned long cur_enirqf(void);
-static inline int cur_irq(void);
+static inline int cur_cxt(void);
static inline unsigned int cur_ctxt_id(void);

static inline struct dept_iecxt *iecxt(struct dept_class *c, int irq)
@@ -1397,7 +1397,7 @@ static void add_dep(struct dept_ecxt *e, struct dept_wait *w)
if (d) {
check_dl_bfs(d);

- for (i = 0; i < DEPT_IRQS_NR; i++) {
+ for (i = 0; i < DEPT_CXT_IRQS_NR; i++) {
struct dept_iwait *fiw = iwait(fc, i);
struct dept_iecxt *found_ie;
struct dept_iwait *found_iw;
@@ -1433,7 +1433,7 @@ static void add_wait(struct dept_class *c, unsigned long ip,
struct dept_task *dt = dept_task();
struct dept_wait *w;
unsigned int wg = 0U;
- int irq;
+ int cxt;
int i;

w = new_wait();
@@ -1445,9 +1445,9 @@ static void add_wait(struct dept_class *c, unsigned long ip,
w->wait_fn = w_fn;
w->wait_stack = get_current_stack();

- irq = cur_irq();
- if (irq < DEPT_IRQS_NR)
- add_iwait(c, irq, w);
+ cxt = cur_cxt();
+ if (cxt == DEPT_CXT_HIRQ || cxt == DEPT_CXT_SIRQ)
+ add_iwait(c, cxt, w);

/*
* Avoid adding dependency between user aware nested ecxt and
@@ -1512,7 +1512,7 @@ static void add_ecxt(void *obj, struct dept_class *c, unsigned long ip,
eh->nest = ne;

irqf = cur_enirqf();
- for_each_set_bit(irq, &irqf, DEPT_IRQS_NR)
+ for_each_set_bit(irq, &irqf, DEPT_CXT_IRQS_NR)
add_iecxt(c, irq, e, false);

del_ecxt(e);
@@ -1639,7 +1639,7 @@ static void do_event(void *obj, struct dept_class *c, unsigned int wg,
break;
}

- for (i = 0; i < DEPT_IRQS_NR; i++) {
+ for (i = 0; i < DEPT_CXT_IRQS_NR; i++) {
struct dept_ecxt *e;

if (before(dt->wgen_enirq[i], wg))
@@ -1681,7 +1681,7 @@ static void disconnect_class(struct dept_class *c)
call_rcu(&d->rh, del_dep_rcu);
}

- for (i = 0; i < DEPT_IRQS_NR; i++) {
+ for (i = 0; i < DEPT_CXT_IRQS_NR; i++) {
stale_iecxt(iecxt(c, i));
stale_iwait(iwait(c, i));
}
@@ -1706,27 +1706,21 @@ static inline unsigned long cur_enirqf(void)
return 0UL;
}

-static inline int cur_irq(void)
+static inline int cur_cxt(void)
{
if (lockdep_softirq_context(current))
- return DEPT_SIRQ;
+ return DEPT_CXT_SIRQ;
if (lockdep_hardirq_context())
- return DEPT_HIRQ;
- return DEPT_IRQS_NR;
+ return DEPT_CXT_HIRQ;
+ return DEPT_CXT_PROCESS;
}

static inline unsigned int cur_ctxt_id(void)
{
struct dept_task *dt = dept_task();
- int irq = cur_irq();
+ int cxt = cur_cxt();

- /*
- * Normal process context
- */
- if (irq == DEPT_IRQS_NR)
- return 0U;
-
- return dt->irq_id[irq] | (1UL << irq);
+ return dt->cxt_id[cxt] | (1UL << cxt);
}

static void enirq_transition(int irq)
@@ -1776,7 +1770,7 @@ static void enirq_update(unsigned long ip)
/*
* Do enirq_transition() only on an OFF -> ON transition.
*/
- for_each_set_bit(irq, &irqf, DEPT_IRQS_NR) {
+ for_each_set_bit(irq, &irqf, DEPT_CXT_IRQS_NR) {
if (prev & (1UL << irq))
continue;

@@ -1879,6 +1873,13 @@ void dept_disable_hardirq(unsigned long ip)
dept_exit(flags);
}

+void dept_kernel_enter(void)
+{
+ struct dept_task *dt = dept_task();
+
+ dt->cxt_id[DEPT_CXT_PROCESS] += (1UL << DEPT_CXTS_NR);
+}
+
/*
* Ensure it's the outmost softirq context.
*/
@@ -1886,7 +1887,7 @@ void dept_softirq_enter(void)
{
struct dept_task *dt = dept_task();

- dt->irq_id[DEPT_SIRQ] += (1UL << DEPT_IRQS_NR);
+ dt->cxt_id[DEPT_CXT_SIRQ] += (1UL << DEPT_CXTS_NR);
}

/*
@@ -1896,7 +1897,7 @@ void dept_hardirq_enter(void)
{
struct dept_task *dt = dept_task();

- dt->irq_id[DEPT_HIRQ] += (1UL << DEPT_IRQS_NR);
+ dt->cxt_id[DEPT_CXT_HIRQ] += (1UL << DEPT_CXTS_NR);
}

/*
diff --git a/kernel/entry/common.c b/kernel/entry/common.c
index bad7136..1826508 100644
--- a/kernel/entry/common.c
+++ b/kernel/entry/common.c
@@ -6,6 +6,7 @@
#include <linux/livepatch.h>
#include <linux/audit.h>
#include <linux/tick.h>
+#include <linux/dept.h>

#include "common.h"

@@ -102,6 +103,7 @@ noinstr long syscall_enter_from_user_mode(struct pt_regs *regs, long syscall)
long ret;

__enter_from_user_mode(regs);
+ dept_kernel_enter();

instrumentation_begin();
local_irq_enable();
@@ -114,6 +116,7 @@ noinstr long syscall_enter_from_user_mode(struct pt_regs *regs, long syscall)
noinstr void syscall_enter_from_user_mode_prepare(struct pt_regs *regs)
{
__enter_from_user_mode(regs);
+ dept_kernel_enter();
instrumentation_begin();
local_irq_enable();
instrumentation_end();
--
1.9.1

2022-03-04 08:08:16

by Byungchul Park

[permalink] [raw]
Subject: [PATCH v4 07/24] dept: Apply Dept to rwlock

Makes Dept able to track dependencies by rwlock.

Signed-off-by: Byungchul Park <[email protected]>
---
include/linux/lockdep.h | 25 ++++++++++++++++----
include/linux/rwlock.h | 52 ++++++++++++++++++++++++++++++++++++++++++
include/linux/rwlock_api_smp.h | 8 +++----
include/linux/rwlock_types.h | 7 ++++++
4 files changed, 83 insertions(+), 9 deletions(-)

diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index 6653a4f..b93a707 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -600,16 +600,31 @@ static inline void print_irqtrace_events(struct task_struct *curr)
dept_spin_unlock(&(l)->dmap, i); \
} while (0)

-#define rwlock_acquire(l, s, t, i) lock_acquire_exclusive(l, s, t, NULL, i)
+#define rwlock_acquire(l, s, t, i) \
+do { \
+ lock_acquire_exclusive(l, s, t, NULL, i); \
+ dept_rwlock_wlock(&(l)->dmap, s, t, NULL, "write_unlock", i); \
+} while (0)
#define rwlock_acquire_read(l, s, t, i) \
do { \
- if (read_lock_is_recursive()) \
+ if (read_lock_is_recursive()) { \
lock_acquire_shared_recursive(l, s, t, NULL, i); \
- else \
+ dept_rwlock_rlock(&(l)->dmap, s, t, NULL, "read_unlock", i, 0);\
+ } else { \
lock_acquire_shared(l, s, t, NULL, i); \
+ dept_rwlock_rlock(&(l)->dmap, s, t, NULL, "read_unlock", i, 1);\
+ } \
+} while (0)
+#define rwlock_release(l, i) \
+do { \
+ lock_release(l, i); \
+ dept_rwlock_wunlock(&(l)->dmap, i); \
+} while (0)
+#define rwlock_release_read(l, i) \
+do { \
+ lock_release(l, i); \
+ dept_rwlock_runlock(&(l)->dmap, i); \
} while (0)
-
-#define rwlock_release(l, i) lock_release(l, i)

#define seqcount_acquire(l, s, t, i) lock_acquire_exclusive(l, s, t, NULL, i)
#define seqcount_acquire_read(l, s, t, i) lock_acquire_shared_recursive(l, s, t, NULL, i)
diff --git a/include/linux/rwlock.h b/include/linux/rwlock.h
index 8f416c5..768ad9e 100644
--- a/include/linux/rwlock.h
+++ b/include/linux/rwlock.h
@@ -28,6 +28,58 @@
do { *(lock) = __RW_LOCK_UNLOCKED(lock); } while (0)
#endif

+#ifdef CONFIG_DEPT
+#define DEPT_EVT_RWLOCK_R 1UL
+#define DEPT_EVT_RWLOCK_W (1UL << 1)
+#define DEPT_EVT_RWLOCK_RW (DEPT_EVT_RWLOCK_R | DEPT_EVT_RWLOCK_W)
+
+#define dept_rwlock_wlock(m, ne, t, n, e_fn, ip) \
+do { \
+ if (t) { \
+ dept_ecxt_enter(m, DEPT_EVT_RWLOCK_W, ip, __func__, e_fn, ne);\
+ dept_ask_event(m); \
+ } else if (n) { \
+ dept_skip(m); \
+ } else { \
+ dept_wait(m, DEPT_EVT_RWLOCK_RW, ip, __func__, ne); \
+ dept_ecxt_enter(m, DEPT_EVT_RWLOCK_W, ip, __func__, e_fn, ne);\
+ dept_ask_event(m); \
+ } \
+} while (0)
+#define dept_rwlock_rlock(m, ne, t, n, e_fn, ip, q) \
+do { \
+ if (t) { \
+ dept_ecxt_enter(m, DEPT_EVT_RWLOCK_R, ip, __func__, e_fn, ne);\
+ dept_ask_event(m); \
+ } else if (n) { \
+ dept_skip(m); \
+ } else { \
+ dept_wait(m, (q) ? DEPT_EVT_RWLOCK_RW : DEPT_EVT_RWLOCK_W, ip, __func__, ne);\
+ dept_ecxt_enter(m, DEPT_EVT_RWLOCK_R, ip, __func__, e_fn, ne);\
+ dept_ask_event(m); \
+ } \
+} while (0)
+#define dept_rwlock_wunlock(m, ip) \
+do { \
+ if (!dept_unskip_if_skipped(m)) { \
+ dept_event(m, DEPT_EVT_RWLOCK_W, ip, __func__); \
+ dept_ecxt_exit(m, ip); \
+ } \
+} while (0)
+#define dept_rwlock_runlock(m, ip) \
+do { \
+ if (!dept_unskip_if_skipped(m)) { \
+ dept_event(m, DEPT_EVT_RWLOCK_R, ip, __func__); \
+ dept_ecxt_exit(m, ip); \
+ } \
+} while (0)
+#else
+#define dept_rwlock_wlock(m, ne, t, n, e_fn, ip) do { } while (0)
+#define dept_rwlock_rlock(m, ne, t, n, e_fn, ip, q) do { } while (0)
+#define dept_rwlock_wunlock(m, ip) do { } while (0)
+#define dept_rwlock_runlock(m, ip) do { } while (0)
+#endif
+
#ifdef CONFIG_DEBUG_SPINLOCK
extern void do_raw_read_lock(rwlock_t *lock) __acquires(lock);
extern int do_raw_read_trylock(rwlock_t *lock);
diff --git a/include/linux/rwlock_api_smp.h b/include/linux/rwlock_api_smp.h
index dceb0a5..a222cf1 100644
--- a/include/linux/rwlock_api_smp.h
+++ b/include/linux/rwlock_api_smp.h
@@ -228,7 +228,7 @@ static inline void __raw_write_unlock(rwlock_t *lock)

static inline void __raw_read_unlock(rwlock_t *lock)
{
- rwlock_release(&lock->dep_map, _RET_IP_);
+ rwlock_release_read(&lock->dep_map, _RET_IP_);
do_raw_read_unlock(lock);
preempt_enable();
}
@@ -236,7 +236,7 @@ static inline void __raw_read_unlock(rwlock_t *lock)
static inline void
__raw_read_unlock_irqrestore(rwlock_t *lock, unsigned long flags)
{
- rwlock_release(&lock->dep_map, _RET_IP_);
+ rwlock_release_read(&lock->dep_map, _RET_IP_);
do_raw_read_unlock(lock);
local_irq_restore(flags);
preempt_enable();
@@ -244,7 +244,7 @@ static inline void __raw_read_unlock(rwlock_t *lock)

static inline void __raw_read_unlock_irq(rwlock_t *lock)
{
- rwlock_release(&lock->dep_map, _RET_IP_);
+ rwlock_release_read(&lock->dep_map, _RET_IP_);
do_raw_read_unlock(lock);
local_irq_enable();
preempt_enable();
@@ -252,7 +252,7 @@ static inline void __raw_read_unlock_irq(rwlock_t *lock)

static inline void __raw_read_unlock_bh(rwlock_t *lock)
{
- rwlock_release(&lock->dep_map, _RET_IP_);
+ rwlock_release_read(&lock->dep_map, _RET_IP_);
do_raw_read_unlock(lock);
__local_bh_enable_ip(_RET_IP_, SOFTIRQ_LOCK_OFFSET);
}
diff --git a/include/linux/rwlock_types.h b/include/linux/rwlock_types.h
index 1948442..74804b7 100644
--- a/include/linux/rwlock_types.h
+++ b/include/linux/rwlock_types.h
@@ -5,11 +5,18 @@
# error "Do not include directly, include spinlock_types.h"
#endif

+#ifdef CONFIG_DEPT
+# define RW_DMAP_INIT(lockname) .dmap = { .name = #lockname, .skip_cnt = ATOMIC_INIT(0) },
+#else
+# define RW_DMAP_INIT(lockname)
+#endif
+
#ifdef CONFIG_DEBUG_LOCK_ALLOC
# define RW_DEP_MAP_INIT(lockname) \
.dep_map = { \
.name = #lockname, \
.wait_type_inner = LD_WAIT_CONFIG, \
+ RW_DMAP_INIT(lockname) \
}
#else
# define RW_DEP_MAP_INIT(lockname)
--
1.9.1

2022-03-04 08:31:51

by Byungchul Park

[permalink] [raw]
Subject: [PATCH v4 03/24] dept: Embed Dept data in Lockdep

Dept should work independently from Lockdep. However, there's no choise
but to rely on Lockdep code and its instances for now.

Signed-off-by: Byungchul Park <[email protected]>
---
include/linux/lockdep.h | 71 ++++++++++++++++++++++++++++++++++++++++---
include/linux/lockdep_types.h | 3 ++
kernel/locking/lockdep.c | 12 ++++----
3 files changed, 76 insertions(+), 10 deletions(-)

diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index 467b942..c56f6b6 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -20,6 +20,33 @@
extern int prove_locking;
extern int lock_stat;

+#ifdef CONFIG_DEPT
+static inline void dept_after_copy_map(struct dept_map *to,
+ struct dept_map *from)
+{
+ int i;
+
+ if (from->keys == &from->keys_local)
+ to->keys = &to->keys_local;
+
+ if (!to->keys)
+ return;
+
+ /*
+ * Since the class cache can be modified concurrently we could observe
+ * half pointers (64bit arch using 32bit copy insns). Therefore clear
+ * the caches and take the performance hit.
+ *
+ * XXX it doesn't work well with lockdep_set_class_and_subclass(), since
+ * that relies on cache abuse.
+ */
+ for (i = 0; i < DEPT_MAX_SUBCLASSES_CACHE; i++)
+ to->keys->classes[i] = NULL;
+}
+#else
+#define dept_after_copy_map(t, f) do { } while (0)
+#endif
+
#ifdef CONFIG_LOCKDEP

#include <linux/linkage.h>
@@ -43,6 +70,8 @@ static inline void lockdep_copy_map(struct lockdep_map *to,
*/
for (i = 0; i < NR_LOCKDEP_CACHING_CLASSES; i++)
to->class_cache[i] = NULL;
+
+ dept_after_copy_map(&to->dmap, &from->dmap);
}

/*
@@ -176,8 +205,19 @@ struct held_lock {
current->lockdep_recursion -= LOCKDEP_OFF; \
} while (0)

-extern void lockdep_register_key(struct lock_class_key *key);
-extern void lockdep_unregister_key(struct lock_class_key *key);
+extern void __lockdep_register_key(struct lock_class_key *key);
+extern void __lockdep_unregister_key(struct lock_class_key *key);
+
+#define lockdep_register_key(k) \
+do { \
+ __lockdep_register_key(k); \
+ dept_key_init(&(k)->dkey); \
+} while (0)
+#define lockdep_unregister_key(k) \
+do { \
+ __lockdep_unregister_key(k); \
+ dept_key_destroy(&(k)->dkey); \
+} while (0)

/*
* These methods are used by specific locking variants (spinlocks,
@@ -185,9 +225,18 @@ struct held_lock {
* to lockdep:
*/

-extern void lockdep_init_map_type(struct lockdep_map *lock, const char *name,
+extern void __lockdep_init_map_type(struct lockdep_map *lock, const char *name,
struct lock_class_key *key, int subclass, u8 inner, u8 outer, u8 lock_type);

+#define lockdep_init_map_type(l, n, k, s, i, o, t) \
+do { \
+ __lockdep_init_map_type(l, n, k, s, i, o, t); \
+ if ((k) == &__lockdep_no_validate__) \
+ dept_map_nocheck(&(l)->dmap); \
+ else \
+ dept_map_init(&(l)->dmap, &(k)->dkey, s, n); \
+} while (0)
+
static inline void
lockdep_init_map_waits(struct lockdep_map *lock, const char *name,
struct lock_class_key *key, int subclass, u8 inner, u8 outer)
@@ -431,13 +480,27 @@ enum xhlock_context_t {
XHLOCK_CTX_NR,
};

+#ifdef CONFIG_DEPT
+/*
+ * TODO: I found the case to use an address of other than a real key as
+ * _key, for instance, in workqueue. So for now, we cannot use the
+ * assignment like '.dmap.keys = &(_key)->dkey' unless it's fixed.
+ */
+#define STATIC_DEPT_MAP_INIT(_name, _key) .dmap = { \
+ .name = (_name), \
+ .keys = NULL },
+#else
+#define STATIC_DEPT_MAP_INIT(_name, _key)
+#endif
+
#define lockdep_init_map_crosslock(m, n, k, s) do {} while (0)
/*
* To initialize a lockdep_map statically use this macro.
* Note that _name must not be NULL.
*/
#define STATIC_LOCKDEP_MAP_INIT(_name, _key) \
- { .name = (_name), .key = (void *)(_key), }
+ { .name = (_name), .key = (void *)(_key), \
+ STATIC_DEPT_MAP_INIT(_name, _key) }

static inline void lockdep_invariant_state(bool force) {}
static inline void lockdep_free_task(struct task_struct *task) {}
diff --git a/include/linux/lockdep_types.h b/include/linux/lockdep_types.h
index d224308..50c8879 100644
--- a/include/linux/lockdep_types.h
+++ b/include/linux/lockdep_types.h
@@ -11,6 +11,7 @@
#define __LINUX_LOCKDEP_TYPES_H

#include <linux/types.h>
+#include <linux/dept.h>

#define MAX_LOCKDEP_SUBCLASSES 8UL

@@ -76,6 +77,7 @@ struct lock_class_key {
struct hlist_node hash_entry;
struct lockdep_subclass_key subkeys[MAX_LOCKDEP_SUBCLASSES];
};
+ struct dept_key dkey;
};

extern struct lock_class_key __lockdep_no_validate__;
@@ -185,6 +187,7 @@ struct lockdep_map {
int cpu;
unsigned long ip;
#endif
+ struct dept_map dmap;
};

struct pin_cookie { unsigned int val; };
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 4a882f8..a85468d 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -1184,7 +1184,7 @@ static inline struct hlist_head *keyhashentry(const struct lock_class_key *key)
}

/* Register a dynamically allocated key. */
-void lockdep_register_key(struct lock_class_key *key)
+void __lockdep_register_key(struct lock_class_key *key)
{
struct hlist_head *hash_head;
struct lock_class_key *k;
@@ -1207,7 +1207,7 @@ void lockdep_register_key(struct lock_class_key *key)
restore_irqs:
raw_local_irq_restore(flags);
}
-EXPORT_SYMBOL_GPL(lockdep_register_key);
+EXPORT_SYMBOL_GPL(__lockdep_register_key);

/* Check whether a key has been registered as a dynamic key. */
static bool is_dynamic_key(const struct lock_class_key *key)
@@ -4771,7 +4771,7 @@ static inline int check_wait_context(struct task_struct *curr,
/*
* Initialize a lock instance's lock-class mapping info:
*/
-void lockdep_init_map_type(struct lockdep_map *lock, const char *name,
+void __lockdep_init_map_type(struct lockdep_map *lock, const char *name,
struct lock_class_key *key, int subclass,
u8 inner, u8 outer, u8 lock_type)
{
@@ -4831,7 +4831,7 @@ void lockdep_init_map_type(struct lockdep_map *lock, const char *name,
raw_local_irq_restore(flags);
}
}
-EXPORT_SYMBOL_GPL(lockdep_init_map_type);
+EXPORT_SYMBOL_GPL(__lockdep_init_map_type);

struct lock_class_key __lockdep_no_validate__;
EXPORT_SYMBOL_GPL(__lockdep_no_validate__);
@@ -6291,7 +6291,7 @@ void lockdep_reset_lock(struct lockdep_map *lock)
}

/* Unregister a dynamically allocated key. */
-void lockdep_unregister_key(struct lock_class_key *key)
+void __lockdep_unregister_key(struct lock_class_key *key)
{
struct hlist_head *hash_head = keyhashentry(key);
struct lock_class_key *k;
@@ -6326,7 +6326,7 @@ void lockdep_unregister_key(struct lock_class_key *key)
/* Wait until is_dynamic_key() has finished accessing k->hash_entry. */
synchronize_rcu();
}
-EXPORT_SYMBOL_GPL(lockdep_unregister_key);
+EXPORT_SYMBOL_GPL(__lockdep_unregister_key);

void __init lockdep_init(void)
{
--
1.9.1

2022-03-04 08:34:25

by Byungchul Park

[permalink] [raw]
Subject: [PATCH v4 16/24] locking/lockdep, cpu/hotplus: Use a weaker annotation in AP thread

cb92173d1f0 (locking/lockdep, cpu/hotplug: Annotate AP thread) was
introduced to make lockdep_assert_cpus_held() work in AP thread.

However, the annotation is too strong for that purpose. We don't have to
use more than try lock annotation for that.

Furthermore, now that Dept was introduced, false positive alarms was
reported by that. Replaced it with try lock annotation.

Signed-off-by: Byungchul Park <[email protected]>
---
kernel/cpu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/cpu.c b/kernel/cpu.c
index 407a256..1f92a42 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -355,7 +355,7 @@ int lockdep_is_cpus_held(void)

static void lockdep_acquire_cpus_lock(void)
{
- rwsem_acquire(&cpu_hotplug_lock.dep_map, 0, 0, _THIS_IP_);
+ rwsem_acquire(&cpu_hotplug_lock.dep_map, 0, 1, _THIS_IP_);
}

static void lockdep_release_cpus_lock(void)
--
1.9.1

2022-03-04 08:39:06

by Byungchul Park

[permalink] [raw]
Subject: [PATCH v4 21/24] dept: Disable Dept on struct crypto_larval's completion for now

struct crypto_larval's completion is used for multiple purposes e.g.
waiting for test to complete or waiting for probe to complete.

The completion variable needs to be split according to what it's used
for. Otherwise, Dept cannot distinguish one from another and doesn't
work properly. Now that it isn't, disable Dept on it.

Signed-off-by: Byungchul Park <[email protected]>
---
crypto/api.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/crypto/api.c b/crypto/api.c
index cf0869d..f501b91 100644
--- a/crypto/api.c
+++ b/crypto/api.c
@@ -115,7 +115,12 @@ struct crypto_larval *crypto_larval_alloc(const char *name, u32 type, u32 mask)
larval->alg.cra_destroy = crypto_larval_destroy;

strlcpy(larval->alg.cra_name, name, CRYPTO_MAX_ALG_NAME);
- init_completion(&larval->completion);
+ /*
+ * TODO: Split ->completion according to what it's used for e.g.
+ * ->test_completion, ->probe_completion and the like, so that
+ * Dept can track its dependency properly.
+ */
+ init_completion_nocheck(&larval->completion);

return larval;
}
--
1.9.1

2022-03-04 08:40:18

by Byungchul Park

[permalink] [raw]
Subject: [PATCH v4 04/24] dept: Add a API for skipping dependency check temporarily

Dept would skip check for dmaps marked by dept_map_nocheck() permanently.
However, sometimes it needs to skip check for some dmaps temporarily and
back to normal, for instance, lock acquisition with a nest lock.

Lock usage check with regard to nest lock could be performed by Lockdep,
however, dependency check is not necessary for that case. So prepared
for it by adding two new APIs, dept_skip() and dept_unskip_if_skipped().

Signed-off-by: Byungchul Park <[email protected]>
---
include/linux/dept.h | 9 +++++++++
include/linux/dept_sdt.h | 2 +-
include/linux/lockdep.h | 4 +++-
kernel/dependency/dept.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/include/linux/dept.h b/include/linux/dept.h
index c3fb3cf..c0bbb8e 100644
--- a/include/linux/dept.h
+++ b/include/linux/dept.h
@@ -352,6 +352,11 @@ struct dept_map {
unsigned int wgen;

/*
+ * for skipping dependency check temporarily
+ */
+ atomic_t skip_cnt;
+
+ /*
* whether this map should be going to be checked or not
*/
bool nocheck;
@@ -444,6 +449,8 @@ struct dept_task {
extern void dept_ask_event(struct dept_map *m);
extern void dept_event(struct dept_map *m, unsigned long e_f, unsigned long ip, const char *e_fn);
extern void dept_ecxt_exit(struct dept_map *m, unsigned long ip);
+extern void dept_skip(struct dept_map *m);
+extern bool dept_unskip_if_skipped(struct dept_map *m);

/*
* for users who want to manage external keys
@@ -475,6 +482,8 @@ struct dept_task {
#define dept_ask_event(m) do { } while (0)
#define dept_event(m, e_f, ip, e_fn) do { (void)(e_fn); } while (0)
#define dept_ecxt_exit(m, ip) do { } while (0)
+#define dept_skip(m) do { } while (0)
+#define dept_unskip_if_skipped(m) (false)
#define dept_key_init(k) do { (void)(k); } while (0)
#define dept_key_destroy(k) do { (void)(k); } while (0)
#endif
diff --git a/include/linux/dept_sdt.h b/include/linux/dept_sdt.h
index 375c4c3..e9d558d 100644
--- a/include/linux/dept_sdt.h
+++ b/include/linux/dept_sdt.h
@@ -13,7 +13,7 @@
#include <linux/dept.h>

#ifdef CONFIG_DEPT
-#define DEPT_SDT_MAP_INIT(dname) { .name = #dname }
+#define DEPT_SDT_MAP_INIT(dname) { .name = #dname, .skip_cnt = ATOMIC_INIT(0) }

/*
* SDT(Single-event Dependency Tracker) APIs
diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index c56f6b6..c1a56fe 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -488,7 +488,9 @@ enum xhlock_context_t {
*/
#define STATIC_DEPT_MAP_INIT(_name, _key) .dmap = { \
.name = (_name), \
- .keys = NULL },
+ .keys = NULL, \
+ .skip_cnt = ATOMIC_INIT(0), \
+ },
#else
#define STATIC_DEPT_MAP_INIT(_name, _key)
#endif
diff --git a/kernel/dependency/dept.c b/kernel/dependency/dept.c
index ec3f131..3f22c5b 100644
--- a/kernel/dependency/dept.c
+++ b/kernel/dependency/dept.c
@@ -1943,6 +1943,7 @@ void dept_map_init(struct dept_map *m, struct dept_key *k, int sub,
m->name = n;
m->wgen = 0U;
m->nocheck = false;
+ atomic_set(&m->skip_cnt, 0);
exit:
dept_exit(flags);
}
@@ -1963,6 +1964,7 @@ void dept_map_reinit(struct dept_map *m)

clean_classes_cache(&m->keys_local);
m->wgen = 0U;
+ atomic_set(&m->skip_cnt, 0);

dept_exit(flags);
}
@@ -2346,6 +2348,53 @@ void dept_ecxt_exit(struct dept_map *m, unsigned long ip)
}
EXPORT_SYMBOL_GPL(dept_ecxt_exit);

+void dept_skip(struct dept_map *m)
+{
+ struct dept_task *dt = dept_task();
+ unsigned long flags;
+
+ if (READ_ONCE(dept_stop) || dt->recursive)
+ return;
+
+ if (m->nocheck)
+ return;
+
+ flags = dept_enter();
+
+ atomic_inc(&m->skip_cnt);
+
+ dept_exit(flags);
+}
+EXPORT_SYMBOL_GPL(dept_skip);
+
+/*
+ * Return true if successfully unskip, otherwise false.
+ */
+bool dept_unskip_if_skipped(struct dept_map *m)
+{
+ struct dept_task *dt = dept_task();
+ unsigned long flags;
+ bool ret = false;
+
+ if (READ_ONCE(dept_stop) || dt->recursive)
+ return false;
+
+ if (m->nocheck)
+ return false;
+
+ flags = dept_enter();
+
+ if (!atomic_read(&m->skip_cnt))
+ goto exit;
+
+ atomic_dec(&m->skip_cnt);
+ ret = true;
+exit:
+ dept_exit(flags);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(dept_unskip_if_skipped);
+
void dept_task_exit(struct task_struct *t)
{
struct dept_task *dt = &t->dept_task;
--
1.9.1

2022-03-04 08:52:09

by Byungchul Park

[permalink] [raw]
Subject: [PATCH v4 15/24] dept: Apply SDT to wait(waitqueue)

Makes SDT able to track dependencies by wait(waitqueue).

Signed-off-by: Byungchul Park <[email protected]>
---
include/linux/wait.h | 6 +++++-
kernel/sched/wait.c | 16 ++++++++++++++++
2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/include/linux/wait.h b/include/linux/wait.h
index 851e07d..2133998 100644
--- a/include/linux/wait.h
+++ b/include/linux/wait.h
@@ -7,6 +7,7 @@
#include <linux/list.h>
#include <linux/stddef.h>
#include <linux/spinlock.h>
+#include <linux/dept_sdt.h>

#include <asm/current.h>
#include <uapi/linux/wait.h>
@@ -37,6 +38,7 @@ struct wait_queue_entry {
struct wait_queue_head {
spinlock_t lock;
struct list_head head;
+ struct dept_map dmap;
};
typedef struct wait_queue_head wait_queue_head_t;

@@ -56,7 +58,8 @@ struct wait_queue_head {

#define __WAIT_QUEUE_HEAD_INITIALIZER(name) { \
.lock = __SPIN_LOCK_UNLOCKED(name.lock), \
- .head = LIST_HEAD_INIT(name.head) }
+ .head = LIST_HEAD_INIT(name.head), \
+ .dmap = DEPT_SDT_MAP_INIT(name) }

#define DECLARE_WAIT_QUEUE_HEAD(name) \
struct wait_queue_head name = __WAIT_QUEUE_HEAD_INITIALIZER(name)
@@ -67,6 +70,7 @@ struct wait_queue_head {
do { \
static struct lock_class_key __key; \
\
+ sdt_map_init(&(wq_head)->dmap); \
__init_waitqueue_head((wq_head), #wq_head, &__key); \
} while (0)

diff --git a/kernel/sched/wait.c b/kernel/sched/wait.c
index eca3810..fc5a16a 100644
--- a/kernel/sched/wait.c
+++ b/kernel/sched/wait.c
@@ -105,6 +105,7 @@ static int __wake_up_common(struct wait_queue_head *wq_head, unsigned int mode,
if (flags & WQ_FLAG_BOOKMARK)
continue;

+ sdt_event(&wq_head->dmap);
ret = curr->func(curr, mode, wake_flags, key);
if (ret < 0)
break;
@@ -268,6 +269,9 @@ void __wake_up_pollfree(struct wait_queue_head *wq_head)
__add_wait_queue(wq_head, wq_entry);
set_current_state(state);
spin_unlock_irqrestore(&wq_head->lock, flags);
+
+ if (state & TASK_NORMAL)
+ sdt_wait_prepare(&wq_head->dmap);
}
EXPORT_SYMBOL(prepare_to_wait);

@@ -286,6 +290,10 @@ void __wake_up_pollfree(struct wait_queue_head *wq_head)
}
set_current_state(state);
spin_unlock_irqrestore(&wq_head->lock, flags);
+
+ if (state & TASK_NORMAL)
+ sdt_wait_prepare(&wq_head->dmap);
+
return was_empty;
}
EXPORT_SYMBOL(prepare_to_wait_exclusive);
@@ -331,6 +339,9 @@ long prepare_to_wait_event(struct wait_queue_head *wq_head, struct wait_queue_en
}
spin_unlock_irqrestore(&wq_head->lock, flags);

+ if (!ret && state & TASK_NORMAL)
+ sdt_wait_prepare(&wq_head->dmap);
+
return ret;
}
EXPORT_SYMBOL(prepare_to_wait_event);
@@ -352,7 +363,9 @@ int do_wait_intr(wait_queue_head_t *wq, wait_queue_entry_t *wait)
return -ERESTARTSYS;

spin_unlock(&wq->lock);
+ sdt_wait_prepare(&wq->dmap);
schedule();
+ sdt_wait_finish();
spin_lock(&wq->lock);

return 0;
@@ -369,7 +382,9 @@ int do_wait_intr_irq(wait_queue_head_t *wq, wait_queue_entry_t *wait)
return -ERESTARTSYS;

spin_unlock_irq(&wq->lock);
+ sdt_wait_prepare(&wq->dmap);
schedule();
+ sdt_wait_finish();
spin_lock_irq(&wq->lock);

return 0;
@@ -389,6 +404,7 @@ void finish_wait(struct wait_queue_head *wq_head, struct wait_queue_entry *wq_en
{
unsigned long flags;

+ sdt_wait_finish();
__set_current_state(TASK_RUNNING);
/*
* We can check for list emptiness outside the lock
--
1.9.1

2022-03-04 09:12:26

by Byungchul Park

[permalink] [raw]
Subject: [PATCH v4 08/24] dept: Apply Dept to wait_for_completion()/complete()

Makes Dept able to track dependencies by
wait_for_completion()/complete().

Signed-off-by: Byungchul Park <[email protected]>
---
include/linux/completion.h | 42 ++++++++++++++++++++++++++++++++++++++++--
kernel/sched/completion.c | 12 ++++++++++--
2 files changed, 50 insertions(+), 4 deletions(-)

diff --git a/include/linux/completion.h b/include/linux/completion.h
index 51d9ab0..a1ad5a8 100644
--- a/include/linux/completion.h
+++ b/include/linux/completion.h
@@ -26,14 +26,48 @@
struct completion {
unsigned int done;
struct swait_queue_head wait;
+ struct dept_map dmap;
};

+#ifdef CONFIG_DEPT
+#define dept_wfc_init(m, k, s, n) dept_map_init(m, k, s, n)
+#define dept_wfc_reinit(m) dept_map_reinit(m)
+#define dept_wfc_wait(m, ip) \
+do { \
+ dept_ask_event(m); \
+ dept_wait(m, 1UL, ip, __func__, 0); \
+} 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)
+#define dept_wfc_exit(m, ip) dept_ecxt_exit(m, ip)
+#else
+#define dept_wfc_init(m, k, s, n) do { (void)(n); (void)(k); } while (0)
+#define dept_wfc_reinit(m) do { } while (0)
+#define dept_wfc_wait(m, ip) do { } while (0)
+#define dept_wfc_complete(m, ip) do { } while (0)
+#define dept_wfc_enter(m, ip) do { } while (0)
+#define dept_wfc_exit(m, ip) do { } while (0)
+#endif
+
+#ifdef CONFIG_DEPT
+#define WFC_DEPT_MAP_INIT(work) .dmap = { .name = #work, .skip_cnt = ATOMIC_INIT(0) }
+#else
+#define WFC_DEPT_MAP_INIT(work)
+#endif
+
+#define init_completion(x) \
+ do { \
+ static struct dept_key __dkey; \
+ __init_completion(x, &__dkey, #x); \
+ } while (0)
+
#define init_completion_map(x, m) init_completion(x)
static inline void complete_acquire(struct completion *x) {}
static inline void complete_release(struct completion *x) {}

#define COMPLETION_INITIALIZER(work) \
- { 0, __SWAIT_QUEUE_HEAD_INITIALIZER((work).wait) }
+ { 0, __SWAIT_QUEUE_HEAD_INITIALIZER((work).wait), \
+ WFC_DEPT_MAP_INIT(work) }

#define COMPLETION_INITIALIZER_ONSTACK_MAP(work, map) \
(*({ init_completion_map(&(work), &(map)); &(work); }))
@@ -81,9 +115,12 @@ static inline void complete_release(struct completion *x) {}
* This inline function will initialize a dynamically created completion
* structure.
*/
-static inline void init_completion(struct completion *x)
+static inline void __init_completion(struct completion *x,
+ struct dept_key *dkey,
+ const char *name)
{
x->done = 0;
+ dept_wfc_init(&x->dmap, dkey, 0, name);
init_swait_queue_head(&x->wait);
}

@@ -97,6 +134,7 @@ static inline void init_completion(struct completion *x)
static inline void reinit_completion(struct completion *x)
{
x->done = 0;
+ dept_wfc_reinit(&x->dmap);
}

extern void wait_for_completion(struct completion *);
diff --git a/kernel/sched/completion.c b/kernel/sched/completion.c
index a778554..6e31cc0 100644
--- a/kernel/sched/completion.c
+++ b/kernel/sched/completion.c
@@ -29,6 +29,7 @@ void complete(struct completion *x)
{
unsigned long flags;

+ dept_wfc_complete(&x->dmap, _RET_IP_);
raw_spin_lock_irqsave(&x->wait.lock, flags);

if (x->done != UINT_MAX)
@@ -58,6 +59,7 @@ void complete_all(struct completion *x)
{
unsigned long flags;

+ dept_wfc_complete(&x->dmap, _RET_IP_);
lockdep_assert_RT_in_threaded_ctx();

raw_spin_lock_irqsave(&x->wait.lock, flags);
@@ -112,17 +114,23 @@ void complete_all(struct completion *x)
}

static long __sched
-wait_for_common(struct completion *x, long timeout, int state)
+_wait_for_common(struct completion *x, long timeout, int state)
{
return __wait_for_common(x, schedule_timeout, timeout, state);
}

static long __sched
-wait_for_common_io(struct completion *x, long timeout, int state)
+_wait_for_common_io(struct completion *x, long timeout, int state)
{
return __wait_for_common(x, io_schedule_timeout, timeout, state);
}

+#define wait_for_common(x, t, s) \
+({ dept_wfc_wait(&(x)->dmap, _RET_IP_); _wait_for_common(x, t, s); })
+
+#define wait_for_common_io(x, t, s) \
+({ dept_wfc_wait(&(x)->dmap, _RET_IP_); _wait_for_common_io(x, t, s); })
+
/**
* wait_for_completion: - waits for completion of a task
* @x: holds the state of this particular completion
--
1.9.1

2022-03-04 09:26:08

by Byungchul Park

[permalink] [raw]
Subject: [PATCH v4 18/24] dept: Distinguish each work from another

Workqueue already provides concurrency control. By that, any wait in a
work doesn't prevents events in other works with the control enabled.
Thus, each work would better be considered a different context.

So let Dept assign a different context id to each work.

Signed-off-by: Byungchul Park <[email protected]>
---
include/linux/dept.h | 2 ++
kernel/dependency/dept.c | 10 ++++++++++
kernel/workqueue.c | 3 +++
3 files changed, 15 insertions(+)

diff --git a/include/linux/dept.h b/include/linux/dept.h
index 1a1c307..55c5ed5 100644
--- a/include/linux/dept.h
+++ b/include/linux/dept.h
@@ -486,6 +486,7 @@ struct dept_task {
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);
+extern void dept_work_enter(void);

/*
* for users who want to manage external keys
@@ -527,6 +528,7 @@ struct dept_task {
#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)
+#define dept_work_enter() do { } while (0)
#define dept_key_init(k) do { (void)(k); } while (0)
#define dept_key_destroy(k) do { (void)(k); } while (0)
#endif
diff --git a/kernel/dependency/dept.c b/kernel/dependency/dept.c
index 8f962ae..5d4efc3 100644
--- a/kernel/dependency/dept.c
+++ b/kernel/dependency/dept.c
@@ -1873,6 +1873,16 @@ void dept_disable_hardirq(unsigned long ip)
dept_exit(flags);
}

+/*
+ * Assign a different context id to each work.
+ */
+void dept_work_enter(void)
+{
+ struct dept_task *dt = dept_task();
+
+ dt->cxt_id[DEPT_CXT_PROCESS] += (1UL << DEPT_CXTS_NR);
+}
+
void dept_kernel_enter(void)
{
struct dept_task *dt = dept_task();
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 33f1106..f5d762c 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -51,6 +51,7 @@
#include <linux/sched/isolation.h>
#include <linux/nmi.h>
#include <linux/kvm_para.h>
+#include <linux/dept.h>

#include "workqueue_internal.h"

@@ -2217,6 +2218,8 @@ static void process_one_work(struct worker *worker, struct work_struct *work)

lockdep_copy_map(&lockdep_map, &work->lockdep_map);
#endif
+ dept_work_enter();
+
/* ensure we're on the correct CPU */
WARN_ON_ONCE(!(pool->flags & POOL_DISASSOCIATED) &&
raw_smp_processor_id() != pool->cpu);
--
1.9.1

2022-03-04 10:09:31

by Byungchul Park

[permalink] [raw]
Subject: [PATCH v4 20/24] dept: Add nocheck version of init_completion()

For completions who don't want to get tracked by Dept, added
init_completion_nocheck() to disable Dept on it.

Signed-off-by: Byungchul Park <[email protected]>
---
include/linux/completion.h | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/include/linux/completion.h b/include/linux/completion.h
index a1ad5a8..9bd3bc9 100644
--- a/include/linux/completion.h
+++ b/include/linux/completion.h
@@ -30,6 +30,7 @@ struct completion {
};

#ifdef CONFIG_DEPT
+#define dept_wfc_nocheck(m) dept_map_nocheck(m)
#define dept_wfc_init(m, k, s, n) dept_map_init(m, k, s, n)
#define dept_wfc_reinit(m) dept_map_reinit(m)
#define dept_wfc_wait(m, ip) \
@@ -41,6 +42,7 @@ struct completion {
#define dept_wfc_enter(m, ip) dept_ecxt_enter(m, 1UL, ip, "completion_context_enter", "complete", 0)
#define dept_wfc_exit(m, ip) dept_ecxt_exit(m, ip)
#else
+#define dept_wfc_nocheck(m) do { } while (0)
#define dept_wfc_init(m, k, s, n) do { (void)(n); (void)(k); } while (0)
#define dept_wfc_reinit(m) do { } while (0)
#define dept_wfc_wait(m, ip) do { } while (0)
@@ -55,10 +57,11 @@ struct completion {
#define WFC_DEPT_MAP_INIT(work)
#endif

+#define init_completion_nocheck(x) __init_completion(x, NULL, #x, false)
#define init_completion(x) \
do { \
static struct dept_key __dkey; \
- __init_completion(x, &__dkey, #x); \
+ __init_completion(x, &__dkey, #x, true); \
} while (0)

#define init_completion_map(x, m) init_completion(x)
@@ -117,10 +120,15 @@ static inline void complete_release(struct completion *x) {}
*/
static inline void __init_completion(struct completion *x,
struct dept_key *dkey,
- const char *name)
+ const char *name, bool check)
{
x->done = 0;
- dept_wfc_init(&x->dmap, dkey, 0, name);
+
+ if (check)
+ dept_wfc_init(&x->dmap, dkey, 0, name);
+ else
+ dept_wfc_nocheck(&x->dmap);
+
init_swait_queue_head(&x->wait);
}

--
1.9.1

2022-03-04 13:48:56

by Byungchul Park

[permalink] [raw]
Subject: [PATCH v4 14/24] dept: Apply SDT to swait

Makes SDT able to track dependencies by swait.

Signed-off-by: Byungchul Park <[email protected]>
---
include/linux/swait.h | 4 ++++
kernel/sched/swait.c | 10 ++++++++++
2 files changed, 14 insertions(+)

diff --git a/include/linux/swait.h b/include/linux/swait.h
index 6a8c22b..dbdf2ce 100644
--- a/include/linux/swait.h
+++ b/include/linux/swait.h
@@ -6,6 +6,7 @@
#include <linux/stddef.h>
#include <linux/spinlock.h>
#include <linux/wait.h>
+#include <linux/dept_sdt.h>
#include <asm/current.h>

/*
@@ -43,6 +44,7 @@
struct swait_queue_head {
raw_spinlock_t lock;
struct list_head task_list;
+ struct dept_map dmap;
};

struct swait_queue {
@@ -61,6 +63,7 @@ struct swait_queue {
#define __SWAIT_QUEUE_HEAD_INITIALIZER(name) { \
.lock = __RAW_SPIN_LOCK_UNLOCKED(name.lock), \
.task_list = LIST_HEAD_INIT((name).task_list), \
+ .dmap = DEPT_SDT_MAP_INIT(name), \
}

#define DECLARE_SWAIT_QUEUE_HEAD(name) \
@@ -72,6 +75,7 @@ extern void __init_swait_queue_head(struct swait_queue_head *q, const char *name
#define init_swait_queue_head(q) \
do { \
static struct lock_class_key __key; \
+ sdt_map_init(&(q)->dmap); \
__init_swait_queue_head((q), #q, &__key); \
} while (0)

diff --git a/kernel/sched/swait.c b/kernel/sched/swait.c
index e1c655f..4ca7d6e 100644
--- a/kernel/sched/swait.c
+++ b/kernel/sched/swait.c
@@ -27,6 +27,7 @@ void swake_up_locked(struct swait_queue_head *q)
return;

curr = list_first_entry(&q->task_list, typeof(*curr), task_list);
+ sdt_event(&q->dmap);
wake_up_process(curr->task);
list_del_init(&curr->task_list);
}
@@ -69,6 +70,7 @@ void swake_up_all(struct swait_queue_head *q)
while (!list_empty(&tmp)) {
curr = list_first_entry(&tmp, typeof(*curr), task_list);

+ sdt_event(&q->dmap);
wake_up_state(curr->task, TASK_NORMAL);
list_del_init(&curr->task_list);

@@ -97,6 +99,9 @@ void prepare_to_swait_exclusive(struct swait_queue_head *q, struct swait_queue *
__prepare_to_swait(q, wait);
set_current_state(state);
raw_spin_unlock_irqrestore(&q->lock, flags);
+
+ if (state & TASK_NORMAL)
+ sdt_wait_prepare(&q->dmap);
}
EXPORT_SYMBOL(prepare_to_swait_exclusive);

@@ -119,12 +124,16 @@ long prepare_to_swait_event(struct swait_queue_head *q, struct swait_queue *wait
}
raw_spin_unlock_irqrestore(&q->lock, flags);

+ if (!ret && state & TASK_NORMAL)
+ sdt_wait_prepare(&q->dmap);
+
return ret;
}
EXPORT_SYMBOL(prepare_to_swait_event);

void __finish_swait(struct swait_queue_head *q, struct swait_queue *wait)
{
+ sdt_wait_finish();
__set_current_state(TASK_RUNNING);
if (!list_empty(&wait->task_list))
list_del_init(&wait->task_list);
@@ -134,6 +143,7 @@ void finish_swait(struct swait_queue_head *q, struct swait_queue *wait)
{
unsigned long flags;

+ sdt_wait_finish();
__set_current_state(TASK_RUNNING);

if (!list_empty_careful(&wait->task_list)) {
--
1.9.1

2022-03-04 23:51:14

by Byungchul Park

[permalink] [raw]
Subject: Re: [PATCH v4 16/24] locking/lockdep, cpu/hotplus: Use a weaker annotation in AP thread

On Fri, Mar 04, 2022 at 10:28:57PM +0300, Sergei Shtylyov wrote:
> On 3/4/22 10:06 AM, Byungchul Park wrote:
>
> > cb92173d1f0 (locking/lockdep, cpu/hotplug: Annotate AP thread) was
>
> You need to enclose the commit summary in (""), not just (). :-)

Thank you! I will!

> > introduced to make lockdep_assert_cpus_held() work in AP thread.
> >
> > However, the annotation is too strong for that purpose. We don't have to
> > use more than try lock annotation for that.
> >
> > Furthermore, now that Dept was introduced, false positive alarms was
> > reported by that. Replaced it with try lock annotation.
> >
> > Signed-off-by: Byungchul Park <[email protected]>
> [...]
>
> MBR, Sergey

2022-03-09 09:42:51

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v4 14/24] dept: Apply SDT to swait

Hi Byungchul,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on tip/sched/core]
[also build test WARNING on linux/master linus/master v5.17-rc7]
[cannot apply to tip/locking/core hnaz-mm/master next-20220308]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/0day-ci/linux/commits/Byungchul-Park/DEPT-Dependency-Tracker/20220304-150943
base: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 25795ef6299f07ce3838f3253a9cb34f64efcfae
config: hexagon-randconfig-r022-20220307 (https://download.01.org/0day-ci/archive/20220309/[email protected]/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project d271fc04d5b97b12e6b797c6067d3c96a8d7470e)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/dbdd22ef0f5b79f561dc8766d253b10b496c0091
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Byungchul-Park/DEPT-Dependency-Tracker/20220304-150943
git checkout dbdd22ef0f5b79f561dc8766d253b10b496c0091
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/target/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

>> drivers/target/target_core_xcopy.c:667:13: warning: stack frame size (1064) exceeds limit (1024) in 'target_xcopy_do_work' [-Wframe-larger-than]
static void target_xcopy_do_work(struct work_struct *work)
^
1 warning generated.

Kconfig warnings: (for reference only)
WARNING: unmet direct dependencies detected for FRAME_POINTER
Depends on DEBUG_KERNEL && (M68K || UML || SUPERH) || ARCH_WANT_FRAME_POINTERS
Selected by
- DEPT && DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT && !MIPS && !PPC && !ARM && !S390 && !MICROBLAZE && !ARC && !X86


vim +/target_xcopy_do_work +667 drivers/target/target_core_xcopy.c

cbf031f425fd0b Nicholas Bellinger 2013-08-20 666
cbf031f425fd0b Nicholas Bellinger 2013-08-20 @667 static void target_xcopy_do_work(struct work_struct *work)
cbf031f425fd0b Nicholas Bellinger 2013-08-20 668 {
cbf031f425fd0b Nicholas Bellinger 2013-08-20 669 struct xcopy_op *xop = container_of(work, struct xcopy_op, xop_work);
cbf031f425fd0b Nicholas Bellinger 2013-08-20 670 struct se_cmd *ec_cmd = xop->xop_se_cmd;
d877d7275be34a Bart Van Assche 2017-05-23 671 struct se_device *src_dev, *dst_dev;
d877d7275be34a Bart Van Assche 2017-05-23 672 sector_t src_lba, dst_lba, end_lba;
cbf031f425fd0b Nicholas Bellinger 2013-08-20 673 unsigned int max_sectors;
d877d7275be34a Bart Van Assche 2017-05-23 674 int rc = 0;
0ad08996da05b6 David Disseldorp 2020-03-27 675 unsigned short nolb, max_nolb, copied_nolb = 0;
0394b5048efd73 Sergey Samoylenko 2021-08-03 676 sense_reason_t sense_rc;
d877d7275be34a Bart Van Assche 2017-05-23 677
0394b5048efd73 Sergey Samoylenko 2021-08-03 678 sense_rc = target_parse_xcopy_cmd(xop);
0394b5048efd73 Sergey Samoylenko 2021-08-03 679 if (sense_rc != TCM_NO_SENSE)
d877d7275be34a Bart Van Assche 2017-05-23 680 goto err_free;
cbf031f425fd0b Nicholas Bellinger 2013-08-20 681
0394b5048efd73 Sergey Samoylenko 2021-08-03 682 if (WARN_ON_ONCE(!xop->src_dev) || WARN_ON_ONCE(!xop->dst_dev)) {
0394b5048efd73 Sergey Samoylenko 2021-08-03 683 sense_rc = TCM_INVALID_PARAMETER_LIST;
d877d7275be34a Bart Van Assche 2017-05-23 684 goto err_free;
0394b5048efd73 Sergey Samoylenko 2021-08-03 685 }
d877d7275be34a Bart Van Assche 2017-05-23 686
d877d7275be34a Bart Van Assche 2017-05-23 687 src_dev = xop->src_dev;
d877d7275be34a Bart Van Assche 2017-05-23 688 dst_dev = xop->dst_dev;
d877d7275be34a Bart Van Assche 2017-05-23 689 src_lba = xop->src_lba;
d877d7275be34a Bart Van Assche 2017-05-23 690 dst_lba = xop->dst_lba;
d877d7275be34a Bart Van Assche 2017-05-23 691 nolb = xop->nolb;
cbf031f425fd0b Nicholas Bellinger 2013-08-20 692 end_lba = src_lba + nolb;
cbf031f425fd0b Nicholas Bellinger 2013-08-20 693 /*
cbf031f425fd0b Nicholas Bellinger 2013-08-20 694 * Break up XCOPY I/O into hw_max_sectors sized I/O based on the
cbf031f425fd0b Nicholas Bellinger 2013-08-20 695 * smallest max_sectors between src_dev + dev_dev, or
cbf031f425fd0b Nicholas Bellinger 2013-08-20 696 */
cbf031f425fd0b Nicholas Bellinger 2013-08-20 697 max_sectors = min(src_dev->dev_attrib.hw_max_sectors,
cbf031f425fd0b Nicholas Bellinger 2013-08-20 698 dst_dev->dev_attrib.hw_max_sectors);
cbf031f425fd0b Nicholas Bellinger 2013-08-20 699 max_sectors = min_t(u32, max_sectors, XCOPY_MAX_SECTORS);
cbf031f425fd0b Nicholas Bellinger 2013-08-20 700
cbf031f425fd0b Nicholas Bellinger 2013-08-20 701 max_nolb = min_t(u16, max_sectors, ((u16)(~0U)));
cbf031f425fd0b Nicholas Bellinger 2013-08-20 702
cbf031f425fd0b Nicholas Bellinger 2013-08-20 703 pr_debug("target_xcopy_do_work: nolb: %hu, max_nolb: %hu end_lba: %llu\n",
cbf031f425fd0b Nicholas Bellinger 2013-08-20 704 nolb, max_nolb, (unsigned long long)end_lba);
cbf031f425fd0b Nicholas Bellinger 2013-08-20 705 pr_debug("target_xcopy_do_work: Starting src_lba: %llu, dst_lba: %llu\n",
cbf031f425fd0b Nicholas Bellinger 2013-08-20 706 (unsigned long long)src_lba, (unsigned long long)dst_lba);
cbf031f425fd0b Nicholas Bellinger 2013-08-20 707
cbf031f425fd0b Nicholas Bellinger 2013-08-20 708 while (src_lba < end_lba) {
0ad08996da05b6 David Disseldorp 2020-03-27 709 unsigned short cur_nolb = min(nolb, max_nolb);
0ad08996da05b6 David Disseldorp 2020-03-27 710 u32 cur_bytes = cur_nolb * src_dev->dev_attrib.block_size;
0ad08996da05b6 David Disseldorp 2020-03-27 711
0ad08996da05b6 David Disseldorp 2020-03-27 712 if (cur_bytes != xop->xop_data_bytes) {
0ad08996da05b6 David Disseldorp 2020-03-27 713 /*
0ad08996da05b6 David Disseldorp 2020-03-27 714 * (Re)allocate a buffer large enough to hold the XCOPY
0ad08996da05b6 David Disseldorp 2020-03-27 715 * I/O size, which can be reused each read / write loop.
0ad08996da05b6 David Disseldorp 2020-03-27 716 */
0ad08996da05b6 David Disseldorp 2020-03-27 717 target_free_sgl(xop->xop_data_sg, xop->xop_data_nents);
0ad08996da05b6 David Disseldorp 2020-03-27 718 rc = target_alloc_sgl(&xop->xop_data_sg,
0ad08996da05b6 David Disseldorp 2020-03-27 719 &xop->xop_data_nents,
0ad08996da05b6 David Disseldorp 2020-03-27 720 cur_bytes,
0ad08996da05b6 David Disseldorp 2020-03-27 721 false, false);
0ad08996da05b6 David Disseldorp 2020-03-27 722 if (rc < 0)
0ad08996da05b6 David Disseldorp 2020-03-27 723 goto out;
0ad08996da05b6 David Disseldorp 2020-03-27 724 xop->xop_data_bytes = cur_bytes;
0ad08996da05b6 David Disseldorp 2020-03-27 725 }
cbf031f425fd0b Nicholas Bellinger 2013-08-20 726
cbf031f425fd0b Nicholas Bellinger 2013-08-20 727 pr_debug("target_xcopy_do_work: Calling read src_dev: %p src_lba: %llu,"
cbf031f425fd0b Nicholas Bellinger 2013-08-20 728 " cur_nolb: %hu\n", src_dev, (unsigned long long)src_lba, cur_nolb);
cbf031f425fd0b Nicholas Bellinger 2013-08-20 729
cbf031f425fd0b Nicholas Bellinger 2013-08-20 730 rc = target_xcopy_read_source(ec_cmd, xop, src_dev, src_lba, cur_nolb);
cbf031f425fd0b Nicholas Bellinger 2013-08-20 731 if (rc < 0)
cbf031f425fd0b Nicholas Bellinger 2013-08-20 732 goto out;
cbf031f425fd0b Nicholas Bellinger 2013-08-20 733
cbf031f425fd0b Nicholas Bellinger 2013-08-20 734 src_lba += cur_nolb;
cbf031f425fd0b Nicholas Bellinger 2013-08-20 735 pr_debug("target_xcopy_do_work: Incremented READ src_lba to %llu\n",
cbf031f425fd0b Nicholas Bellinger 2013-08-20 736 (unsigned long long)src_lba);
cbf031f425fd0b Nicholas Bellinger 2013-08-20 737
cbf031f425fd0b Nicholas Bellinger 2013-08-20 738 pr_debug("target_xcopy_do_work: Calling write dst_dev: %p dst_lba: %llu,"
cbf031f425fd0b Nicholas Bellinger 2013-08-20 739 " cur_nolb: %hu\n", dst_dev, (unsigned long long)dst_lba, cur_nolb);
cbf031f425fd0b Nicholas Bellinger 2013-08-20 740
cbf031f425fd0b Nicholas Bellinger 2013-08-20 741 rc = target_xcopy_write_destination(ec_cmd, xop, dst_dev,
cbf031f425fd0b Nicholas Bellinger 2013-08-20 742 dst_lba, cur_nolb);
b92fcfcb687de7 David Disseldorp 2020-03-27 743 if (rc < 0)
cbf031f425fd0b Nicholas Bellinger 2013-08-20 744 goto out;
cbf031f425fd0b Nicholas Bellinger 2013-08-20 745
cbf031f425fd0b Nicholas Bellinger 2013-08-20 746 dst_lba += cur_nolb;
cbf031f425fd0b Nicholas Bellinger 2013-08-20 747 pr_debug("target_xcopy_do_work: Incremented WRITE dst_lba to %llu\n",
cbf031f425fd0b Nicholas Bellinger 2013-08-20 748 (unsigned long long)dst_lba);
cbf031f425fd0b Nicholas Bellinger 2013-08-20 749
cbf031f425fd0b Nicholas Bellinger 2013-08-20 750 copied_nolb += cur_nolb;
cbf031f425fd0b Nicholas Bellinger 2013-08-20 751 nolb -= cur_nolb;
cbf031f425fd0b Nicholas Bellinger 2013-08-20 752 }
cbf031f425fd0b Nicholas Bellinger 2013-08-20 753
cbf031f425fd0b Nicholas Bellinger 2013-08-20 754 xcopy_pt_undepend_remotedev(xop);
0ad08996da05b6 David Disseldorp 2020-03-27 755 target_free_sgl(xop->xop_data_sg, xop->xop_data_nents);
cbf031f425fd0b Nicholas Bellinger 2013-08-20 756 kfree(xop);
cbf031f425fd0b Nicholas Bellinger 2013-08-20 757
cbf031f425fd0b Nicholas Bellinger 2013-08-20 758 pr_debug("target_xcopy_do_work: Final src_lba: %llu, dst_lba: %llu\n",
cbf031f425fd0b Nicholas Bellinger 2013-08-20 759 (unsigned long long)src_lba, (unsigned long long)dst_lba);
cbf031f425fd0b Nicholas Bellinger 2013-08-20 760 pr_debug("target_xcopy_do_work: Blocks copied: %hu, Bytes Copied: %u\n",
cbf031f425fd0b Nicholas Bellinger 2013-08-20 761 copied_nolb, copied_nolb * dst_dev->dev_attrib.block_size);
cbf031f425fd0b Nicholas Bellinger 2013-08-20 762
cbf031f425fd0b Nicholas Bellinger 2013-08-20 763 pr_debug("target_xcopy_do_work: Setting X-COPY GOOD status -> sending response\n");
cbf031f425fd0b Nicholas Bellinger 2013-08-20 764 target_complete_cmd(ec_cmd, SAM_STAT_GOOD);
cbf031f425fd0b Nicholas Bellinger 2013-08-20 765 return;
cbf031f425fd0b Nicholas Bellinger 2013-08-20 766
cbf031f425fd0b Nicholas Bellinger 2013-08-20 767 out:
0394b5048efd73 Sergey Samoylenko 2021-08-03 768 /*
0394b5048efd73 Sergey Samoylenko 2021-08-03 769 * The XCOPY command was aborted after some data was transferred.
0394b5048efd73 Sergey Samoylenko 2021-08-03 770 * Terminate command with CHECK CONDITION status, with the sense key
0394b5048efd73 Sergey Samoylenko 2021-08-03 771 * set to COPY ABORTED.
0394b5048efd73 Sergey Samoylenko 2021-08-03 772 */
0394b5048efd73 Sergey Samoylenko 2021-08-03 773 sense_rc = TCM_COPY_TARGET_DEVICE_NOT_REACHABLE;
cbf031f425fd0b Nicholas Bellinger 2013-08-20 774 xcopy_pt_undepend_remotedev(xop);
0ad08996da05b6 David Disseldorp 2020-03-27 775 target_free_sgl(xop->xop_data_sg, xop->xop_data_nents);
d877d7275be34a Bart Van Assche 2017-05-23 776
d877d7275be34a Bart Van Assche 2017-05-23 777 err_free:
cbf031f425fd0b Nicholas Bellinger 2013-08-20 778 kfree(xop);
0394b5048efd73 Sergey Samoylenko 2021-08-03 779 pr_warn_ratelimited("target_xcopy_do_work: rc: %d, sense: %u, XCOPY operation failed\n",
0394b5048efd73 Sergey Samoylenko 2021-08-03 780 rc, sense_rc);
0394b5048efd73 Sergey Samoylenko 2021-08-03 781 target_complete_cmd_with_sense(ec_cmd, SAM_STAT_CHECK_CONDITION, sense_rc);
cbf031f425fd0b Nicholas Bellinger 2013-08-20 782 }
cbf031f425fd0b Nicholas Bellinger 2013-08-20 783

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]

2022-03-15 08:35:38

by Byungchul Park

[permalink] [raw]
Subject: Re: [PATCH v4 00/24] DEPT(Dependency Tracker)

On Sat, Mar 12, 2022 at 01:53:26AM +0000, Hyeonggon Yoo wrote:
> On Fri, Mar 04, 2022 at 04:06:19PM +0900, Byungchul Park wrote:
> > Hi Linus and folks,
> >
> > I've been developing a tool for detecting deadlock possibilities by
> > tracking wait/event rather than lock(?) acquisition order to try to
> > cover all synchonization machanisms. It's done on v5.17-rc1 tag.
> >
> > https://github.com/lgebyungchulpark/linux-dept/commits/dept1.14_on_v5.17-rc1
> >
>
> Small feedback unrelated to thread:
> I'm not sure "Need to expand the ring buffer" is something to call
> WARN(). Is this stack trace useful for something?

Yeah. It seems to happen too often. I won't warn it. Thanks.

> ========
>
> Hello Byungchul. These are two warnings of DEPT on system.
> Both cases look similar.
>
> In what case DEPT says (unknown)?
> I'm not sure we can properly debug this.
>
> ===================================================
> DEPT: Circular dependency has been detected.
> 5.17.0-rc1+ #3 Tainted: G W
> ---------------------------------------------------
> summary
> ---------------------------------------------------
> *** AA DEADLOCK ***
>
> context A
> [S] (unknown)(&vfork:0)
> [W] wait_for_completion_killable(&vfork:0)
> [E] complete(&vfork:0)

All the reports look like having to do with kernel_clone(). I need to
check it more. Thank you very much.

You are awesome, Hyeonggon.

Thank you,
Byungchul

> [S]: start of the event context
> [W]: the wait blocked
> [E]: the event not reachable
> ---------------------------------------------------
> context A's detail
> ---------------------------------------------------
> context A
> [S] (unknown)(&vfork:0)
> [W] wait_for_completion_killable(&vfork:0)
> [E] complete(&vfork:0)
>
> [S] (unknown)(&vfork:0):
> (N/A)
>
> [W] wait_for_completion_killable(&vfork:0):
> [<ffffffc00802204c>] kernel_clone+0x25c/0x2b8
> stacktrace:
> dept_wait+0x74/0x88
> wait_for_completion_killable+0x60/0xa0
> kernel_clone+0x25c/0x2b8
> __do_sys_clone+0x5c/0x74
> __arm64_sys_clone+0x18/0x20
> invoke_syscall.constprop.0+0x78/0xc4
> do_el0_svc+0x98/0xd0
> el0_svc+0x44/0xe4
> el0t_64_sync_handler+0xb0/0x12c
> el0t_64_sync+0x158/0x15c
>
> [E] complete(&vfork:0):
> [<ffffffc00801f49c>] mm_release+0x7c/0x90
> stacktrace:
> dept_event+0xe0/0x100
> complete+0x48/0x98
> mm_release+0x7c/0x90
> exit_mm_release+0xc/0x14
> do_exit+0x1b4/0x81c
> do_group_exit+0x30/0x9c
> __wake_up_parent+0x0/0x24
> invoke_syscall.constprop.0+0x78/0xc4
> do_el0_svc+0x98/0xd0
> el0_svc+0x44/0xe4
> el0t_64_sync_handler+0xb0/0x12c
> el0t_64_sync+0x158/0x15c
> ---------------------------------------------------
> information that might be helpful
> ---------------------------------------------------
> CPU: 6 PID: 229 Comm: start-stop-daem Tainted: G W 5.17.0-rc1+ #3
> Hardware name: linux,dummy-virt (DT)
> Call trace:
> dump_backtrace.part.0+0x9c/0xc4
> show_stack+0x14/0x28
> dump_stack_lvl+0x9c/0xcc
> dump_stack+0x14/0x2c
> print_circle+0x2d4/0x438
> cb_check_dl+0x44/0x70
> bfs+0x60/0x168
> add_dep+0x88/0x11c
> do_event.constprop.0+0x19c/0x2c0
> dept_event+0xe0/0x100
> complete+0x48/0x98
> mm_release+0x7c/0x90
> exit_mm_release+0xc/0x14
> do_exit+0x1b4/0x81c
> do_group_exit+0x30/0x9c
> __wake_up_parent+0x0/0x24
> invoke_syscall.constprop.0+0x78/0xc4
> do_el0_svc+0x98/0xd0
> el0_svc+0x44/0xe4
> el0t_64_sync_handler+0xb0/0x12c
> el0t_64_sync+0x158/0x15c
>
>
>
>
> ===================================================
> DEPT: Circular dependency has been detected.
> 5.17.0-rc1+ #3 Tainted: G W
> ---------------------------------------------------
> summary
> ---------------------------------------------------
> *** AA DEADLOCK ***
>
> context A
> [S] (unknown)(&try_completion:0)
> [W] wait_for_completion_timeout(&try_completion:0)
> [E] complete(&try_completion:0)
>
> [S]: start of the event context
> [W]: the wait blocked
> [E]: the event not reachable
> ---------------------------------------------------
> context A's detail
> ---------------------------------------------------
> context A
> [S] (unknown)(&try_completion:0)
> [W] wait_for_completion_timeout(&try_completion:0)
> [E] complete(&try_completion:0)
>
> [S] (unknown)(&try_completion:0):
> (N/A)
>
> [W] wait_for_completion_timeout(&try_completion:0):
> [<ffffffc008166bf4>] kunit_try_catch_run+0xb4/0x160
> stacktrace:
> dept_wait+0x74/0x88
> wait_for_completion_timeout+0x64/0xa0
> kunit_try_catch_run+0xb4/0x160
> kunit_test_try_catch_successful_try_no_catch+0x3c/0x98
> kunit_try_run_case+0x9c/0xa0
> kunit_generic_run_threadfn_adapter+0x1c/0x28
> kthread+0xd4/0xe4
> ret_from_fork+0x10/0x20
>
> [E] complete(&try_completion:0):
> [<ffffffc00803dce4>] kthread_complete_and_exit+0x18/0x20
> stacktrace:
> dept_event+0xe0/0x100
> complete+0x48/0x98
> kthread_complete_and_exit+0x18/0x20
> kunit_try_catch_throw+0x0/0x1c
> kthread+0xd4/0xe4
> ret_from_fork+0x10/0x20
>
> ---------------------------------------------------
> information that might be helpful
> ---------------------------------------------------
> CPU: 15 PID: 132 Comm: kunit_try_catch Tainted: G W 5.17.0-rc1+ #3
> Hardware name: linux,dummy-virt (DT)
> Call trace:
> dump_backtrace.part.0+0x9c/0xc4
> show_stack+0x14/0x28
> dump_stack_lvl+0x9c/0xcc
> dump_stack+0x14/0x2c
> print_circle+0x2d4/0x438
> cb_check_dl+0x44/0x70
> bfs+0x60/0x168
> add_dep+0x88/0x11c
> do_event.constprop.0+0x19c/0x2c0
> dept_event+0xe0/0x100
> complete+0x48/0x98
> kthread_complete_and_exit+0x18/0x20
> kunit_try_catch_throw+0x0/0x1c
> kthread+0xd4/0xe4
> ret_from_fork+0x10/0x20


> --
> Thank you, You are awesome!
> Hyeonggon :-)

2022-03-17 03:32:30

by Hyeonggon Yoo

[permalink] [raw]
Subject: Re: [PATCH v4 00/24] DEPT(Dependency Tracker)

On Wed, Mar 16, 2022 at 01:32:13PM +0900, Byungchul Park wrote:
> On Sat, Mar 12, 2022 at 01:53:26AM +0000, Hyeonggon Yoo wrote:
> > On Fri, Mar 04, 2022 at 04:06:19PM +0900, Byungchul Park wrote:
> > > Hi Linus and folks,
> > >
> > > I've been developing a tool for detecting deadlock possibilities by
> > > tracking wait/event rather than lock(?) acquisition order to try to
> > > cover all synchonization machanisms. It's done on v5.17-rc1 tag.
> > >
> > > https://github.com/lgebyungchulpark/linux-dept/commits/dept1.14_on_v5.17-rc1
> > >
> >
> > Small feedback unrelated to thread:
> > I'm not sure "Need to expand the ring buffer" is something to call
> > WARN(). Is this stack trace useful for something?
> > ========
> >
> > Hello Byungchul. These are two warnings of DEPT on system.
>
> Hi Hyeonggon,
>
> Could you run scripts/decode_stacktrace.sh and share the result instead
> of the raw format below if the reports still appear with PATCH v5? It'd
> be appreciated (:
>

Hi Byungchul.

on dept1.18_on_v5.17-rc7, the kernel_clone() warning has gone.
There is one warning remaining on my system:

It warns when running kunit-try-catch-test testcase.

===================================================
DEPT: Circular dependency has been detected.
5.17.0-rc7+ #4 Not tainted
---------------------------------------------------
summary
---------------------------------------------------
*** AA DEADLOCK ***

context A
[S] (unknown)(&try_completion:0)
[W] wait_for_completion_timeout(&try_completion:0)
[E] complete(&try_completion:0)

[S]: start of the event context
[W]: the wait blocked
[E]: the event not reachable
---------------------------------------------------
context A's detail
---------------------------------------------------
context A
[S] (unknown)(&try_completion:0)
[W] wait_for_completion_timeout(&try_completion:0)
[E] complete(&try_completion:0)

[S] (unknown)(&try_completion:0):
(N/A)

[W] wait_for_completion_timeout(&try_completion:0):
kunit_try_catch_run (lib/kunit/try-catch.c:78 (discriminator 1))
stacktrace:
dept_wait (kernel/dependency/dept.c:2149)
wait_for_completion_timeout (kernel/sched/completion.c:119 (discriminator 4) kernel/sched/completion.c:165 (discriminator 4))
kunit_try_catch_run (lib/kunit/try-catch.c:78 (discriminator 1))
kunit_test_try_catch_successful_try_no_catch (lib/kunit/kunit-test.c:43)
kunit_try_run_case (lib/kunit/test.c:333 lib/kunit/test.c:374)
kunit_generic_run_threadfn_adapter (lib/kunit/try-catch.c:30)
kthread (kernel/kthread.c:379)
ret_from_fork (arch/arm64/kernel/entry.S:757)

[E] complete(&try_completion:0):
kthread_complete_and_exit (kernel/kthread.c:327)
stacktrace:
dept_event (kernel/dependency/dept.c:2376 (discriminator 2))
complete (kernel/sched/completion.c:33 (discriminator 4))
kthread_complete_and_exit (kernel/kthread.c:327)
kunit_try_catch_throw (lib/kunit/try-catch.c:18)
kthread (kernel/kthread.c:379)
ret_from_fork (arch/arm64/kernel/entry.S:757)

---------------------------------------------------
information that might be helpful
---------------------------------------------------
Hardware name: linux,dummy-virt (DT)
Call trace:
dump_backtrace.part.0 (arch/arm64/kernel/stacktrace.c:186)
show_stack (arch/arm64/kernel/stacktrace.c:193)
dump_stack_lvl (lib/dump_stack.c:107 (discriminator 4))
dump_stack (lib/dump_stack.c:114)
print_circle (./arch/arm64/include/asm/atomic_ll_sc.h:112 ./arch/arm64/include/asm/atomic.h:30 ./include/linux/atomic/atomic-arch-fallback.h:511 ./include/linux/atomic/atomic-instrumented.h:258 kernel/dependency/dept.c:140 kernel/dependency/dept.c:748)
cb_check_dl (kernel/dependency/dept.c:1083 kernel/dependency/dept.c:1064)
bfs (kernel/dependency/dept.c:833)
add_dep (kernel/dependency/dept.c:1409)
do_event (kernel/dependency/dept.c:175 kernel/dependency/dept.c:1644)
dept_event (kernel/dependency/dept.c:2376 (discriminator 2))
complete (kernel/sched/completion.c:33 (discriminator 4))
kthread_complete_and_exit (kernel/kthread.c:327)
kunit_try_catch_throw (lib/kunit/try-catch.c:18)
kthread (kernel/kthread.c:379)
ret_from_fork (arch/arm64/kernel/entry.S:757)

--
Thank you, You are awesome!
Hyeonggon :-)

> https://lkml.org/lkml/2022/3/15/1277
> (or https://github.com/lgebyungchulpark/linux-dept/commits/dept1.18_on_v5.17-rc7)
>
> Thank you very much!
>
> --
> Byungchul
>
> > Both cases look similar.
> >
> > In what case DEPT says (unknown)?
> > I'm not sure we can properly debug this.
> >
> > ===================================================
> > DEPT: Circular dependency has been detected.
> > 5.17.0-rc1+ #3 Tainted: G W
> > ---------------------------------------------------
> > summary
> > ---------------------------------------------------
> > *** AA DEADLOCK ***
> >
> > context A
> > [S] (unknown)(&vfork:0)
> > [W] wait_for_completion_killable(&vfork:0)
> > [E] complete(&vfork:0)
> >
> > [S]: start of the event context
> > [W]: the wait blocked
> > [E]: the event not reachable
> > ---------------------------------------------------
> > context A's detail
> > ---------------------------------------------------
> > context A
> > [S] (unknown)(&vfork:0)
> > [W] wait_for_completion_killable(&vfork:0)
> > [E] complete(&vfork:0)
> >
> > [S] (unknown)(&vfork:0):
> > (N/A)
> >
> > [W] wait_for_completion_killable(&vfork:0):
> > [<ffffffc00802204c>] kernel_clone+0x25c/0x2b8
> > stacktrace:
> > dept_wait+0x74/0x88
> > wait_for_completion_killable+0x60/0xa0
> > kernel_clone+0x25c/0x2b8
> > __do_sys_clone+0x5c/0x74
> > __arm64_sys_clone+0x18/0x20
> > invoke_syscall.constprop.0+0x78/0xc4
> > do_el0_svc+0x98/0xd0
> > el0_svc+0x44/0xe4
> > el0t_64_sync_handler+0xb0/0x12c
> > el0t_64_sync+0x158/0x15c
> >
> > [E] complete(&vfork:0):
> > [<ffffffc00801f49c>] mm_release+0x7c/0x90
> > stacktrace:
> > dept_event+0xe0/0x100
> > complete+0x48/0x98
> > mm_release+0x7c/0x90
> > exit_mm_release+0xc/0x14
> > do_exit+0x1b4/0x81c
> > do_group_exit+0x30/0x9c
> > __wake_up_parent+0x0/0x24
> > invoke_syscall.constprop.0+0x78/0xc4
> > do_el0_svc+0x98/0xd0
> > el0_svc+0x44/0xe4
> > el0t_64_sync_handler+0xb0/0x12c
> > el0t_64_sync+0x158/0x15c
> > ---------------------------------------------------
> > information that might be helpful
> > ---------------------------------------------------
> > CPU: 6 PID: 229 Comm: start-stop-daem Tainted: G W 5.17.0-rc1+ #3
> > Hardware name: linux,dummy-virt (DT)
> > Call trace:
> > dump_backtrace.part.0+0x9c/0xc4
> > show_stack+0x14/0x28
> > dump_stack_lvl+0x9c/0xcc
> > dump_stack+0x14/0x2c
> > print_circle+0x2d4/0x438
> > cb_check_dl+0x44/0x70
> > bfs+0x60/0x168
> > add_dep+0x88/0x11c
> > do_event.constprop.0+0x19c/0x2c0
> > dept_event+0xe0/0x100
> > complete+0x48/0x98
> > mm_release+0x7c/0x90
> > exit_mm_release+0xc/0x14
> > do_exit+0x1b4/0x81c
> > do_group_exit+0x30/0x9c
> > __wake_up_parent+0x0/0x24
> > invoke_syscall.constprop.0+0x78/0xc4
> > do_el0_svc+0x98/0xd0
> > el0_svc+0x44/0xe4
> > el0t_64_sync_handler+0xb0/0x12c
> > el0t_64_sync+0x158/0x15c
> >
> >
> >
> >
> > ===================================================
> > DEPT: Circular dependency has been detected.
> > 5.17.0-rc1+ #3 Tainted: G W
> > ---------------------------------------------------
> > summary
> > ---------------------------------------------------
> > *** AA DEADLOCK ***
> >
> > context A
> > [S] (unknown)(&try_completion:0)
> > [W] wait_for_completion_timeout(&try_completion:0)
> > [E] complete(&try_completion:0)
> >
> > [S]: start of the event context
> > [W]: the wait blocked
> > [E]: the event not reachable
> > ---------------------------------------------------
> > context A's detail
> > ---------------------------------------------------
> > context A
> > [S] (unknown)(&try_completion:0)
> > [W] wait_for_completion_timeout(&try_completion:0)
> > [E] complete(&try_completion:0)
> >
> > [S] (unknown)(&try_completion:0):
> > (N/A)
> >
> > [W] wait_for_completion_timeout(&try_completion:0):
> > [<ffffffc008166bf4>] kunit_try_catch_run+0xb4/0x160
> > stacktrace:
> > dept_wait+0x74/0x88
> > wait_for_completion_timeout+0x64/0xa0
> > kunit_try_catch_run+0xb4/0x160
> > kunit_test_try_catch_successful_try_no_catch+0x3c/0x98
> > kunit_try_run_case+0x9c/0xa0
> > kunit_generic_run_threadfn_adapter+0x1c/0x28
> > kthread+0xd4/0xe4
> > ret_from_fork+0x10/0x20
> >
> > [E] complete(&try_completion:0):
> > [<ffffffc00803dce4>] kthread_complete_and_exit+0x18/0x20
> > stacktrace:
> > dept_event+0xe0/0x100
> > complete+0x48/0x98
> > kthread_complete_and_exit+0x18/0x20
> > kunit_try_catch_throw+0x0/0x1c
> > kthread+0xd4/0xe4
> > ret_from_fork+0x10/0x20
> >
> > ---------------------------------------------------
> > information that might be helpful
> > ---------------------------------------------------
> > CPU: 15 PID: 132 Comm: kunit_try_catch Tainted: G W 5.17.0-rc1+ #3
> > Hardware name: linux,dummy-virt (DT)
> > Call trace:
> > dump_backtrace.part.0+0x9c/0xc4
> > show_stack+0x14/0x28
> > dump_stack_lvl+0x9c/0xcc
> > dump_stack+0x14/0x2c
> > print_circle+0x2d4/0x438
> > cb_check_dl+0x44/0x70
> > bfs+0x60/0x168
> > add_dep+0x88/0x11c
> > do_event.constprop.0+0x19c/0x2c0
> > dept_event+0xe0/0x100
> > complete+0x48/0x98
> > kthread_complete_and_exit+0x18/0x20
> > kunit_try_catch_throw+0x0/0x1c
> > kthread+0xd4/0xe4
> > ret_from_fork+0x10/0x20
> >
> >
> > > Benifit:
> > >
> > > 0. Works with all lock primitives.
> > > 1. Works with wait_for_completion()/complete().
> > > 2. Works with 'wait' on PG_locked.
> > > 3. Works with 'wait' on PG_writeback.
> > > 4. Works with swait/wakeup.
> > > 5. Works with waitqueue.
> > > 6. Multiple reports are allowed.
> > > 7. Deduplication control on multiple reports.
> > > 8. Withstand false positives thanks to 6.
> > > 9. Easy to tag any wait/event.
> > >
> > > Future work:
> >
> > [...]
> >
> > > --
> > > 1.9.1
> > >
> >
> > --
> > Thank you, You are awesome!
> > Hyeonggon :-)

2022-03-21 22:34:13

by Byungchul Park

[permalink] [raw]
Subject: Re: [PATCH v4 00/24] DEPT(Dependency Tracker)

On Fri, Mar 18, 2022 at 04:51:29PM +0900, Byungchul Park wrote:
> On Wed, Mar 16, 2022 at 09:30:02AM +0000, Hyeonggon Yoo wrote:
> > On Wed, Mar 16, 2022 at 01:32:13PM +0900, Byungchul Park wrote:
> > > On Sat, Mar 12, 2022 at 01:53:26AM +0000, Hyeonggon Yoo wrote:
> > > > On Fri, Mar 04, 2022 at 04:06:19PM +0900, Byungchul Park wrote:
> > > > > Hi Linus and folks,
> > > > >
> > > > > I've been developing a tool for detecting deadlock possibilities by
> > > > > tracking wait/event rather than lock(?) acquisition order to try to
> > > > > cover all synchonization machanisms. It's done on v5.17-rc1 tag.
> > > > >
> > > > > https://github.com/lgebyungchulpark/linux-dept/commits/dept1.14_on_v5.17-rc1
> > > > >
> > > >
> > > > Small feedback unrelated to thread:
> > > > I'm not sure "Need to expand the ring buffer" is something to call
> > > > WARN(). Is this stack trace useful for something?
> > > > ========
> > > >
> > > > Hello Byungchul. These are two warnings of DEPT on system.
> > >
> > > Hi Hyeonggon,
> > >
> > > Could you run scripts/decode_stacktrace.sh and share the result instead
> > > of the raw format below if the reports still appear with PATCH v5? It'd
> > > be appreciated (:
> > >
> >
> > Hi Byungchul.
> >
> > on dept1.18_on_v5.17-rc7, the kernel_clone() warning has gone.
> > There is one warning remaining on my system:
> >
> > It warns when running kunit-try-catch-test testcase.
>
> Hi Hyeonggon,
>
> I can reproduce it thanks to you. I will let you know on all works done.

Hi Hyeonggon,

All works wrt this issue have been done. I've just updated the same
branch.

https://github.com/lgebyungchulpark/linux-dept/commits/dept1.18_on_v5.17-rc7

This is just for your information.

Thanks,
Byungchul

>
> Thanks,
> Byungchul
>
> > ===================================================
> > DEPT: Circular dependency has been detected.
> > 5.17.0-rc7+ #4 Not tainted
> > ---------------------------------------------------
> > summary
> > ---------------------------------------------------
> > *** AA DEADLOCK ***
> >
> > context A
> > [S] (unknown)(&try_completion:0)
> > [W] wait_for_completion_timeout(&try_completion:0)
> > [E] complete(&try_completion:0)
> >
> > [S]: start of the event context
> > [W]: the wait blocked
> > [E]: the event not reachable
> > ---------------------------------------------------
> > context A's detail
> > ---------------------------------------------------
> > context A
> > [S] (unknown)(&try_completion:0)
> > [W] wait_for_completion_timeout(&try_completion:0)
> > [E] complete(&try_completion:0)
> >
> > [S] (unknown)(&try_completion:0):
> > (N/A)
> >
> > [W] wait_for_completion_timeout(&try_completion:0):
> > kunit_try_catch_run (lib/kunit/try-catch.c:78 (discriminator 1))
> > stacktrace:
> > dept_wait (kernel/dependency/dept.c:2149)
> > wait_for_completion_timeout (kernel/sched/completion.c:119 (discriminator 4) kernel/sched/completion.c:165 (discriminator 4))
> > kunit_try_catch_run (lib/kunit/try-catch.c:78 (discriminator 1))
> > kunit_test_try_catch_successful_try_no_catch (lib/kunit/kunit-test.c:43)
> > kunit_try_run_case (lib/kunit/test.c:333 lib/kunit/test.c:374)
> > kunit_generic_run_threadfn_adapter (lib/kunit/try-catch.c:30)
> > kthread (kernel/kthread.c:379)
> > ret_from_fork (arch/arm64/kernel/entry.S:757)
> >
> > [E] complete(&try_completion:0):
> > kthread_complete_and_exit (kernel/kthread.c:327)
> > stacktrace:
> > dept_event (kernel/dependency/dept.c:2376 (discriminator 2))
> > complete (kernel/sched/completion.c:33 (discriminator 4))
> > kthread_complete_and_exit (kernel/kthread.c:327)
> > kunit_try_catch_throw (lib/kunit/try-catch.c:18)
> > kthread (kernel/kthread.c:379)
> > ret_from_fork (arch/arm64/kernel/entry.S:757)
> >
> > ---------------------------------------------------
> > information that might be helpful
> > ---------------------------------------------------
> > Hardware name: linux,dummy-virt (DT)
> > Call trace:
> > dump_backtrace.part.0 (arch/arm64/kernel/stacktrace.c:186)
> > show_stack (arch/arm64/kernel/stacktrace.c:193)
> > dump_stack_lvl (lib/dump_stack.c:107 (discriminator 4))
> > dump_stack (lib/dump_stack.c:114)
> > print_circle (./arch/arm64/include/asm/atomic_ll_sc.h:112 ./arch/arm64/include/asm/atomic.h:30 ./include/linux/atomic/atomic-arch-fallback.h:511 ./include/linux/atomic/atomic-instrumented.h:258 kernel/dependency/dept.c:140 kernel/dependency/dept.c:748)
> > cb_check_dl (kernel/dependency/dept.c:1083 kernel/dependency/dept.c:1064)
> > bfs (kernel/dependency/dept.c:833)
> > add_dep (kernel/dependency/dept.c:1409)
> > do_event (kernel/dependency/dept.c:175 kernel/dependency/dept.c:1644)
> > dept_event (kernel/dependency/dept.c:2376 (discriminator 2))
> > complete (kernel/sched/completion.c:33 (discriminator 4))
> > kthread_complete_and_exit (kernel/kthread.c:327)
> > kunit_try_catch_throw (lib/kunit/try-catch.c:18)
> > kthread (kernel/kthread.c:379)
> > ret_from_fork (arch/arm64/kernel/entry.S:757)
> >
> > --
> > Thank you, You are awesome!
> > Hyeonggon :-)
> >
> > > https://lkml.org/lkml/2022/3/15/1277
> > > (or https://github.com/lgebyungchulpark/linux-dept/commits/dept1.18_on_v5.17-rc7)
> > >
> > > Thank you very much!
> > >
> > > --
> > > Byungchul
> > >
> > > > Both cases look similar.
> > > >
> > > > In what case DEPT says (unknown)?
> > > > I'm not sure we can properly debug this.
> > > >
> > > > ===================================================
> > > > DEPT: Circular dependency has been detected.
> > > > 5.17.0-rc1+ #3 Tainted: G W
> > > > ---------------------------------------------------
> > > > summary
> > > > ---------------------------------------------------
> > > > *** AA DEADLOCK ***
> > > >
> > > > context A
> > > > [S] (unknown)(&vfork:0)
> > > > [W] wait_for_completion_killable(&vfork:0)
> > > > [E] complete(&vfork:0)
> > > >
> > > > [S]: start of the event context
> > > > [W]: the wait blocked
> > > > [E]: the event not reachable
> > > > ---------------------------------------------------
> > > > context A's detail
> > > > ---------------------------------------------------
> > > > context A
> > > > [S] (unknown)(&vfork:0)
> > > > [W] wait_for_completion_killable(&vfork:0)
> > > > [E] complete(&vfork:0)
> > > >
> > > > [S] (unknown)(&vfork:0):
> > > > (N/A)
> > > >
> > > > [W] wait_for_completion_killable(&vfork:0):
> > > > [<ffffffc00802204c>] kernel_clone+0x25c/0x2b8
> > > > stacktrace:
> > > > dept_wait+0x74/0x88
> > > > wait_for_completion_killable+0x60/0xa0
> > > > kernel_clone+0x25c/0x2b8
> > > > __do_sys_clone+0x5c/0x74
> > > > __arm64_sys_clone+0x18/0x20
> > > > invoke_syscall.constprop.0+0x78/0xc4
> > > > do_el0_svc+0x98/0xd0
> > > > el0_svc+0x44/0xe4
> > > > el0t_64_sync_handler+0xb0/0x12c
> > > > el0t_64_sync+0x158/0x15c
> > > >
> > > > [E] complete(&vfork:0):
> > > > [<ffffffc00801f49c>] mm_release+0x7c/0x90
> > > > stacktrace:
> > > > dept_event+0xe0/0x100
> > > > complete+0x48/0x98
> > > > mm_release+0x7c/0x90
> > > > exit_mm_release+0xc/0x14
> > > > do_exit+0x1b4/0x81c
> > > > do_group_exit+0x30/0x9c
> > > > __wake_up_parent+0x0/0x24
> > > > invoke_syscall.constprop.0+0x78/0xc4
> > > > do_el0_svc+0x98/0xd0
> > > > el0_svc+0x44/0xe4
> > > > el0t_64_sync_handler+0xb0/0x12c
> > > > el0t_64_sync+0x158/0x15c
> > > > ---------------------------------------------------
> > > > information that might be helpful
> > > > ---------------------------------------------------
> > > > CPU: 6 PID: 229 Comm: start-stop-daem Tainted: G W 5.17.0-rc1+ #3
> > > > Hardware name: linux,dummy-virt (DT)
> > > > Call trace:
> > > > dump_backtrace.part.0+0x9c/0xc4
> > > > show_stack+0x14/0x28
> > > > dump_stack_lvl+0x9c/0xcc
> > > > dump_stack+0x14/0x2c
> > > > print_circle+0x2d4/0x438
> > > > cb_check_dl+0x44/0x70
> > > > bfs+0x60/0x168
> > > > add_dep+0x88/0x11c
> > > > do_event.constprop.0+0x19c/0x2c0
> > > > dept_event+0xe0/0x100
> > > > complete+0x48/0x98
> > > > mm_release+0x7c/0x90
> > > > exit_mm_release+0xc/0x14
> > > > do_exit+0x1b4/0x81c
> > > > do_group_exit+0x30/0x9c
> > > > __wake_up_parent+0x0/0x24
> > > > invoke_syscall.constprop.0+0x78/0xc4
> > > > do_el0_svc+0x98/0xd0
> > > > el0_svc+0x44/0xe4
> > > > el0t_64_sync_handler+0xb0/0x12c
> > > > el0t_64_sync+0x158/0x15c
> > > >
> > > >
> > > >
> > > >
> > > > ===================================================
> > > > DEPT: Circular dependency has been detected.
> > > > 5.17.0-rc1+ #3 Tainted: G W
> > > > ---------------------------------------------------
> > > > summary
> > > > ---------------------------------------------------
> > > > *** AA DEADLOCK ***
> > > >
> > > > context A
> > > > [S] (unknown)(&try_completion:0)
> > > > [W] wait_for_completion_timeout(&try_completion:0)
> > > > [E] complete(&try_completion:0)
> > > >
> > > > [S]: start of the event context
> > > > [W]: the wait blocked
> > > > [E]: the event not reachable
> > > > ---------------------------------------------------
> > > > context A's detail
> > > > ---------------------------------------------------
> > > > context A
> > > > [S] (unknown)(&try_completion:0)
> > > > [W] wait_for_completion_timeout(&try_completion:0)
> > > > [E] complete(&try_completion:0)
> > > >
> > > > [S] (unknown)(&try_completion:0):
> > > > (N/A)
> > > >
> > > > [W] wait_for_completion_timeout(&try_completion:0):
> > > > [<ffffffc008166bf4>] kunit_try_catch_run+0xb4/0x160
> > > > stacktrace:
> > > > dept_wait+0x74/0x88
> > > > wait_for_completion_timeout+0x64/0xa0
> > > > kunit_try_catch_run+0xb4/0x160
> > > > kunit_test_try_catch_successful_try_no_catch+0x3c/0x98
> > > > kunit_try_run_case+0x9c/0xa0
> > > > kunit_generic_run_threadfn_adapter+0x1c/0x28
> > > > kthread+0xd4/0xe4
> > > > ret_from_fork+0x10/0x20
> > > >
> > > > [E] complete(&try_completion:0):
> > > > [<ffffffc00803dce4>] kthread_complete_and_exit+0x18/0x20
> > > > stacktrace:
> > > > dept_event+0xe0/0x100
> > > > complete+0x48/0x98
> > > > kthread_complete_and_exit+0x18/0x20
> > > > kunit_try_catch_throw+0x0/0x1c
> > > > kthread+0xd4/0xe4
> > > > ret_from_fork+0x10/0x20
> > > >
> > > > ---------------------------------------------------
> > > > information that might be helpful
> > > > ---------------------------------------------------
> > > > CPU: 15 PID: 132 Comm: kunit_try_catch Tainted: G W 5.17.0-rc1+ #3
> > > > Hardware name: linux,dummy-virt (DT)
> > > > Call trace:
> > > > dump_backtrace.part.0+0x9c/0xc4
> > > > show_stack+0x14/0x28
> > > > dump_stack_lvl+0x9c/0xcc
> > > > dump_stack+0x14/0x2c
> > > > print_circle+0x2d4/0x438
> > > > cb_check_dl+0x44/0x70
> > > > bfs+0x60/0x168
> > > > add_dep+0x88/0x11c
> > > > do_event.constprop.0+0x19c/0x2c0
> > > > dept_event+0xe0/0x100
> > > > complete+0x48/0x98
> > > > kthread_complete_and_exit+0x18/0x20
> > > > kunit_try_catch_throw+0x0/0x1c
> > > > kthread+0xd4/0xe4
> > > > ret_from_fork+0x10/0x20
> > > >
> > > >
> > > > > Benifit:
> > > > >
> > > > > 0. Works with all lock primitives.
> > > > > 1. Works with wait_for_completion()/complete().
> > > > > 2. Works with 'wait' on PG_locked.
> > > > > 3. Works with 'wait' on PG_writeback.
> > > > > 4. Works with swait/wakeup.
> > > > > 5. Works with waitqueue.
> > > > > 6. Multiple reports are allowed.
> > > > > 7. Deduplication control on multiple reports.
> > > > > 8. Withstand false positives thanks to 6.
> > > > > 9. Easy to tag any wait/event.
> > > > >
> > > > > Future work:
> > > >
> > > > [...]
> > > >
> > > > > --
> > > > > 1.9.1
> > > > >
> > > >
> > > > --
> > > > Thank you, You are awesome!
> > > > Hyeonggon :-)