diff mbox

[RFC] xfs: remove i_iolock and use i_rwsem in the VFS inode instead

Message ID 20160907074312.GQ10153@twins.programming.kicks-ass.net (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Peter Zijlstra Sept. 7, 2016, 7:43 a.m. UTC 
On Mon, Sep 05, 2016 at 05:12:44PM +0200, Christoph Hellwig wrote:
> Peter, this is the fixed up patch.  Can you write a proper changelog
> and add your signoff?

Yes, sorry for the delay.

Did you get the workqueue thing sorted, where you rely on another task
holding the lock for you?

I simplified the implementation a little, since I noticed pushing the
@read argument all the way down to match_held_lock() didn't really make
all that much sense and caused a lot of churn.

I also added CONFIG_LOCKDEP=n stubs for the new lockdep_assert_held*()
macros (which you don't use :-).

Feel free to push this through the XFS tree where you add its first use.

---
Subject: locking/lockdep: Provide a type check for lock_is_held
From: Peter Zijlstra <peterz@infradead.org>
Date: Mon, 5 Sep 2016 17:12:44 +0200

Christoph requested lockdep_assert_held() variants that distinguish
between held-for-read or held-for-write.

Provide:

  int lock_is_held_type(struct lockdep_map *lock, int read)

which takes the same argument as lock_acquire(.read) and matches it to
the held_lock instance.

Use of this function should be gated by the debug_locks variable. When
that is 0 the return value of the lock_is_held_type() function is
undefined. This is done to allow both negative and positive tests for
holding locks.

By default we provide (positive)
lockdep_assert_held{,_exclusive,_read}() macros.

Requested-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 include/linux/lockdep.h  |   23 +++++++++++++++++++++--
 kernel/locking/lockdep.c |   20 ++++++++++++--------
 2 files changed, 33 insertions(+), 10 deletions(-)

Comments

Ingo Molnar Sept. 8, 2016, 6:06 a.m. UTC | #1 
* Peter Zijlstra <peterz@infradead.org> wrote:

> On Mon, Sep 05, 2016 at 05:12:44PM +0200, Christoph Hellwig wrote:
> > Peter, this is the fixed up patch.  Can you write a proper changelog
> > and add your signoff?
> 
> Yes, sorry for the delay.
> 
> Did you get the workqueue thing sorted, where you rely on another task
> holding the lock for you?
> 
> I simplified the implementation a little, since I noticed pushing the
> @read argument all the way down to match_held_lock() didn't really make
> all that much sense and caused a lot of churn.
> 
> I also added CONFIG_LOCKDEP=n stubs for the new lockdep_assert_held*()
> macros (which you don't use :-).
> 
> Feel free to push this through the XFS tree where you add its first use.

I can also create a separate one-commit tree for it which you can pull into the 
XFS tree. This way there won't be conflicts between the XFS tree and the locking 
tree.

Thanks,

	Ingo
diff mbox

Patch

--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -338,9 +338,18 @@  extern void lock_acquire(struct lockdep_
 extern void lock_release(struct lockdep_map *lock, int nested,
 			 unsigned long ip);
 
-#define lockdep_is_held(lock)	lock_is_held(&(lock)->dep_map)
+/*
+ * Same "read" as for lock_acquire(), except -1 means any.
+ */
+extern int lock_is_held_type(struct lockdep_map *lock, int read);
+
+static inline int lock_is_held(struct lockdep_map *lock)
+{
+	return lock_is_held_type(lock, -1);
+}
 
-extern int lock_is_held(struct lockdep_map *lock);
+#define lockdep_is_held(lock)		lock_is_held(&(lock)->dep_map)
+#define lockdep_is_held_type(lock, r)	lock_is_held_type(&(lock)->dep_map, (r))
 
 extern void lock_set_class(struct lockdep_map *lock, const char *name,
 			   struct lock_class_key *key, unsigned int subclass,
@@ -372,6 +381,14 @@  extern void lock_unpin_lock(struct lockd
 		WARN_ON(debug_locks && !lockdep_is_held(l));	\
 	} while (0)
 
+#define lockdep_assert_held_exclusive(l)	do {			\
+		WARN_ON(debug_locks && !lockdep_is_held_type(l, 0));	\
+	} while (0)
+
+#define lockdep_assert_held_read(l)	do {				\
+		WARN_ON(debug_locks && !lockdep_is_held_type(l, 1));	\
+	} while (0)
+
 #define lockdep_assert_held_once(l)	do {				\
 		WARN_ON_ONCE(debug_locks && !lockdep_is_held(l));	\
 	} while (0)
@@ -429,6 +446,8 @@  struct lock_class_key { };
 #define lockdep_depth(tsk)	(0)
 
 #define lockdep_assert_held(l)			do { (void)(l); } while (0)
+#define lockdep_assert_held_exclusive(l)	do { (void)(l); } while (0)
+#define lockdep_assert_held_read(l)		do { (void)(l); } while (0)
 #define lockdep_assert_held_once(l)		do { (void)(l); } while (0)
 
 #define lockdep_recursing(tsk)			(0)
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -3188,7 +3188,7 @@  print_lock_nested_lock_not_held(struct t
 	return 0;
 }
 
-static int __lock_is_held(struct lockdep_map *lock);
+static int __lock_is_held(struct lockdep_map *lock, int read);
 
 /*
  * This gets called for every mutex_lock*()/spin_lock*() operation.
@@ -3329,7 +3329,7 @@  static int __lock_acquire(struct lockdep
 	}
 	chain_key = iterate_chain_key(chain_key, class_idx);
 
-	if (nest_lock && !__lock_is_held(nest_lock))
+	if (nest_lock && !__lock_is_held(nest_lock, -1))
 		return print_lock_nested_lock_not_held(curr, hlock, ip);
 
 	if (!validate_chain(curr, lock, hlock, chain_head, chain_key))
@@ -3576,7 +3576,7 @@  __lock_release(struct lockdep_map *lock,
 	return 1;
 }
 
-static int __lock_is_held(struct lockdep_map *lock)
+static int __lock_is_held(struct lockdep_map *lock, int read)
 {
 	struct task_struct *curr = current;
 	int i;
@@ -3584,8 +3584,12 @@  static int __lock_is_held(struct lockdep
 	for (i = 0; i < curr->lockdep_depth; i++) {
 		struct held_lock *hlock = curr->held_locks + i;
 
-		if (match_held_lock(hlock, lock))
-			return 1;
+		if (match_held_lock(hlock, lock)) {
+			if (read == -1 || hlock->read == read)
+				return 1;
+
+			return 0;
+		}
 	}
 
 	return 0;
@@ -3769,7 +3773,7 @@  void lock_release(struct lockdep_map *lo
 }
 EXPORT_SYMBOL_GPL(lock_release);
 
-int lock_is_held(struct lockdep_map *lock)
+int lock_is_held_type(struct lockdep_map *lock, int read)
 {
 	unsigned long flags;
 	int ret = 0;
@@ -3781,13 +3785,13 @@  int lock_is_held(struct lockdep_map *loc
 	check_flags(flags);
 
 	current->lockdep_recursion = 1;
-	ret = __lock_is_held(lock);
+	ret = __lock_is_held(lock, read);
 	current->lockdep_recursion = 0;
 	raw_local_irq_restore(flags);
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(lock_is_held);
+EXPORT_SYMBOL_GPL(lock_is_held_type);
 
 struct pin_cookie lock_pin_lock(struct lockdep_map *lock)
 {