From 79c4b2fb58b48cbf543a10bb196ed60f583e12d3 Mon Sep 17 00:00:00 2001 From: Ashish Kumar Gupta Date: Wed, 27 Mar 2024 13:56:45 +0000 Subject: [PATCH] ANDROID: usb: gadget: android_f_accessory: use spinlock instead of mutex lock The configfs_composite_disconnect function was holding a spin lock, and the android_acc_disconnect function was holding a mutex lock. As a result of this, sleeping function called from invalid context error was coming. [ 106.229993][ T987] BUG: sleeping function called from invalid context at kernel/locking/mutex.c:289 [ 106.231143][ T987] in_atomic(): 1, irqs_disabled(): 1, non_block: 0, pid: 987, name: android.hardwar [ 106.232305][ T987] preempt_count: 1, expected: 0 //hold spin_lock_irqsave(&gi->spinlock, flags) and preempt_count will plus one [ 106.232899][ T987] RCU nest depth: 0, expected: 0 [ 106.233505][ T987] Preemption disabled at: [ 106.233507][ T987] [] configfs_composite_disconnect+0x2c/0x6c [ 106.234055][ T987] CPU: 6 PID: 987 Comm: android.hardwar Tainted: G W OE 6.6.17-android15-1-ge8779207b2e9 #1 [ 106.236422][ T987] Hardware name: MT6991(ENG) (DT) [ 106.236425][ T987] Call trace: [ 106.236426][ T987] dump_backtrace+0xec/0x138 [ 106.236434][ T987] show_stack+0x18/0x24 [ 106.236437][ T987] dump_stack_lvl+0x50/0x6c [ 106.236449][ T987] dump_stack+0x18/0x24 [ 106.236452][ T987] __might_resched+0x150/0x170 [ 106.236458][ T987] __might_sleep+0x48/0x7c [ 106.236460][ T987] mutex_lock+0x24/0x74 [ 106.236465][ T987] android_acc_disconnect+0x1c/0xe0 [ 106.236472][ T987] __composite_disconnect+0x20/0x12c Steps to reproduce the issue: 1. Connect the Android device via USB cable to the computer. 2. Open the USB Preference to change the USB function. 3. Select the MTP, PTP, or other USB function and switch it. 4. Kernel log will show this Warning log after switch USB function. Test: manually tested the above steps, and successfully switch between USB modes. Bug: 322732675 Change-Id: I8131595b02ecb2631defcca4cdde456d83f76525 Signed-off-by: Ashish Kumar Gupta --- .../usb/gadget/function/android_f_accessory.c | 32 +++++++++++++------ 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/drivers/usb/gadget/function/android_f_accessory.c b/drivers/usb/gadget/function/android_f_accessory.c index 85241f1beadf..936e8998166b 100644 --- a/drivers/usb/gadget/function/android_f_accessory.c +++ b/drivers/usb/gadget/function/android_f_accessory.c @@ -262,7 +262,7 @@ static struct usb_gadget_strings *acc_strings[] = { NULL, }; -static DEFINE_MUTEX(acc_dev_instance_lock); +static DEFINE_SPINLOCK(acc_dev_instance_lock); static struct acc_dev *acc_dev_instance; struct acc_instance { @@ -272,10 +272,12 @@ struct acc_instance { static struct acc_dev *get_acc_dev(void) { - mutex_lock(&acc_dev_instance_lock); + unsigned long flags; + + spin_lock_irqsave(&acc_dev_instance_lock, flags); if (acc_dev_instance) kref_get(&acc_dev_instance->kref); - mutex_unlock(&acc_dev_instance_lock); + spin_unlock_irqrestore(&acc_dev_instance_lock, flags); return acc_dev_instance; } @@ -297,8 +299,17 @@ static void __acc_dev_instance_release(struct kref *kref) static void put_acc_dev(struct acc_dev *dev) { - kref_put_mutex(&acc_dev_instance->kref, __acc_dev_instance_release, - &acc_dev_instance_lock); + unsigned long flags; + + /* + * This is not best engineering practice, and does cause coupling with + * kref internal structure. It might cause an issue if kref internal + * refcount structure is changed. We will remove this implementation + * in the next kernel version. + */ + if (refcount_dec_and_lock_irqsave(&acc_dev_instance->kref.refcount, + &acc_dev_instance_lock, &flags)) + __acc_dev_instance_release(&acc_dev_instance->kref); } static inline struct acc_dev *func_to_dev(struct usb_function *f) @@ -1243,15 +1254,16 @@ static int acc_init(void) { struct acc_dev *dev; int ret; + unsigned long flags; - mutex_lock(&acc_dev_instance_lock); + spin_lock_irqsave(&acc_dev_instance_lock, flags); if (acc_dev_instance) { - mutex_unlock(&acc_dev_instance_lock); + spin_unlock_irqrestore(&acc_dev_instance_lock, flags); return -EBUSY; } dev = kzalloc(sizeof(*dev), GFP_KERNEL); if (!dev) { - mutex_unlock(&acc_dev_instance_lock); + spin_unlock_irqrestore(&acc_dev_instance_lock, flags); return -ENOMEM; } @@ -1273,12 +1285,12 @@ static int acc_init(void) kref_init(&dev->kref); acc_dev_instance = dev; - mutex_unlock(&acc_dev_instance_lock); + spin_unlock_irqrestore(&acc_dev_instance_lock, flags); return 0; err_free_dev: kfree(dev); - mutex_unlock(&acc_dev_instance_lock); + spin_unlock_irqrestore(&acc_dev_instance_lock, flags); pr_err("USB accessory gadget driver failed to initialize\n"); return ret; }