diff mbox series

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

Message ID 20220530231512.9729-3-dave@stgolabs.net
State New
Headers show
Series scsi: Replace tasklets as BH | expand

Commit Message

Davidlohr Bueso May 30, 2022, 11:15 p.m. UTC 
Tasklets have long been deprecated as being too heavy on the system
by running in irq context - and this is not a performance critical
path. If a higher priority process wants to run, it must wait for
the tasklet to finish before doing so. A more suitable equivalent
is to converted to threaded irq instead and do the ack sequence
in task context.

Cc: Kashyap Desai <kashyap.desai@broadcom.com>
Cc: Sumit Saxena <sumit.saxena@broadcom.com>
Cc: Shivasharan S <shivasharan.srikanteshwara@broadcom.com>
Cc: megaraidlinux.pdl@broadcom.com
Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
---
 drivers/scsi/megaraid/mega_common.h   |  2 --
 drivers/scsi/megaraid/megaraid_mbox.c | 52 ++++++++++-----------------
 2 files changed, 19 insertions(+), 35 deletions(-)

Comments

Sebastian Andrzej Siewior June 2, 2022, 8:36 a.m. UTC | #1 
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
diff mbox series

Patch

diff --git a/drivers/scsi/megaraid/mega_common.h b/drivers/scsi/megaraid/mega_common.h
index 2ad0aa2f837d..3e56f74061b4 100644
--- a/drivers/scsi/megaraid/mega_common.h
+++ b/drivers/scsi/megaraid/mega_common.h
@@ -95,7 +95,6 @@  typedef struct {
 
 /**
  * struct adapter_t - driver's initialization structure
- * @aram dpc_h			: tasklet handle
  * @pdev			: pci configuration pointer for kernel
  * @host			: pointer to host structure of mid-layer
  * @lock			: synchronization lock for mid-layer and driver
@@ -149,7 +148,6 @@  typedef struct {
 #define VERSION_SIZE	16
 
 typedef struct {
-	struct tasklet_struct	dpc_h;
 	struct pci_dev		*pdev;
 	struct Scsi_Host	*host;
 	spinlock_t		lock;
diff --git a/drivers/scsi/megaraid/megaraid_mbox.c b/drivers/scsi/megaraid/megaraid_mbox.c
index 2a339d4a7e9d..b76f67887592 100644
--- a/drivers/scsi/megaraid/megaraid_mbox.c
+++ b/drivers/scsi/megaraid/megaraid_mbox.c
@@ -118,8 +118,7 @@  static void megaraid_mbox_prepare_epthru(adapter_t *, scb_t *,
 		struct scsi_cmnd *);
 
 static irqreturn_t megaraid_isr(int, void *);
-
-static void megaraid_mbox_dpc(unsigned long);
+static irqreturn_t megaraid_mbox_dpc(int, void *);
 
 static ssize_t megaraid_mbox_app_hndl_show(struct device *, struct device_attribute *attr, char *);
 static ssize_t megaraid_mbox_ld_show(struct device *, struct device_attribute *attr, char *);
@@ -764,9 +763,8 @@  megaraid_init_mbox(adapter_t *adapter)
 	 */
 
 	/* request IRQ and register the interrupt service routine */
-	if (request_irq(adapter->irq, megaraid_isr, IRQF_SHARED, "megaraid",
-		adapter)) {
-
+	if (request_threaded_irq(adapter->irq, megaraid_isr, megaraid_mbox_dpc,
+				 IRQF_SHARED, "megaraid", adapter)) {
 		con_log(CL_ANN, (KERN_WARNING
 			"megaraid: Couldn't register IRQ %d!\n", adapter->irq));
 		goto out_alloc_cmds;
@@ -879,10 +877,6 @@  megaraid_init_mbox(adapter_t *adapter)
 		}
 	}
 
-	// setup tasklet for DPC
-	tasklet_init(&adapter->dpc_h, megaraid_mbox_dpc,
-			(unsigned long)adapter);
-
 	con_log(CL_DLEVEL1, (KERN_INFO
 		"megaraid mbox hba successfully initialized\n"));
 
@@ -917,8 +911,6 @@  megaraid_fini_mbox(adapter_t *adapter)
 	// flush all caches
 	megaraid_mbox_flush_cache(adapter);
 
-	tasklet_kill(&adapter->dpc_h);
-
 	megaraid_sysfs_free_resources(adapter);
 
 	megaraid_free_cmd_packets(adapter);
@@ -2027,7 +2019,7 @@  megaraid_mbox_prepare_epthru(adapter_t *adapter, scb_t *scb,
  *
  * Returns:	1 if the interrupt is valid, 0 otherwise
  */
-static int
+static irqreturn_t
 megaraid_ack_sequence(adapter_t *adapter)
 {
 	mraid_device_t		*raid_dev = ADAP2RAIDDEV(adapter);
@@ -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;
 	uint32_t		dword;
 	unsigned long		flags;
 	int			i, j;
@@ -2048,7 +2040,6 @@  megaraid_ack_sequence(adapter_t *adapter)
 	INIT_LIST_HEAD(&clist);
 
 	// loop till F/W has more commands for us to complete
-	handled = 0;
 	spin_lock_irqsave(MAILBOX_LOCK(raid_dev), flags);
 	do {
 		/*
@@ -2056,9 +2047,10 @@  megaraid_ack_sequence(adapter_t *adapter)
 		 * interrupt line low.
 		 */
 		dword = RDOUTDOOR(raid_dev);
-		if (dword != 0x10001234) break;
+		if (dword != 0x10001234)
+			break;
 
-		handled = 1;
+		ret = IRQ_WAKE_THREAD;
 
 		WROUTDOOR(raid_dev, 0x10001234);
 
@@ -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;
 }
 
 
@@ -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;
 	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;
 
 	raid_dev = ADAP2RAIDDEV(adapter);
 
@@ -2361,8 +2347,8 @@  megaraid_mbox_dpc(unsigned long devp)
 		// send the scsi packet back to kernel
 		scsi_done(scp);
 	}
-
-	return;
+done:
+	return IRQ_HANDLED;
 }