lkml.org 
[lkml]   [2016]   [Nov]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Date
    Subject[PATCH 3.2 137/152] can: dev: fix deadlock reported after bus-off
    3.2.84-rc1 review patch.  If anyone has any objections, please let me know.

    ------------------

    From: Sergei Miroshnichenko <sergeimir@emcraft.com>

    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>
    ---
    drivers/net/can/dev.c | 27 +++++++++++++++++----------
    include/linux/can/dev.h | 3 ++-
    2 files changed, 19 insertions(+), 11 deletions(-)

    --- 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/workqueue.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
    */
    -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;
    @@ -403,6 +403,14 @@ restart:
    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);
    @@ -416,8 +424,8 @@ int can_restart_now(struct net_device *d
    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;
    }
    @@ -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);

    @@ -515,6 +523,7 @@ struct net_device *alloc_candev(int size
    return NULL;

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

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

    priv->state = CAN_STATE_STOPPED;

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

    return dev;
    }
    @@ -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);
    @@ -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);
    --- 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 {
    + struct net_device *dev;
    struct can_device_stats can_stats;

    struct can_bittiming bittiming;
    @@ -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);
    \
     
     \ /
      Last update: 2016-11-14 03:11    [W:3.203 / U:0.284 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site