Hi Duoming,
On Fri, Apr 8, 2022 at 10:51 PM Duoming Zhou <[email protected]> wrote:
> There is a deadlock in iss_net_close(), which is shown
> below:
>
> (Thread 1) | (Thread 2)
> | iss_net_open()
> iss_net_close() | mod_timer()
> spin_lock_bh() //(1) | (wait a time)
> ... | iss_net_timer()
> del_timer_sync() | spin_lock() //(2)
> (wait timer to stop) | ...
>
> We hold lp->lock in position (1) of thread 1 and use
> del_timer_sync() to wait timer to stop, but timer handler
> also need lp->lock in position (2) of thread 2. As a result,
> iss_net_close() will block forever.
Would block forever in SMP, that's true. But the ISS machine
that uses this driver had never had SMP support, so we
haven't seen it blocking so far.
> This patch extracts del_timer_sync() from the protection of
> spin_lock_bh(), which could let timer handler to obtain
> the needed lock. What`s more, we should remove spin_lock()
> in iss_net_timer(), because there is no resource need to
> protect and spin_lock() may cause deadlock in timer handler.
There's a lot more strange locking in this driver, so I've made
a cleanup series that gets rid of most of it, including what
causes the deadlock mentioned above. I'll post it shortly.
--
Thanks.
-- Max