Re: [Intel-wired-lan] [PATCH net] ice: ice_vsi_release cleanup

[Petr Oros]

Maciej Fijalkowski píše v Po 19. 06. 2023 v 14:38 +0200:
> On Mon, Jun 19, 2023 at 10:49:48AM +0200, Petr Oros wrote:
> > Since commit 6624e780a577fc ("ice: split ice_vsi_setup into smaller
> > functions") ice_vsi_release does things twice. There is unregister
> > netdev which is unregistered in ice_deinit_eth also.
> > 
> > It also unregisters the devlink_port twice which is also
> > unregistered
> > in ice_deinit_eth(). This double deregistration is hidden because
> > devl_port_unregister ignores the return value of xa_erase.
> > 
> > [   68.642167] Call Trace:
> > [   68.650385]  ice_devlink_destroy_pf_port+0xe/0x20 [ice]
> > [   68.655656]  ice_vsi_release+0x445/0x690 [ice]
> > [   68.660147]  ice_deinit+0x99/0x280 [ice]
> > [   68.664117]  ice_remove+0x1b6/0x5c0 [ice]
> > 
> > [  171.103841] Call Trace:
> > [  171.109607]  ice_devlink_destroy_pf_port+0xf/0x20 [ice]
> > [  171.114841]  ice_remove+0x158/0x270 [ice]
> > [  171.118854]  pci_device_remove+0x3b/0xc0
> > [  171.122779]  device_release_driver_internal+0xc7/0x170
> > [  171.127912]  driver_detach+0x54/0x8c
> > [  171.131491]  bus_remove_driver+0x77/0xd1
> > [  171.135406]  pci_unregister_driver+0x2d/0xb0
> > [  171.139670]  ice_module_exit+0xc/0x55f [ice]
> 
> Hi Petr,
> can you tell us when in particular this call trace was observed?

I found out that the devlink port is deregistered twice. This calltrace
was generated by dump_stack() placed in devlink_port_unregister -> it
is testing trace, not from production.

Regards,
Petr


> 
> CC: Michal Swiatkowski <michal.swiatkowski@xxxxxxxxx>
> 
> > 
> > Fixes: 6624e780a577 ("ice: split ice_vsi_setup into smaller
> > functions")
> > Signed-off-by: Petr Oros <poros@xxxxxxxxxx>
> > ---
> >  drivers/net/ethernet/intel/ice/ice_lib.c | 27 --------------------
> > ----
> >  1 file changed, 27 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c
> > b/drivers/net/ethernet/intel/ice/ice_lib.c
> > index 11ae0e41f518a1..284a1f0bfdb545 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_lib.c
> > +++ b/drivers/net/ethernet/intel/ice/ice_lib.c
> > @@ -3272,39 +3272,12 @@ int ice_vsi_release(struct ice_vsi *vsi)
> >                 return -ENODEV;
> >         pf = vsi->back;
> >  
> > -       /* do not unregister while driver is in the reset recovery
> > pending
> > -        * state. Since reset/rebuild happens through PF service
> > task workqueue,
> > -        * it's not a good idea to unregister netdev that is
> > associated to the
> > -        * PF that is running the work queue items currently. This
> > is done to
> > -        * avoid check_flush_dependency() warning on this wq
> > -        */
> > -       if (vsi->netdev && !ice_is_reset_in_progress(pf->state) &&
> > -           (test_bit(ICE_VSI_NETDEV_REGISTERED, vsi->state))) {
> > -               unregister_netdev(vsi->netdev);
> > -               clear_bit(ICE_VSI_NETDEV_REGISTERED, vsi->state);
> > -       }
> > -
> > -       if (vsi->type == ICE_VSI_PF)
> > -               ice_devlink_destroy_pf_port(pf);
> > -
> >         if (test_bit(ICE_FLAG_RSS_ENA, pf->flags))
> >                 ice_rss_clean(vsi);
> >  
> >         ice_vsi_close(vsi);
> >         ice_vsi_decfg(vsi);
> >  
> > -       if (vsi->netdev) {
> > -               if (test_bit(ICE_VSI_NETDEV_REGISTERED, vsi-
> > >state)) {
> > -                       unregister_netdev(vsi->netdev);
> > -                       clear_bit(ICE_VSI_NETDEV_REGISTERED, vsi-
> > >state);
> > -               }
> > -               if (test_bit(ICE_VSI_NETDEV_ALLOCD, vsi->state)) {
> > -                       free_netdev(vsi->netdev);
> > -                       vsi->netdev = NULL;
> > -                       clear_bit(ICE_VSI_NETDEV_ALLOCD, vsi-
> > >state);
> > -               }
> > -       }
> > -
> >         /* retain SW VSI data structure since it is needed to
> > unregister and
> >          * free VSI netdev when PF is not in reset recovery pending
> > state,\
> >          * for ex: during rmmod.
> > -- 
> > 2.41.0
> > 
> > _______________________________________________
> > Intel-wired-lan mailing list
> > Intel-wired-lan@xxxxxxxxxx
> > https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
>