On 2022/07/30 4:24, Greg Tulli wrote: > I was advised to use a vanilla kernel with CONFIG_PROVE_LOCKING and > decode the symbols in the report. So I followed > https://kernelnewbies.org/OutreachyfirstpatchSetup and here's the > result. Thanks, Tetsuo Handa. > > WARNING: possible circular locking dependency detected > 5.19.0-rc6iforce-00194-gf3a76018dd55 #2 Not tainted OK. You confirmed that this is a real deadlock. > 2022-07-11 11:32 GMT+02:00, Greg T <greg.iforce@xxxxxxxxx>: > >> That problem vanishes if we don't call iforce_process_packet directly >> from iforce_serio_irq, but from a tasklet. Is that a right approach? Does untested diff shown below solve the problem? I don't know about flush ordering in iforce_serio_disconnect(). drivers/input/joystick/iforce/iforce-serio.c | 51 +++++++++++++++++++- 1 file changed, 49 insertions(+), 2 deletions(-) diff --git a/drivers/input/joystick/iforce/iforce-serio.c b/drivers/input/joystick/iforce/iforce-serio.c index f95a81b9fac7..5ff650ce05ab 100644 --- a/drivers/input/joystick/iforce/iforce-serio.c +++ b/drivers/input/joystick/iforce/iforce-serio.c @@ -124,8 +124,7 @@ static void iforce_serio_write_wakeup(struct serio *serio) iforce_serio_xmit(iforce); } -static irqreturn_t iforce_serio_irq(struct serio *serio, - unsigned char data, unsigned int flags) +static void do_iforce_serio(struct serio *serio, unsigned char data) { struct iforce_serio *iforce_serio = serio_get_drvdata(serio); struct iforce *iforce = &iforce_serio->iforce; @@ -183,6 +182,53 @@ static irqreturn_t iforce_serio_irq(struct serio *serio, iforce_serio->csum = 0; } out: + return; +} + +struct iforce_serio_irq { + struct list_head list; + struct serio *serio; + unsigned char data; +}; +static LIST_HEAD(iforce_serio_irq_list); +static DEFINE_SPINLOCK(iforce_serio_irq_list_lock); + +static void iforce_serio_workfn(struct work_struct *work) +{ + static DEFINE_MUTEX(lock); + struct iforce_serio_irq *ev; + + mutex_lock(&lock); + spin_lock_irq(&iforce_serio_irq_list_lock); + while (!list_empty(&iforce_serio_irq_list)) { + ev = list_first_entry(&iforce_serio_irq_list, struct iforce_serio_irq, list); + list_del(&ev->list); + spin_unlock_irq(&iforce_serio_irq_list_lock); + do_iforce_serio(ev->serio, ev->data); + kfree(ev); + spin_lock_irq(&iforce_serio_irq_list_lock); + } + spin_unlock_irq(&iforce_serio_irq_list_lock); + mutex_unlock(&lock); +} +static DECLARE_WORK(iforce_serio_work, iforce_serio_workfn); + +static irqreturn_t iforce_serio_irq(struct serio *serio, + unsigned char data, unsigned int flags) +{ + bool was_empty; + struct iforce_serio_irq *ev = kmalloc(sizeof(*ev), GFP_ATOMIC | __GFP_NOWARN); + + if (ev) { + ev->serio = serio; + ev->data = data; + spin_lock(&iforce_serio_irq_list_lock); + was_empty = list_empty(&iforce_serio_irq_list); + list_add_tail(&ev->list, &iforce_serio_irq_list); + spin_unlock(&iforce_serio_irq_list_lock); + if (was_empty) + queue_work(system_highpri_wq, &iforce_serio_work); + } return IRQ_HANDLED; } @@ -220,6 +266,7 @@ static void iforce_serio_disconnect(struct serio *serio) { struct iforce_serio *iforce_serio = serio_get_drvdata(serio); + flush_work(&iforce_serio_work); input_unregister_device(iforce_serio->iforce.dev); serio_close(serio); serio_set_drvdata(serio, NULL); -- 2.18.4
