Re: [PATCH 1/2] net: ethernet: sxgbe: remove private tx queue lock

Hi,

On 09.12.2016 12:21, Pavel Machek wrote:

On Fri 2016-12-09 00:19:43, Francois Romieu wrote:

quoted

Lino Sanfilippo [off-list ref] :
[...]

quoted

OTOH Pavel said that he actually could produce a deadlock. Now I wonder if
this is caused by that locking scheme (in a way I have not figured out yet)
or if it is a different issue.

stmmac_tx_err races with stmmac_xmit.

Umm, yes, that looks real.

And that means that removing tx_lock will not be completely trivial
:-(. Lino, any ideas there?

Ok, the race is there but it looks like a problem that is not related to 
the use or removal of the private lock.
By a glimpse into other drivers (e.g sky2 or e1000), a possible way to handle a 
tx error is to start a separate task and restart the tx path in that task instead
the irq handler (or timer in case of the watchdog).

In that task we could do:
1. deactivate napi
2. deactivate irqs
3. wait for running napi/irqs do complete (_sync)
4. call stmmac_tx_err()
5. reenable napi
6. reenable irqs

We have to ensure that no xmit() is executing while stmmac_tx_err() does the cleanup,
so stmmac_tx_err() should IMO rather call netif_tx_disable() instead of netif_stop_queue()
(the former grabs the xmit lock before it sets __QUEUE_STATE_DRV_XOFF to disable
the queue).

Regards,
Lino