Skip to content

Commit

Permalink
can: dev: fix deadlock reported after bus-off
Browse files Browse the repository at this point in the history
commit 9abefcb 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] victronenergy/venus#24

Signed-off-by: Sergei Miroshnichenko <[email protected]>
Signed-off-by: Marc Kleine-Budde <[email protected]>
[bwh: Backported to 3.2: adjust context]
Signed-off-by: Ben Hutchings <[email protected]>
  • Loading branch information
Sergei Miroshnichenko authored and bwhacks committed Nov 20, 2016
1 parent aa1e138 commit 1f53d4c
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 11 deletions.
27 changes: 17 additions & 10 deletions drivers/net/can/dev.c
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#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>
Expand Down Expand Up @@ -361,9 +362,8 @@ EXPORT_SYMBOL_GPL(can_free_echo_skb);
/*
* 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;
Expand Down Expand Up @@ -403,6 +403,14 @@ void can_restart(unsigned long data)
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);
Expand All @@ -416,8 +424,8 @@ int can_restart_now(struct net_device *dev)
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;
}
Expand All @@ -439,8 +447,8 @@ void can_bus_off(struct net_device *dev)
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);

Expand Down Expand Up @@ -515,6 +523,7 @@ struct net_device *alloc_candev(int sizeof_priv, unsigned int echo_skb_max)
return NULL;

priv = netdev_priv(dev);
priv->dev = dev;

if (echo_skb_max) {
priv->echo_skb_max = echo_skb_max;
Expand All @@ -524,7 +533,7 @@ struct net_device *alloc_candev(int sizeof_priv, unsigned int echo_skb_max)

priv->state = CAN_STATE_STOPPED;

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

return dev;
}
Expand Down Expand Up @@ -558,8 +567,6 @@ int open_candev(struct net_device *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);
Expand All @@ -574,7 +581,7 @@ void close_candev(struct net_device *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);
Expand Down
3 changes: 2 additions & 1 deletion include/linux/can/dev.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ enum can_mode {
* CAN common private data
*/
struct can_priv {
struct net_device *dev;
struct can_device_stats can_stats;

struct can_bittiming bittiming;
Expand All @@ -41,7 +42,7 @@ struct can_priv {
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);
Expand Down

0 comments on commit 1f53d4c

Please sign in to comment.