Re: [PATCH 02/10] scsi/megaraid: Replace adapter->dpc_h tasklet with threaded irq

[Sebastian Andrzej Siewior <bigeasy@linutronix.de>]

On 2022-05-30 16:15:04 [-0700], Davidlohr Bueso wrote:
> @@ -2036,7 +2028,7 @@ megaraid_ack_sequence(adapter_t *adapter)
>  	uint8_t			nstatus;
>  	uint8_t			completed[MBOX_MAX_FIRMWARE_STATUS];
>  	struct list_head	clist;
> -	int			handled;
> +	int			ret = IRQ_NONE;

irqreturn_t ret = IRQ_NONE;

>  	uint32_t		dword;
>  	unsigned long		flags;
>  	int			i, j;
> @@ -2124,12 +2116,7 @@ megaraid_ack_sequence(adapter_t *adapter)
>  
>  	spin_unlock_irqrestore(COMPLETED_LIST_LOCK(adapter), flags);
>  
> -
> -	// schedule the DPC if there is some work for it
> -	if (handled)
> -		tasklet_schedule(&adapter->dpc_h);
> -
> -	return handled;
> +	return ret;
>  }

megaraid_ack_sequence() has another caller, the
scsi_host_template::eh_host_reset_handler callback
(megaraid_reset_handler). With that change, that threaded handler will
not be invoked in the reset case.

> @@ -2144,29 +2131,27 @@ static irqreturn_t
>  megaraid_isr(int irq, void *devp)
>  {
>  	adapter_t	*adapter = devp;
> -	int		handled;
> +	int		ret;
>  
> -	handled = megaraid_ack_sequence(adapter);
> +	ret = megaraid_ack_sequence(adapter);
>  
>  	/* Loop through any pending requests */
>  	if (!adapter->quiescent) {
>  		megaraid_mbox_runpendq(adapter, NULL);
>  	}
>  
> -	return IRQ_RETVAL(handled);
> +	return ret;
>  }
>  
>  
>  /**
> - * megaraid_mbox_dpc - the tasklet to complete the commands from completed list
> - * @devp	: pointer to HBA soft state
> + * megaraid_mbox_dpc - complete the commands from completed list
>   *
>   * Pick up the commands from the completed list and send back to the owners.
>   * This is a reentrant function and does not assume any locks are held while
> - * it is being called.
> + * it is being called. Runs in process context.
>   */
> -static void
> -megaraid_mbox_dpc(unsigned long devp)
> +static irqreturn_t megaraid_mbox_dpc(int irq, void *devp)
>  {
>  	adapter_t		*adapter = (adapter_t *)devp;

that cast could vanish.

>  	mraid_device_t		*raid_dev;
> @@ -2188,7 +2173,8 @@ megaraid_mbox_dpc(unsigned long devp)
>  	uioc_t			*kioc;
>  
>  
> -	if (!adapter) return;
> +	if (!adapter)
> +		goto done;

return IRQ_NONE;

>  	raid_dev = ADAP2RAIDDEV(adapter);
>  

Sebastian