Re: [PATCH net-next v4 01/10] bng_en: add per-PF workqueue, timer, and slow-path task

[Jakub Kicinski <kuba@kernel.org>]

On Fri,  6 Mar 2026 01:30:08 +0530 Bhargava Marreddy wrote:
> +static void bnge_sp_task(struct work_struct *work)
> +{
> +	struct bnge_net *bn = container_of(work, struct bnge_net, sp_task);
> +	struct bnge_dev *bd = bn->bd;
> +
> +	set_bit(BNGE_STATE_IN_SP_TASK, &bn->state);
> +	/* Ensure sp_task state is visible before checking BNGE_STATE_OPEN */
> +	smp_mb__after_atomic();
> +	if (!test_bit(BNGE_STATE_OPEN, &bd->state)) {
> +		clear_bit(BNGE_STATE_IN_SP_TASK, &bn->state);
> +		return;
> +	}
> +
> +	/* Event handling work added by later patches */
> +
> +	/* Ensure all sp_task work is done before clearing the state */
> +	smp_mb__before_atomic();
> +	clear_bit(BNGE_STATE_IN_SP_TASK, &bn->state);
> +}
> +
> +static bool bnge_drv_busy(struct bnge_net *bn)
> +{
> +	return test_bit(BNGE_STATE_IN_SP_TASK, &bn->state);
> +}

> @@ -2542,6 +2584,12 @@ static void bnge_close_core(struct bnge_net *bn)
>  	bnge_tx_disable(bn);
>  
>  	clear_bit(BNGE_STATE_OPEN, &bd->state);
> +	/* Ensure BNGE_STATE_OPEN is cleared before checking drv_busy */
> +	smp_mb__after_atomic();
> +	while (bnge_drv_busy(bn))
> +		msleep(20);

AI code review points out that patch 2 will cause a deadlock.
This waits for the service task under the instance lock.
But service task will try to acquire that lock in patch 2.

I previously complained about flag-based-locking in the stats,
this is similarly problematic. Can we make the service task
take the instance lock across the entire function?
And check if OPEN is set under the instance lock?
-- 
pw-bot: cr