lkml.org 
[lkml]   [2015]   [Jan]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    Subject[PATCH v5 1/5] can: kvaser_usb: Update net interface state before exiting on OOM
    From: Ahmed S. Darwish <ahmed.darwish@valeo.com>

    Update all of the can interface's state and error counters before
    trying any skb allocation that can actually fail with -ENOMEM.

    Suggested-by: Marc Kleine-Budde <mkl@pengutronix.de>
    Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
    ---
    drivers/net/can/usb/kvaser_usb.c | 182 +++++++++++++++++++++++----------------
    1 file changed, 106 insertions(+), 76 deletions(-)

    diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
    index c32cd61..971c5f9 100644
    --- a/drivers/net/can/usb/kvaser_usb.c
    +++ b/drivers/net/can/usb/kvaser_usb.c
    @@ -273,6 +273,10 @@ struct kvaser_msg {
    } u;
    } __packed;

    +struct kvaser_usb_error_summary {
    + u8 channel, status, txerr, rxerr, error_factor;
    +};
    +
    struct kvaser_usb_tx_urb_context {
    struct kvaser_usb_net_priv *priv;
    u32 echo_index;
    @@ -615,6 +619,55 @@ static void kvaser_usb_unlink_tx_urbs(struct kvaser_usb_net_priv *priv)
    priv->tx_contexts[i].echo_index = MAX_TX_URBS;
    }

    +static void kvaser_usb_rx_error_update_can_state(struct kvaser_usb_net_priv *priv,
    + const struct kvaser_usb_error_summary *es)
    +{
    + struct net_device_stats *stats;
    + enum can_state new_state;
    +
    + stats = &priv->netdev->stats;
    + new_state = priv->can.state;
    +
    + netdev_dbg(priv->netdev, "Error status: 0x%02x\n", es->status);
    +
    + if (es->status & M16C_STATE_BUS_OFF) {
    + priv->can.can_stats.bus_off++;
    + new_state = CAN_STATE_BUS_OFF;
    + } else if (es->status & M16C_STATE_BUS_PASSIVE) {
    + if (priv->can.state != CAN_STATE_ERROR_PASSIVE)
    + priv->can.can_stats.error_passive++;
    + new_state = CAN_STATE_ERROR_PASSIVE;
    + }
    +
    + if (es->status == M16C_STATE_BUS_ERROR) {
    + if ((priv->can.state < CAN_STATE_ERROR_WARNING) &&
    + ((es->txerr >= 96) || (es->rxerr >= 96))) {
    + priv->can.can_stats.error_warning++;
    + new_state = CAN_STATE_ERROR_WARNING;
    + } else if (priv->can.state > CAN_STATE_ERROR_ACTIVE) {
    + new_state = CAN_STATE_ERROR_ACTIVE;
    + }
    + }
    +
    + if (!es->status)
    + new_state = CAN_STATE_ERROR_ACTIVE;
    +
    + if (priv->can.restart_ms &&
    + (priv->can.state >= CAN_STATE_BUS_OFF) &&
    + (new_state < CAN_STATE_BUS_OFF)) {
    + priv->can.can_stats.restarts++;
    + }
    +
    + if (es->error_factor) {
    + priv->can.can_stats.bus_error++;
    + stats->rx_errors++;
    + }
    +
    + priv->bec.txerr = es->txerr;
    + priv->bec.rxerr = es->rxerr;
    + priv->can.state = new_state;
    +}
    +
    static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
    const struct kvaser_msg *msg)
    {
    @@ -622,30 +675,30 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
    struct sk_buff *skb;
    struct net_device_stats *stats;
    struct kvaser_usb_net_priv *priv;
    - unsigned int new_state;
    - u8 channel, status, txerr, rxerr, error_factor;
    + struct kvaser_usb_error_summary es = { };
    + enum can_state old_state;

    switch (msg->id) {
    case CMD_CAN_ERROR_EVENT:
    - channel = msg->u.error_event.channel;
    - status = msg->u.error_event.status;
    - txerr = msg->u.error_event.tx_errors_count;
    - rxerr = msg->u.error_event.rx_errors_count;
    - error_factor = msg->u.error_event.error_factor;
    + es.channel = msg->u.error_event.channel;
    + es.status = msg->u.error_event.status;
    + es.txerr = msg->u.error_event.tx_errors_count;
    + es.rxerr = msg->u.error_event.rx_errors_count;
    + es.error_factor = msg->u.error_event.error_factor;
    break;
    case CMD_LOG_MESSAGE:
    - channel = msg->u.log_message.channel;
    - status = msg->u.log_message.data[0];
    - txerr = msg->u.log_message.data[2];
    - rxerr = msg->u.log_message.data[3];
    - error_factor = msg->u.log_message.data[1];
    + es.channel = msg->u.log_message.channel;
    + es.status = msg->u.log_message.data[0];
    + es.txerr = msg->u.log_message.data[2];
    + es.rxerr = msg->u.log_message.data[3];
    + es.error_factor = msg->u.log_message.data[1];
    break;
    case CMD_CHIP_STATE_EVENT:
    - channel = msg->u.chip_state_event.channel;
    - status = msg->u.chip_state_event.status;
    - txerr = msg->u.chip_state_event.tx_errors_count;
    - rxerr = msg->u.chip_state_event.rx_errors_count;
    - error_factor = 0;
    + es.channel = msg->u.chip_state_event.channel;
    + es.status = msg->u.chip_state_event.status;
    + es.txerr = msg->u.chip_state_event.tx_errors_count;
    + es.rxerr = msg->u.chip_state_event.rx_errors_count;
    + es.error_factor = 0;
    break;
    default:
    dev_err(dev->udev->dev.parent, "Invalid msg id (%d)\n",
    @@ -653,122 +706,99 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
    return;
    }

    - if (channel >= dev->nchannels) {
    + if (es.channel >= dev->nchannels) {
    dev_err(dev->udev->dev.parent,
    - "Invalid channel number (%d)\n", channel);
    + "Invalid channel number (%d)\n", es.channel);
    return;
    }

    - priv = dev->nets[channel];
    + priv = dev->nets[es.channel];
    stats = &priv->netdev->stats;

    - if (status & M16C_STATE_BUS_RESET) {
    + if (es.status & M16C_STATE_BUS_RESET) {
    kvaser_usb_unlink_tx_urbs(priv);
    return;
    }

    + /* Update all of the can interface's state and error counters before
    + * trying any skb allocation that can actually fail with -ENOMEM.
    + */
    + old_state = priv->can.state;
    + kvaser_usb_rx_error_update_can_state(priv, &es);
    +
    skb = alloc_can_err_skb(priv->netdev, &cf);
    if (!skb) {
    stats->rx_dropped++;
    return;
    }

    - new_state = priv->can.state;
    -
    - netdev_dbg(priv->netdev, "Error status: 0x%02x\n", status);
    -
    - if (status & M16C_STATE_BUS_OFF) {
    + if (es.status & M16C_STATE_BUS_OFF) {
    cf->can_id |= CAN_ERR_BUSOFF;

    - priv->can.can_stats.bus_off++;
    if (!priv->can.restart_ms)
    kvaser_usb_simple_msg_async(priv, CMD_STOP_CHIP);
    -
    netif_carrier_off(priv->netdev);
    -
    - new_state = CAN_STATE_BUS_OFF;
    - } else if (status & M16C_STATE_BUS_PASSIVE) {
    - if (priv->can.state != CAN_STATE_ERROR_PASSIVE) {
    + } else if (es.status & M16C_STATE_BUS_PASSIVE) {
    + if (old_state != CAN_STATE_ERROR_PASSIVE) {
    cf->can_id |= CAN_ERR_CRTL;

    - if (txerr || rxerr)
    - cf->data[1] = (txerr > rxerr)
    + if (es.txerr || es.rxerr)
    + cf->data[1] = (es.txerr > es.rxerr)
    ? CAN_ERR_CRTL_TX_PASSIVE
    : CAN_ERR_CRTL_RX_PASSIVE;
    else
    cf->data[1] = CAN_ERR_CRTL_TX_PASSIVE |
    CAN_ERR_CRTL_RX_PASSIVE;
    -
    - priv->can.can_stats.error_passive++;
    }
    -
    - new_state = CAN_STATE_ERROR_PASSIVE;
    }

    - if (status == M16C_STATE_BUS_ERROR) {
    - if ((priv->can.state < CAN_STATE_ERROR_WARNING) &&
    - ((txerr >= 96) || (rxerr >= 96))) {
    + if (es.status == M16C_STATE_BUS_ERROR) {
    + if ((old_state < CAN_STATE_ERROR_WARNING) &&
    + ((es.txerr >= 96) || (es.rxerr >= 96))) {
    cf->can_id |= CAN_ERR_CRTL;
    - cf->data[1] = (txerr > rxerr)
    + cf->data[1] = (es.txerr > es.rxerr)
    ? CAN_ERR_CRTL_TX_WARNING
    : CAN_ERR_CRTL_RX_WARNING;
    -
    - priv->can.can_stats.error_warning++;
    - new_state = CAN_STATE_ERROR_WARNING;
    - } else if (priv->can.state > CAN_STATE_ERROR_ACTIVE) {
    + } else if (old_state > CAN_STATE_ERROR_ACTIVE) {
    cf->can_id |= CAN_ERR_PROT;
    cf->data[2] = CAN_ERR_PROT_ACTIVE;
    -
    - new_state = CAN_STATE_ERROR_ACTIVE;
    }
    }

    - if (!status) {
    + if (!es.status) {
    cf->can_id |= CAN_ERR_PROT;
    cf->data[2] = CAN_ERR_PROT_ACTIVE;
    -
    - new_state = CAN_STATE_ERROR_ACTIVE;
    }

    if (priv->can.restart_ms &&
    - (priv->can.state >= CAN_STATE_BUS_OFF) &&
    - (new_state < CAN_STATE_BUS_OFF)) {
    + (old_state >= CAN_STATE_BUS_OFF) &&
    + (priv->can.state < CAN_STATE_BUS_OFF)) {
    cf->can_id |= CAN_ERR_RESTARTED;
    netif_carrier_on(priv->netdev);
    -
    - priv->can.can_stats.restarts++;
    }

    - if (error_factor) {
    - priv->can.can_stats.bus_error++;
    - stats->rx_errors++;
    -
    + if (es.error_factor) {
    cf->can_id |= CAN_ERR_BUSERROR | CAN_ERR_PROT;

    - if (error_factor & M16C_EF_ACKE)
    + if (es.error_factor & M16C_EF_ACKE)
    cf->data[3] |= (CAN_ERR_PROT_LOC_ACK);
    - if (error_factor & M16C_EF_CRCE)
    + if (es.error_factor & M16C_EF_CRCE)
    cf->data[3] |= (CAN_ERR_PROT_LOC_CRC_SEQ |
    CAN_ERR_PROT_LOC_CRC_DEL);
    - if (error_factor & M16C_EF_FORME)
    + if (es.error_factor & M16C_EF_FORME)
    cf->data[2] |= CAN_ERR_PROT_FORM;
    - if (error_factor & M16C_EF_STFE)
    + if (es.error_factor & M16C_EF_STFE)
    cf->data[2] |= CAN_ERR_PROT_STUFF;
    - if (error_factor & M16C_EF_BITE0)
    + if (es.error_factor & M16C_EF_BITE0)
    cf->data[2] |= CAN_ERR_PROT_BIT0;
    - if (error_factor & M16C_EF_BITE1)
    + if (es.error_factor & M16C_EF_BITE1)
    cf->data[2] |= CAN_ERR_PROT_BIT1;
    - if (error_factor & M16C_EF_TRE)
    + if (es.error_factor & M16C_EF_TRE)
    cf->data[2] |= CAN_ERR_PROT_TX;
    }

    - cf->data[6] = txerr;
    - cf->data[7] = rxerr;
    -
    - priv->bec.txerr = txerr;
    - priv->bec.rxerr = rxerr;
    -
    - priv->can.state = new_state;
    + cf->data[6] = es.txerr;
    + cf->data[7] = es.rxerr;

    stats->rx_packets++;
    stats->rx_bytes += cf->can_dlc;
    @@ -792,6 +822,9 @@ static void kvaser_usb_rx_can_err(const struct kvaser_usb_net_priv *priv,
    }

    if (msg->u.rx_can.flag & MSG_FLAG_OVERRUN) {
    + stats->rx_over_errors++;
    + stats->rx_errors++;
    +
    skb = alloc_can_err_skb(priv->netdev, &cf);
    if (!skb) {
    stats->rx_dropped++;
    @@ -801,9 +834,6 @@ static void kvaser_usb_rx_can_err(const struct kvaser_usb_net_priv *priv,
    cf->can_id |= CAN_ERR_CRTL;
    cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;

    - stats->rx_over_errors++;
    - stats->rx_errors++;
    -
    stats->rx_packets++;
    stats->rx_bytes += cf->can_dlc;
    netif_rx(skb);
    --
    1.9.1


    \
     
     \ /
      Last update: 2015-01-20 23:21    [W:4.108 / U:0.152 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site