can: dev: fix deadlock reported after bus-off

commit 9abefcb1aaa58b9d5aa40a8bb12c87d02415e4c8 upstream.

A timer was used to restart after the bus-off state, leading to a
relatively large can_restart() executed in an interrupt context,
which in turn sets up pinctrl. When this happens during system boot,
there is a high probability of grabbing the pinctrl_list_mutex,
which is locked already by the probe() of other device, making the
kernel suspect a deadlock condition [1].

To resolve this issue, the restart_timer is replaced by a delayed
work.

[1] https://github.com/victronenergy/venus/issues/24

Signed-off-by: Sergei Miroshnichenko <sergeimir@emcraft.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
[bwh: Backported to 3.2: adjust context]
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>

diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index 217b0a3..a1f434e 100644 (file)
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -22,6 +22,7 @@
 #include <linux/slab.h>
 #include <linux/netdevice.h>
 #include <linux/if_arp.h>
 #include <linux/slab.h>
 #include <linux/netdevice.h>
 #include <linux/if_arp.h>

+#include <linux/workqueue.h>

 #include <linux/can.h>
 #include <linux/can/dev.h>
 #include <linux/can/netlink.h>
 #include <linux/can.h>
 #include <linux/can/dev.h>
 #include <linux/can/netlink.h>


@@ -361,9 +362,8 @@ EXPORT_SYMBOL_GPL(can_free_echo_skb);
 /*
  * CAN device restart for bus-off recovery
  */
 /*
  * CAN device restart for bus-off recovery
  */

-void can_restart(unsigned long data)
+void can_restart(struct net_device *dev)

 {
 {

-       struct net_device *dev = (struct net_device *)data;

        struct can_priv *priv = netdev_priv(dev);
        struct net_device_stats *stats = &dev->stats;
        struct sk_buff *skb;
        struct can_priv *priv = netdev_priv(dev);
        struct net_device_stats *stats = &dev->stats;
        struct sk_buff *skb;


@@ -403,6 +403,14 @@ restart:
                dev_err(dev->dev.parent, "Error %d during restart", err);
 }
 
                dev_err(dev->dev.parent, "Error %d during restart", err);
 }
 

+static void can_restart_work(struct work_struct *work)
+{
+       struct delayed_work *dwork = to_delayed_work(work);
+       struct can_priv *priv = container_of(dwork, struct can_priv, restart_work);
+
+       can_restart(priv->dev);
+}
+

 int can_restart_now(struct net_device *dev)
 {
        struct can_priv *priv = netdev_priv(dev);
 int can_restart_now(struct net_device *dev)
 {
        struct can_priv *priv = netdev_priv(dev);


@@ -416,8 +424,8 @@ int can_restart_now(struct net_device *dev)
        if (priv->state != CAN_STATE_BUS_OFF)
                return -EBUSY;
 
        if (priv->state != CAN_STATE_BUS_OFF)
                return -EBUSY;
 

-       /* Runs as soon as possible in the timer context */
-       mod_timer(&priv->restart_timer, jiffies);
+       cancel_delayed_work_sync(&priv->restart_work);
+       can_restart(dev);

 
        return 0;
 }
 
        return 0;
 }


@@ -439,8 +447,8 @@ void can_bus_off(struct net_device *dev)
        priv->can_stats.bus_off++;
 
        if (priv->restart_ms)
        priv->can_stats.bus_off++;
 
        if (priv->restart_ms)

-               mod_timer(&priv->restart_timer,
-                         jiffies + (priv->restart_ms * HZ) / 1000);
+               schedule_delayed_work(&priv->restart_work,
+                                     msecs_to_jiffies(priv->restart_ms));

 }
 EXPORT_SYMBOL_GPL(can_bus_off);
 
 }
 EXPORT_SYMBOL_GPL(can_bus_off);
 


@@ -515,6 +523,7 @@ struct net_device *alloc_candev(int sizeof_priv, unsigned int echo_skb_max)
                return NULL;
 
        priv = netdev_priv(dev);
                return NULL;
 
        priv = netdev_priv(dev);

+       priv->dev = dev;

 
        if (echo_skb_max) {
                priv->echo_skb_max = echo_skb_max;
 
        if (echo_skb_max) {
                priv->echo_skb_max = echo_skb_max;


@@ -524,7 +533,7 @@ struct net_device *alloc_candev(int sizeof_priv, unsigned int echo_skb_max)
 
        priv->state = CAN_STATE_STOPPED;
 
 
        priv->state = CAN_STATE_STOPPED;
 

-       init_timer(&priv->restart_timer);
+       INIT_DELAYED_WORK(&priv->restart_work, can_restart_work);

 
        return dev;
 }
 
        return dev;
 }


@@ -558,8 +567,6 @@ int open_candev(struct net_device *dev)
        if (!netif_carrier_ok(dev))
                netif_carrier_on(dev);
 
        if (!netif_carrier_ok(dev))
                netif_carrier_on(dev);
 

-       setup_timer(&priv->restart_timer, can_restart, (unsigned long)dev);
-

        return 0;
 }
 EXPORT_SYMBOL_GPL(open_candev);
        return 0;
 }
 EXPORT_SYMBOL_GPL(open_candev);


@@ -574,7 +581,7 @@ void close_candev(struct net_device *dev)
 {
        struct can_priv *priv = netdev_priv(dev);
 
 {
        struct can_priv *priv = netdev_priv(dev);
 

-       del_timer_sync(&priv->restart_timer);
+       cancel_delayed_work_sync(&priv->restart_work);

        can_flush_echo_skb(dev);
 }
 EXPORT_SYMBOL_GPL(close_candev);
        can_flush_echo_skb(dev);
 }
 EXPORT_SYMBOL_GPL(close_candev);

diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
index a0969fc..309d2aa 100644 (file)
--- a/include/linux/can/dev.h
+++ b/include/linux/can/dev.h
@@ -30,6 +30,7 @@ enum can_mode {
  * CAN common private data
  */
 struct can_priv {
  * CAN common private data
  */
 struct can_priv {

+       struct net_device *dev;

        struct can_device_stats can_stats;
 
        struct can_bittiming bittiming;
        struct can_device_stats can_stats;
 
        struct can_bittiming bittiming;


@@ -41,7 +42,7 @@ struct can_priv {
        u32 ctrlmode_supported;
 
        int restart_ms;
        u32 ctrlmode_supported;
 
        int restart_ms;

-       struct timer_list restart_timer;
+       struct delayed_work restart_work;

 
        int (*do_set_bittiming)(struct net_device *dev);
        int (*do_set_mode)(struct net_device *dev, enum can_mode mode);
 
        int (*do_set_bittiming)(struct net_device *dev);
        int (*do_set_mode)(struct net_device *dev, enum can_mode mode);