Re: Converting dev->mutex into dev->spinlock ?

[Alan Stern <stern@rowland.harvard.edu>]

On Wed, Feb 08, 2023 at 07:17:20AM +0900, Tetsuo Handa wrote:
> On 2023/02/08 2:46, Alan Stern wrote:
> > The real question is what will happen in your syzbot test scenarios.  
> > Lockdep certainly ought to be able to detect a real deadlock when one 
> > occurs.  It will be more interesting to find out if it can warn about 
> > potential deadlocks _without_ them occurring.
> 
> For example, https://syzkaller.appspot.com/x/repro.c?x=15556074480000 generates
> below warning, but I don't have syzbot environment. Please propose an updated
> patch (which won't hit WARN_ON_ONCE()) for allowing people to try it in syzbot
> environment.

Here is a patch.  I haven't tried to compile it.

Alan Stern



Index: usb-devel/drivers/base/core.c
===================================================================
--- usb-devel.orig/drivers/base/core.c
+++ usb-devel/drivers/base/core.c
@@ -2322,6 +2322,9 @@ static void device_release(struct kobjec
 	devres_release_all(dev);
 
 	kfree(dev->dma_range_map);
+	mutex_destroy(&dev->mutex);
+	if (!lockdep_static_obj(dev))
+		lockdep_unregister_key(&dev->mutex_key);
 
 	if (dev->release)
 		dev->release(dev);
@@ -2941,7 +2944,10 @@ void device_initialize(struct device *de
 	kobject_init(&dev->kobj, &device_ktype);
 	INIT_LIST_HEAD(&dev->dma_pools);
 	mutex_init(&dev->mutex);
-	lockdep_set_novalidate_class(&dev->mutex);
+	if (!lockdep_static_obj(dev)) {
+		lockdep_register_key(&dev->mutex_key);
+		lockdep_set_class(&dev->mutex, &dev->mutex_key);
+	}
 	spin_lock_init(&dev->devres_lock);
 	INIT_LIST_HEAD(&dev->devres_head);
 	device_pm_init(dev);
Index: usb-devel/include/linux/device.h
===================================================================
--- usb-devel.orig/include/linux/device.h
+++ usb-devel/include/linux/device.h
@@ -570,6 +570,7 @@ struct device {
 	struct mutex		mutex;	/* mutex to synchronize calls to
 					 * its driver.
 					 */
+	struct lock_class_key	mutex_key;	/* Unique key for each device */
 
 	struct dev_links_info	links;
 	struct dev_pm_info	power;
Index: usb-devel/include/linux/lockdep.h
===================================================================
--- usb-devel.orig/include/linux/lockdep.h
+++ usb-devel/include/linux/lockdep.h
@@ -172,6 +172,7 @@ do {							\
 	current->lockdep_recursion -= LOCKDEP_OFF;	\
 } while (0)
 
+extern int lockdep_static_obj(const void *obj);
 extern void lockdep_register_key(struct lock_class_key *key);
 extern void lockdep_unregister_key(struct lock_class_key *key);
 
Index: usb-devel/kernel/locking/lockdep.c
===================================================================
--- usb-devel.orig/kernel/locking/lockdep.c
+++ usb-devel/kernel/locking/lockdep.c
@@ -831,7 +831,7 @@ static int arch_is_kernel_initmem_freed(
 }
 #endif
 
-static int static_obj(const void *obj)
+int lockdep_static_obj(const void *obj)
 {
 	unsigned long start = (unsigned long) &_stext,
 		      end   = (unsigned long) &_end,
@@ -857,6 +857,7 @@ static int static_obj(const void *obj)
 	 */
 	return is_module_address(addr) || is_module_percpu_address(addr);
 }
+EXPORT_SYMBOL_GPL(lockdep_static_obj);
 #endif
 
 /*
@@ -969,7 +970,7 @@ static bool assign_lock_key(struct lockd
 		lock->key = (void *)can_addr;
 	else if (__is_module_percpu_address(addr, &can_addr))
 		lock->key = (void *)can_addr;
-	else if (static_obj(lock))
+	else if (lockdep_static_obj(lock))
 		lock->key = (void *)lock;
 	else {
 		/* Debug-check: all keys must be persistent! */
@@ -1220,7 +1221,7 @@ void lockdep_register_key(struct lock_cl
 	struct lock_class_key *k;
 	unsigned long flags;
 
-	if (WARN_ON_ONCE(static_obj(key)))
+	if (WARN_ON_ONCE(lockdep_static_obj(key)))
 		return;
 	hash_head = keyhashentry(key);
 
@@ -1246,7 +1247,7 @@ static bool is_dynamic_key(const struct
 	struct lock_class_key *k;
 	bool found = false;
 
-	if (WARN_ON_ONCE(static_obj(key)))
+	if (WARN_ON_ONCE(lockdep_static_obj(key)))
 		return false;
 
 	/*
@@ -1293,7 +1294,7 @@ register_lock_class(struct lockdep_map *
 	if (!lock->key) {
 		if (!assign_lock_key(lock))
 			return NULL;
-	} else if (!static_obj(lock->key) && !is_dynamic_key(lock->key)) {
+	} else if (!lockdep_static_obj(lock->key) && !is_dynamic_key(lock->key)) {
 		return NULL;
 	}
 
@@ -4836,7 +4837,7 @@ void lockdep_init_map_type(struct lockde
 	 * Sanity check, the lock-class key must either have been allocated
 	 * statically or must have been registered as a dynamic key.
 	 */
-	if (!static_obj(key) && !is_dynamic_key(key)) {
+	if (!lockdep_static_obj(key) && !is_dynamic_key(key)) {
 		if (debug_locks)
 			printk(KERN_ERR "BUG: key %px has not been registered!\n", key);
 		DEBUG_LOCKS_WARN_ON(1);
@@ -6335,7 +6336,7 @@ void lockdep_unregister_key(struct lock_
 
 	might_sleep();
 
-	if (WARN_ON_ONCE(static_obj(key)))
+	if (WARN_ON_ONCE(lockdep_static_obj(key)))
 		return;
 
 	raw_local_irq_save(flags);