Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1567145

Re: [PATCH v5 05/13] lockdep: Pass a callback arg to check_prev_add() to handle stack_trace

Show all headers | View raw

I fixed a hole that peterz pointed out. And then, I think the following
is reasonable. Don't you think so?

----->8-----
commit ac185d1820ee7223773ec3e23f614c1fe5c079fc
Author: Byungchul Park <byungchul.park@lge.com>
Date:   Tue Jan 24 14:46:14 2017 +0900

    lockdep: Pass a callback arg to check_prev_add() to handle stack_trace
    
    Currently, a separate stack_trace instance cannot be used in
    check_prev_add(). The simplest way to achieve it is to pass a
    stack_trace instance to check_prev_add() as an argument after
    saving it. However, unnecessary saving can happen if so implemented.
    
    The proper solution is to pass a callback function additionally along
    with a stack_trace so that a caller can decide the way to save. Actually,
    crossrelease don't need to save stack_trace of current, but only need to
    copy stack_traces from temporary buffers to the global stack_trace[].
    
    In addition, check_prev_add() returns 2 in case that the lock does not
    need to be added into the dependency graph because it was already in.
    However, the return value is not used any more. So, this patch changes
    it to mean that lockdep successfully save stack_trace and add the lock
    to the graph.
    
    Signed-off-by: Byungchul Park <byungchul.park@lge.com>

diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 7fe6af1..9562b29 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -1805,20 +1805,13 @@ static inline void inc_chains(void)
  */
 static int
 check_prev_add(struct task_struct *curr, struct held_lock *prev,
-	       struct held_lock *next, int distance, int *stack_saved)
+	       struct held_lock *next, int distance, struct stack_trace *trace,
+	       int (*save)(struct stack_trace *trace))
 {
 	struct lock_list *entry;
 	int ret;
 	struct lock_list this;
 	struct lock_list *uninitialized_var(target_entry);
-	/*
-	 * Static variable, serialized by the graph_lock().
-	 *
-	 * We use this static variable to save the stack trace in case
-	 * we call into this function multiple times due to encountering
-	 * trylocks in the held lock stack.
-	 */
-	static struct stack_trace trace;
 
 	/*
 	 * Prove that the new <prev> -> <next> dependency would not
@@ -1862,15 +1855,12 @@ static inline void inc_chains(void)
 		if (entry->class == hlock_class(next)) {
 			if (distance == 1)
 				entry->distance = 1;
-			return 2;
+			return 1;
 		}
 	}
 
-	if (!*stack_saved) {
-		if (!save_trace(&trace))
-			return 0;
-		*stack_saved = 1;
-	}
+	if (save && !save(trace))
+		return 0;
 
 	/*
 	 * Ok, all validations passed, add the new lock
@@ -1878,14 +1868,14 @@ static inline void inc_chains(void)
 	 */
 	ret = add_lock_to_list(hlock_class(prev), hlock_class(next),
 			       &hlock_class(prev)->locks_after,
-			       next->acquire_ip, distance, &trace);
+			       next->acquire_ip, distance, trace);
 
 	if (!ret)
 		return 0;
 
 	ret = add_lock_to_list(hlock_class(next), hlock_class(prev),
 			       &hlock_class(next)->locks_before,
-			       next->acquire_ip, distance, &trace);
+			       next->acquire_ip, distance, trace);
 	if (!ret)
 		return 0;
 
@@ -1893,8 +1883,6 @@ static inline void inc_chains(void)
 	 * Debugging printouts:
 	 */
 	if (verbose(hlock_class(prev)) || verbose(hlock_class(next))) {
-		/* We drop graph lock, so another thread can overwrite trace. */
-		*stack_saved = 0;
 		graph_unlock();
 		printk("\n new dependency: ");
 		print_lock_name(hlock_class(prev));
@@ -1902,9 +1890,10 @@ static inline void inc_chains(void)
 		print_lock_name(hlock_class(next));
 		printk(KERN_CONT "\n");
 		dump_stack();
-		return graph_lock();
+		if (!graph_lock())
+			return 0;
 	}
-	return 1;
+	return 2;
 }
 
 /*
@@ -1917,8 +1906,9 @@ static inline void inc_chains(void)
 check_prevs_add(struct task_struct *curr, struct held_lock *next)
 {
 	int depth = curr->lockdep_depth;
-	int stack_saved = 0;
 	struct held_lock *hlock;
+	struct stack_trace trace;
+	int (*save)(struct stack_trace *trace) = save_trace;
 
 	/*
 	 * Debugging checks.
@@ -1943,9 +1933,18 @@ static inline void inc_chains(void)
 		 * added:
 		 */
 		if (hlock->read != 2 && hlock->check) {
-			if (!check_prev_add(curr, hlock, next,
-						distance, &stack_saved))
+			int ret = check_prev_add(curr, hlock, next,
+						distance, &trace, save);
+			if (!ret)
 				return 0;
+
+			/*
+			 * Stop saving stack_trace if save_trace() was
+			 * called at least once:
+			 */
+			if (save && ret == 2)
+				save = NULL;
+
 			/*
 			 * Stop after the first non-trylock entry,
 			 * as non-trylock entries have added their

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

[PATCH v5 00/13] lockdep: Implement crossrelease feature Byungchul Park <byungchul.park@lge.com> - 2017-01-18 14:20 +0100
  [PATCH v5 01/13] lockdep: Refactor lookup_chain_cache() Byungchul Park <byungchul.park@lge.com> - 2017-01-18 14:20 +0100
    Re: [PATCH v5 01/13] lockdep: Refactor lookup_chain_cache() Boqun Feng <boqun.feng@gmail.com> - 2017-01-19 10:20 +0100
      Re: [PATCH v5 01/13] lockdep: Refactor lookup_chain_cache() Byungchul Park <byungchul.park@lge.com> - 2017-01-19 11:10 +0100
      Re: [PATCH v5 01/13] lockdep: Refactor lookup_chain_cache() Byungchul Park <byungchul.park@lge.com> - 2017-01-26 09:00 +0100
  [PATCH v5 09/13] pagemap.h: Remove trailing white space Byungchul Park <byungchul.park@lge.com> - 2017-01-18 14:30 +0100
  [PATCH v5 10/13] lockdep: Apply crossrelease to PG_locked locks Byungchul Park <byungchul.park@lge.com> - 2017-01-18 14:30 +0100
  [PATCH v5 13/13] lockdep: Crossrelease feature documentation Byungchul Park <byungchul.park@lge.com> - 2017-01-18 14:30 +0100
    [REVISED DOCUMENT] lockdep: Crossrelease feature documentation Byungchul Park <byungchul.park@lge.com> - 2017-01-20 10:10 +0100
  [PATCH v5 06/13] lockdep: Implement crossrelease feature Byungchul Park <byungchul.park@lge.com> - 2017-01-18 14:30 +0100
  [PATCH v5 08/13] lockdep: Apply crossrelease to completions Byungchul Park <byungchul.park@lge.com> - 2017-01-18 14:30 +0100
  [PATCH v5 11/13] lockdep: Apply lock_acquire(release) on __Set(__Clear)PageLocked Byungchul Park <byungchul.park@lge.com> - 2017-01-18 14:30 +0100
  [PATCH v5 03/13] lockdep: Add a function building a chain between two classes Byungchul Park <byungchul.park@lge.com> - 2017-01-18 14:30 +0100
  [PATCH v5 07/13] lockdep: Make print_circular_bug() aware of crossrelease Byungchul Park <byungchul.park@lge.com> - 2017-01-18 14:30 +0100
  [PATCH v5 05/13] lockdep: Pass a callback arg to check_prev_add() to handle stack_trace Byungchul Park <byungchul.park@lge.com> - 2017-01-18 14:30 +0100
    Re: [PATCH v5 05/13] lockdep: Pass a callback arg to  check_prev_add() to handle stack_trace Byungchul Park <byungchul.park@lge.com> - 2017-01-26 08:50 +0100

csiph-web