Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1538907

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

Show all headers | View raw

[Multipart message — attachments visible in raw view] - view raw

On Thu 2016-12-08 21:32:12, Lino Sanfilippo wrote:
> Hi,
> 
> On 08.12.2016 00:15, Francois Romieu wrote:
> > Lino Sanfilippo <LinoSanfilippo@gmx.de> :
> >> The driver uses a private lock for synchronization between the xmit
> >> function and the xmit completion handler, but since the NETIF_F_LLTX flag
> >> is not set, the xmit function is also called with the xmit_lock held.
> >> 
> >> On the other hand the xmit completion handler first takes the private lock
> >> and (in case that the tx queue has been stopped) the xmit_lock, leading
> >> to a reverse locking order and the potential danger of a deadlock.
> > 
> > netif_tx_stop_queue is used by:
> > 1. xmit function before releasing lock and returning.
> > 2. sxgbe_restart_tx_queue()
> >    <- sxgbe_tx_interrupt
> >    <- sxgbe_reset_all_tx_queues()
> >       <- sxgbe_tx_timeout()
> > 
> > Given xmit won't be called again until tx queue is enabled, it's not clear
> > how a deadlock could happen due to #1.
> > 
> 
> 
> After spending more thoughts on this I tend to agree with you. Yes, we have the
> different locking order for the xmit_lock and the private lock in two concurrent
> threads. And one of the first things one learns about locking is that this is a
> good way to create a deadlock sooner or later. But in our case the deadlock 
> can only occur if the xmit function and the tx completion handler perceive different
>  states for the tx queue, or to be more specific: 
> the completion handler sees the tx queue in state "stopped" while the xmit handler 
> sees it in state "running" at the same time. Only then both functions would try to
> take both locks, which could lead to a deadlock.
> 
> 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.

Pavel has some problems, but that's on different hardware.. and it is
possible that it is deadlock (or something else) somewhere else.

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

Remove private locks to avoid possible deadlock Lino Sanfilippo <LinoSanfilippo@gmx.de> - 2016-12-07 21:10 +0100
  [PATCH 1/2] net: ethernet: sxgbe: remove private tx queue lock Lino Sanfilippo <LinoSanfilippo@gmx.de> - 2016-12-07 21:10 +0100
    Re: [PATCH 1/2] net: ethernet: sxgbe: remove private tx queue lock Francois Romieu <romieu@fr.zoreil.com> - 2016-12-08 00:20 +0100
      Re: [PATCH 1/2] net: ethernet: sxgbe: remove private tx queue lock Lino Sanfilippo <LinoSanfilippo@gmx.de> - 2016-12-08 21:40 +0100
        Re: [PATCH 1/2] net: ethernet: sxgbe: remove private tx queue lock Pavel Machek <pavel@ucw.cz> - 2016-12-08 23:00 +0100
          Re: [PATCH 1/2] net: ethernet: sxgbe: remove private tx queue lock Lino Sanfilippo <LinoSanfilippo@gmx.de> - 2016-12-08 23:20 +0100
            Re: [PATCH 1/2] net: ethernet: sxgbe: remove private tx queue lock Pavel Machek <pavel@ucw.cz> - 2016-12-08 23:20 +0100
              Re: [PATCH 1/2] net: ethernet: sxgbe: remove private tx queue lock Lino Sanfilippo <LinoSanfilippo@gmx.de> - 2016-12-08 23:50 +0100
        Re: [PATCH 1/2] net: ethernet: sxgbe: remove private tx queue lock Francois Romieu <romieu@fr.zoreil.com> - 2016-12-09 00:30 +0100
          Re: [PATCH 1/2] net: ethernet: sxgbe: remove private tx queue lock Pavel Machek <pavel@ucw.cz> - 2016-12-09 12:30 +0100
            Re: [PATCH 1/2] net: ethernet: sxgbe: remove private tx queue lock Lino Sanfilippo <LinoSanfilippo@gmx.de> - 2016-12-10 03:30 +0100
  [PATCH 2/2] net: ethernet: stmmac: remove private tx queue lock Lino Sanfilippo <LinoSanfilippo@gmx.de> - 2016-12-07 21:10 +0100
    Re: [PATCH 2/2] net: ethernet: stmmac: remove private tx queue lock Pavel Machek <pavel@ucw.cz> - 2016-12-07 22:00 +0100
    Re: [PATCH 2/2] net: ethernet: stmmac: remove private tx queue lock Pavel Machek <pavel@ucw.cz> - 2016-12-07 22:00 +0100
    Re: [PATCH 2/2] net: ethernet: stmmac: remove private tx queue lock Pavel Machek <pavel@ucw.cz> - 2016-12-07 22:40 +0100
      Re: [PATCH 2/2] net: ethernet: stmmac: remove private tx queue lock Lino Sanfilippo <LinoSanfilippo@gmx.de> - 2016-12-07 22:50 +0100
        Re: [PATCH 2/2] net: ethernet: stmmac: remove private tx queue lock Lino Sanfilippo <LinoSanfilippo@gmx.de> - 2016-12-07 23:40 +0100
          Re: [PATCH 2/2] net: ethernet: stmmac: remove private tx queue lock Pavel Machek <pavel@ucw.cz> - 2016-12-08 00:30 +0100
      Re: [PATCH 2/2] net: ethernet: stmmac: remove private tx queue lock David Miller <davem@davemloft.net> - 2016-12-08 00:50 +0100
        Re: [PATCH 2/2] net: ethernet: stmmac: remove private tx queue lock Pavel Machek <pavel@ucw.cz> - 2016-12-08 15:20 +0100
          Re: [PATCH 2/2] net: ethernet: stmmac: remove private tx queue lock David Miller <davem@davemloft.net> - 2016-12-08 16:30 +0100
            Re: [PATCH 2/2] net: ethernet: stmmac: remove private tx queue lock Pavel Machek <pavel@ucw.cz> - 2016-12-08 17:00 +0100

csiph-web