Hi Matteo,

On Fri, Jul 26, 2019 at 01:19:31AM +0200, Matteo Croce wrote:
> The MTU change code can call napi_disable() with the device already down,
> leading to a deadlock. Also, lot of code is duplicated unnecessarily.
> 
> Rework mvpp2_change_mtu() to avoid the deadlock and remove duplicated code.
> 
> Signed-off-by: Matteo Croce <mcr...@redhat.com>

As this is a fix sent to net, you could add a Fixes: tag.

Otherwise this looks good,
Acked-by: Antoine Tenart <antoine.ten...@bootlin.com>

Thanks!
Antoine

> ---
>  .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 41 ++++++-------------
>  1 file changed, 13 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c 
> b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> index 2f7286bd203b..60eb98f99571 100644
> --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> @@ -3612,6 +3612,7 @@ static int mvpp2_set_mac_address(struct net_device 
> *dev, void *p)
>  static int mvpp2_change_mtu(struct net_device *dev, int mtu)
>  {
>       struct mvpp2_port *port = netdev_priv(dev);
> +     bool running = netif_running(dev);
>       int err;
>  
>       if (!IS_ALIGNED(MVPP2_RX_PKT_SIZE(mtu), 8)) {
> @@ -3620,40 +3621,24 @@ static int mvpp2_change_mtu(struct net_device *dev, 
> int mtu)
>               mtu = ALIGN(MVPP2_RX_PKT_SIZE(mtu), 8);
>       }
>  
> -     if (!netif_running(dev)) {
> -             err = mvpp2_bm_update_mtu(dev, mtu);
> -             if (!err) {
> -                     port->pkt_size =  MVPP2_RX_PKT_SIZE(mtu);
> -                     return 0;
> -             }
> -
> -             /* Reconfigure BM to the original MTU */
> -             err = mvpp2_bm_update_mtu(dev, dev->mtu);
> -             if (err)
> -                     goto log_error;
> -     }
> -
> -     mvpp2_stop_dev(port);
> +     if (running)
> +             mvpp2_stop_dev(port);
>  
>       err = mvpp2_bm_update_mtu(dev, mtu);
> -     if (!err) {
> +     if (err) {
> +             netdev_err(dev, "failed to change MTU\n");
> +             /* Reconfigure BM to the original MTU */
> +             mvpp2_bm_update_mtu(dev, dev->mtu);
> +     } else {
>               port->pkt_size =  MVPP2_RX_PKT_SIZE(mtu);
> -             goto out_start;
>       }
>  
> -     /* Reconfigure BM to the original MTU */
> -     err = mvpp2_bm_update_mtu(dev, dev->mtu);
> -     if (err)
> -             goto log_error;
> -
> -out_start:
> -     mvpp2_start_dev(port);
> -     mvpp2_egress_enable(port);
> -     mvpp2_ingress_enable(port);
> +     if (running) {
> +             mvpp2_start_dev(port);
> +             mvpp2_egress_enable(port);
> +             mvpp2_ingress_enable(port);
> +     }
>  
> -     return 0;
> -log_error:
> -     netdev_err(dev, "failed to change MTU\n");
>       return err;
>  }
>  
> -- 
> 2.21.0
> 

-- 
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Reply via email to