Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

locking/lockdep: Fix the dep path printing for backwards BFS

We use the same code to print backwards lock dependency path as the
forwards lock dependency path, and this could result into incorrect
printing because for a backwards lock_list ->trace is not the call trace
where the lock of ->class is acquired.

Fix this by introducing a separate function on printing the backwards
dependency path. Also add a few comments about the printing while we are
at it.

Reported-by: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lore.kernel.org/r/20210618170110.3699115-2-boqun.feng@gmail.com

authored by

Boqun Feng and committed by
Peter Zijlstra
69c7a5fb 7cb5dd8e

+106 -2
+106 -2
kernel/locking/lockdep.c
··· 2304 2304 } 2305 2305 2306 2306 /* 2307 - * printk the shortest lock dependencies from @start to @end in reverse order: 2307 + * Dependency path printing: 2308 + * 2309 + * After BFS we get a lock dependency path (linked via ->parent of lock_list), 2310 + * printing out each lock in the dependency path will help on understanding how 2311 + * the deadlock could happen. Here are some details about dependency path 2312 + * printing: 2313 + * 2314 + * 1) A lock_list can be either forwards or backwards for a lock dependency, 2315 + * for a lock dependency A -> B, there are two lock_lists: 2316 + * 2317 + * a) lock_list in the ->locks_after list of A, whose ->class is B and 2318 + * ->links_to is A. In this case, we can say the lock_list is 2319 + * "A -> B" (forwards case). 2320 + * 2321 + * b) lock_list in the ->locks_before list of B, whose ->class is A 2322 + * and ->links_to is B. In this case, we can say the lock_list is 2323 + * "B <- A" (bacwards case). 2324 + * 2325 + * The ->trace of both a) and b) point to the call trace where B was 2326 + * acquired with A held. 2327 + * 2328 + * 2) A "helper" lock_list is introduced during BFS, this lock_list doesn't 2329 + * represent a certain lock dependency, it only provides an initial entry 2330 + * for BFS. For example, BFS may introduce a "helper" lock_list whose 2331 + * ->class is A, as a result BFS will search all dependencies starting with 2332 + * A, e.g. A -> B or A -> C. 2333 + * 2334 + * The notation of a forwards helper lock_list is like "-> A", which means 2335 + * we should search the forwards dependencies starting with "A", e.g A -> B 2336 + * or A -> C. 2337 + * 2338 + * The notation of a bacwards helper lock_list is like "<- B", which means 2339 + * we should search the backwards dependencies ending with "B", e.g. 2340 + * B <- A or B <- C. 2341 + */ 2342 + 2343 + /* 2344 + * printk the shortest lock dependencies from @root to @leaf in reverse order. 2345 + * 2346 + * We have a lock dependency path as follow: 2347 + * 2348 + * @root @leaf 2349 + * | | 2350 + * V V 2351 + * ->parent ->parent 2352 + * | lock_list | <--------- | lock_list | ... | lock_list | <--------- | lock_list | 2353 + * | -> L1 | | L1 -> L2 | ... |Ln-2 -> Ln-1| | Ln-1 -> Ln| 2354 + * 2355 + * , so it's natural that we start from @leaf and print every ->class and 2356 + * ->trace until we reach the @root. 2308 2357 */ 2309 2358 static void __used 2310 2359 print_shortest_lock_dependencies(struct lock_list *leaf, ··· 2370 2321 printk("%*s ... acquired at:\n", depth, ""); 2371 2322 print_lock_trace(entry->trace, 2); 2372 2323 printk("\n"); 2324 + 2325 + if (depth == 0 && (entry != root)) { 2326 + printk("lockdep:%s bad path found in chain graph\n", __func__); 2327 + break; 2328 + } 2329 + 2330 + entry = get_lock_parent(entry); 2331 + depth--; 2332 + } while (entry && (depth >= 0)); 2333 + } 2334 + 2335 + /* 2336 + * printk the shortest lock dependencies from @leaf to @root. 2337 + * 2338 + * We have a lock dependency path (from a backwards search) as follow: 2339 + * 2340 + * @leaf @root 2341 + * | | 2342 + * V V 2343 + * ->parent ->parent 2344 + * | lock_list | ---------> | lock_list | ... | lock_list | ---------> | lock_list | 2345 + * | L2 <- L1 | | L3 <- L2 | ... | Ln <- Ln-1 | | <- Ln | 2346 + * 2347 + * , so when we iterate from @leaf to @root, we actually print the lock 2348 + * dependency path L1 -> L2 -> .. -> Ln in the non-reverse order. 2349 + * 2350 + * Another thing to notice here is that ->class of L2 <- L1 is L1, while the 2351 + * ->trace of L2 <- L1 is the call trace of L2, in fact we don't have the call 2352 + * trace of L1 in the dependency path, which is alright, because most of the 2353 + * time we can figure out where L1 is held from the call trace of L2. 2354 + */ 2355 + static void __used 2356 + print_shortest_lock_dependencies_backwards(struct lock_list *leaf, 2357 + struct lock_list *root) 2358 + { 2359 + struct lock_list *entry = leaf; 2360 + const struct lock_trace *trace = NULL; 2361 + int depth; 2362 + 2363 + /*compute depth from generated tree by BFS*/ 2364 + depth = get_lock_depth(leaf); 2365 + 2366 + do { 2367 + print_lock_class_header(entry->class, depth); 2368 + if (trace) { 2369 + printk("%*s ... acquired at:\n", depth, ""); 2370 + print_lock_trace(trace, 2); 2371 + printk("\n"); 2372 + } 2373 + 2374 + /* 2375 + * Record the pointer to the trace for the next lock_list 2376 + * entry, see the comments for the function. 2377 + */ 2378 + trace = entry->trace; 2373 2379 2374 2380 if (depth == 0 && (entry != root)) { 2375 2381 printk("lockdep:%s bad path found in chain graph\n", __func__); ··· 2553 2449 prev_root->trace = save_trace(); 2554 2450 if (!prev_root->trace) 2555 2451 return; 2556 - print_shortest_lock_dependencies(backwards_entry, prev_root); 2452 + print_shortest_lock_dependencies_backwards(backwards_entry, prev_root); 2557 2453 2558 2454 pr_warn("\nthe dependencies between the lock to be acquired"); 2559 2455 pr_warn(" and %s-irq-unsafe lock:\n", irqclass);