Re: [PATCH 2/5] ice: retain the adapter throughout hardware initialization and teardown

[Przemek Kitszel]

On 10/19/26 12:51, Aaron Ma wrote:
> Adapter-scoped synchronization must be available to Admin Queue users
> from initial queue creation through the last teardown command. Probe
> currently obtains its adapter reference only after hardware and package
> initialization, and error cleanup and removal drop it before hardware
> teardown.
>
> Acquire the reference before either normal hardware initialization or
> firmware recovery-mode initialization. Release it after hardware and
> device cleanup, balancing recovery-mode probe failures and removal too.
>
> This establishes the lifetime prerequisite for moving package ownership
> exclusion from a module-global mutex to the shared adapter.
>
> Signed-off-by: Aaron Ma <aaron.ma@xxxxxxxxxxxxx>

it was me to move hw init before adapter init, to make hw details
readily accessible to the adapter

but I agree that SW lock protection covering HW reinit will be great,
and my needs could be achieved by other means, even if less elegant

so, I'm happy with your patch
OTOH I have a big series that git-conflicts, would be easiest if your
series will be merged first (so I will adjust), but I don't want to be
delayed (will be CCing you on it going forward)

the real thing is of course the 3rd patch not this one

> ---
>   drivers/net/ethernet/intel/ice/ice_main.c | 31 +++++++++++++----------
>   1 file changed, 17 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
> index 164428ba116a0..ddebb28b2454f 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -5242,27 +5242,29 @@ ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent)
>   		hw->debug_mask = debug;
>   #endif
>   
> -	if (ice_is_recovery_mode(hw))
> -		return ice_probe_recovery_mode(pf);
> +	adapter = ice_adapter_get(pdev);
> +	if (IS_ERR(adapter))
> +		return PTR_ERR(adapter);
> +	pf->adapter = adapter;
> +
> +	if (ice_is_recovery_mode(hw)) {
> +		err = ice_probe_recovery_mode(pf);
> +		if (err)
> +			ice_adapter_put(pdev);
> +		return err;
> +	}
>   
>   	err = ice_init_hw(hw);
>   	if (err) {
>   		dev_err(dev, "ice_init_hw failed: %d\n", err);
> -		return err;
> +		goto unroll_adapter;
>   	}
>   
>   	ice_init_dev_hw(pf);
>   
> -	adapter = ice_adapter_get(pdev);
> -	if (IS_ERR(adapter)) {
> -		err = PTR_ERR(adapter);
> -		goto unroll_hw_init;
> -	}
> -	pf->adapter = adapter;
> -
>   	err = ice_init_dev(pf);
>   	if (err)
> -		goto unroll_adapter;
> +		goto unroll_hw_init;
>   
>   	err = ice_init(pf);
>   	if (err)
> @@ -5287,12 +5289,12 @@ ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent)
>   	ice_deinit(pf);
>   unroll_dev_init:
>   	need_dev_deinit = true;
> -unroll_adapter:
> -	ice_adapter_put(pdev);
>   unroll_hw_init:
>   	ice_deinit_hw(hw);
>   	if (need_dev_deinit)
>   		ice_deinit_dev(pf);
> +unroll_adapter:
> +	ice_adapter_put(pdev);
>   	return err;
>   }
>   
> @@ -5377,6 +5379,7 @@ static void ice_remove(struct pci_dev *pdev)
>   		scoped_guard(devl, priv_to_devlink(pf)) {
>   			ice_deinit_devlink(pf);
>   		}
> +		ice_adapter_put(pdev);
>   		return;
>   	}
>   
> @@ -5403,11 +5406,11 @@ static void ice_remove(struct pci_dev *pdev)
>   	ice_setup_mc_magic_wake(pf);
>   	ice_set_wake(pf);
>   
> -	ice_adapter_put(pdev);
>   	ice_deinit_hw(&pf->hw);
>   
>   	ice_deinit_dev(pf);
>   	ice_aq_cancel_waiting_tasks(pf);
> +	ice_adapter_put(pdev);
>   	set_bit(ICE_DOWN, pf->state);
>   }
>