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


Groups > linux.kernel > #1602997 > unrolled thread

Re: [PATCH 4.4 14/36] IB/ipoib: Fix deadlock between rmmod and set_mode

Started byBen Hutchings <ben@decadent.org.uk>
First post2017-03-17 03:30 +0100
Last post2017-03-17 03:30 +0100
Articles 1 — 1 participant

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH 4.4 14/36] IB/ipoib: Fix deadlock between rmmod and  set_mode Ben Hutchings <ben@decadent.org.uk> - 2017-03-17 03:30 +0100

#1602997 — Re: [PATCH 4.4 14/36] IB/ipoib: Fix deadlock between rmmod and set_mode

FromBen Hutchings <ben@decadent.org.uk>
Date2017-03-17 03:30 +0100
SubjectRe: [PATCH 4.4 14/36] IB/ipoib: Fix deadlock between rmmod and set_mode
Message-ID<tlPOq-7wu-11@gated-at.bofh.it>

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

On Mon, 2017-03-13 at 16:39 +0800, Greg Kroah-Hartman wrote:
> 4.4-stable review patch.  If anyone has any objections, please let me know.
> 
> ------------------
> 
> From: Feras Daoud <ferasda@mellanox.com>
> 
> commit 0a0007f28304cb9fc87809c86abb80ec71317f20 upstream.
> 
> When calling set_mode from sys/fs, the call flow locks the sys/fs lock
> first and then tries to lock rtnl_lock (when calling ipoib_set_mod).
> On the other hand, the rmmod call flow takes the rtnl_lock first
> (when calling unregister_netdev) and then tries to take the sys/fs
> lock. Deadlock a->b, b->a.
> 
> The problem starts when ipoib_set_mod frees it's rtnl_lck and tries
> to get it after that.
[...]
> --- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
> +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> @@ -464,8 +464,7 @@ int ipoib_set_mode(struct net_device *de
>                 priv->tx_wr.wr.send_flags &= ~IB_SEND_IP_CSUM;
>  
>                 ipoib_flush_paths(dev);
> -               rtnl_lock();
> -               return 0;
> +               return (!rtnl_trylock()) ? -EBUSY : 0;
>         }
>  
>         if (!strcmp(buf, "datagram\n")) {
> @@ -474,8 +473,7 @@ int ipoib_set_mode(struct net_device *de
>                 dev_set_mtu(dev, min(priv->mcast_mtu, dev->mtu));
>                 rtnl_unlock();
>                 ipoib_flush_paths(dev);
> -               rtnl_lock();
> -               return 0;
> +               return (!rtnl_trylock()) ? -EBUSY : 0;
>         }
>  
>         return -EINVAL;

Since you didn't change ipoib_changelink() to handle this, that now has
a potential lock imbalance.

Ben.

-- 
Ben Hutchings
Hoare's Law of Large Problems:
        Inside every large problem is a small problem struggling to get
out.

[toc] | [standalone]


Back to top | Article view | linux.kernel


csiph-web