On Sun, Aug 28, 2022 at 05:05:09PM +0800, Hillf Danton wrote: > On Fri, 29 Jul 2022 21:24:13 +0200 Greg Tulli <greg.iforce@xxxxxxxxx> > > > > Chain exists of: > > &iforce->xmit_lock --> &port_lock_key --> &serport->lock > > > > Possible unsafe locking scenario: > > > > CPU0 CPU1 > > ---- ---- > > lock(&serport->lock); > > lock(&port_lock_key); > > lock(&serport->lock); > > lock(&iforce->xmit_lock); > > > > *** 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? > > Another option is to do wakeup without serport->lock held, > given iforce->xmit_lock. Your test will provide info about the races > that may come up due to the serport->lock. No, I think we should use work to process the outbound buffer/queue, as the comment in include/linux/tty_ldisc.h recommends. I believe a single work instance in struct iforce_serio will suffice: iforce_serio_xmit already should be able to handle concurrent invocations, so we just need to schedule the work from iforce_serio_write_wakeup() (and it is fine if it is already scheduled) and let iforce_serio_xmit() do its job. We can wait for the buffer to empty (which should take care of the work running, but we may also do cancel_work_sync() for a good measure) in iforce_serio_stop_io(). Thanks. -- Dmitry
