Commitd61fc96

fbq

authored and

Peter Zijlstra

committed

lockdep: Avoid to modify chain keys in validate_chain()

Chris Wilson reported a problem spotted by check_chain_key(): a chainkey got changed in validate_chain() because we modify the ->read invalidate_chain() to skip checks for dependency adding, and ->read istaken into calculation for chain key since commitf611e8c("lockdep: Take read/write status in consideration when generatechainkey").Fix this by avoiding to modify ->read in validate_chain() based on twofacts: a) since we now support recursive read lock detection, there isno need to skip checks for dependency adding for recursive readers, b)since we have a), there is only one case left (nest_lock) where we wantto skip checks in validate_chain(), we simply remove the modificationfor ->read and rely on the return value of check_deadlock() to skip thedependency adding.Reported-by: Chris Wilson <chris@chris-wilson.co.uk>Signed-off-by: Boqun Feng <boqun.feng@gmail.com>Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>Link:https://lkml.kernel.org/r/20201102053743.450459-1-boqun.feng@gmail.com

1 parent1e106aa commitd61fc96Copy full SHA for d61fc96

File tree

1 file changed

-10

lines changed

kernel/locking
- lockdep.c

1 file changed

-10

lines changed

`‎kernel/locking/lockdep.c‎`

Lines changed: 9 additions & 10 deletions

Original file line number	Diff line number	Diff line change
`@@ -2765,7 +2765,9 @@ print_deadlock_bug(struct task_struct curr, struct held_lock prev,`
`2765`	`2765`	`* (Note that this has to be done separately, because the graph cannot`
`2766`	`2766`	`* detect such classes of deadlocks.)`
`2767`	`2767`	`*`
`2768`		`- * Returns: 0 on deadlock detected, 1 on OK, 2 on recursive read`
	`2768`	`+ * Returns: 0 on deadlock detected, 1 on OK, 2 if another lock with the same`
	`2769`	`+ * lock class is held but nest_lock is also held, i.e. we rely on the`
	`2770`	`+ * nest_lock to avoid the deadlock.`
`2769`	`2771`	`*/`
`2770`	`2772`	`staticint`
`2771`	`2773`	`check_deadlock(structtask_structcurr,structheld_locknext)`
`@@ -2788,7 +2790,7 @@ check_deadlock(struct task_struct curr, struct held_lock next)`
`2788`	`2790`	`* lock class (i.e. read_lock(lock)+read_lock(lock)):`
`2789`	`2791`	`*/`
`2790`	`2792`	`if ((next->read==2)&&prev->read)`
`2791`		`-return2;`
	`2793`	`+continue;`
`2792`	`2794`
`2793`	`2795`	`/*`
`2794`	`2796`	`* We're holding the nest_lock, which serializes this lock's`
`@@ -3592,16 +3594,13 @@ static int validate_chain(struct task_struct *curr,`
`3592`	`3594`
`3593`	`3595`	`if (!ret)`
`3594`	`3596`	`return0;`
`3595`		`-/*`
`3596`		`- * Mark recursive read, as we jump over it when`
`3597`		`- * building dependencies (just like we jump over`
`3598`		`- * trylock entries):`
`3599`		`- */`
`3600`		`-if (ret==2)`
`3601`		`-hlock->read=2;`
`3602`	`3597`	`/*`
`3603`	`3598`	`* Add dependency only if this lock is not the head`
`3604`		`- * of the chain, and if it's not a secondary read-lock:`
	`3599`	`+ * of the chain, and if the new lock introduces no more`
	`3600`	`+ * lock dependency (because we already hold a lock with the`
	`3601`	`+ * same lock class) nor deadlock (because the nest_lock`
	`3602`	`+ * serializes nesting locks), see the comments for`
	`3603`	`+ * check_deadlock().`
`3605`	`3604`	`*/`
`3606`	`3605`	`if (!chain_head&&ret!=2) {`
`3607`	`3606`	`if (!check_prevs_add(curr,hlock))`

0 commit comments

Comments

(0)

Movatterモバイル変換

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Commitd61fc96

File tree

1 file changed

1 file changed

`‎kernel/locking/lockdep.c‎`

0 commit comments