Re: [RFC PATCH] scsi: core: Drop Scsi_Host.default_lock

On 8/4/26 4:34 AM, John Garry wrote:
> Getting the address of embedded host_lock structure just requires adding
> a fixed offset value to the shost pointer. However, getting the value of
> the host_lock pointer requires loading from a fixed offset to the shost
> pointer. The latter should be very slightly slower, which is relevant as
> this lock is used a lot throughout the core code and drivers.

I like the idea behind this patch. However, I think this patch is
incomplete. The changes shown below are needed to make all SCSI drivers
build again with the script available at
https://github.com/bvanassche/build-scsi-drivers:

diff --git a/drivers/s390/scsi/zfcp_sysfs.c b/drivers/s390/scsi/zfcp_sysfs.c
index 4f23d585d062..b1ed0cb30a72 100644
--- a/drivers/s390/scsi/zfcp_sysfs.c
+++ b/drivers/s390/scsi/zfcp_sysfs.c
@@ -270,7 +270,7 @@ static bool zfcp_sysfs_port_in_use(struct zfcp_port 
*const port)
  	if (atomic_read(&port->units) > 0)
  		goto unlock_port_units_mutex; /* zfcp_unit(s) under port */

-	spin_lock_irqsave(adapter->scsi_host->host_lock, flags);
+	spin_lock_irqsave(&adapter->scsi_host->host_lock, flags);
  	__shost_for_each_device(sdev, adapter->scsi_host) {
  		const struct zfcp_scsi_dev *zsdev = sdev_to_zfcp(sdev);

@@ -288,7 +288,7 @@ static bool zfcp_sysfs_port_in_use(struct zfcp_port 
*const port)
  	in_use = false;

  unlock_host_lock:
-	spin_unlock_irqrestore(adapter->scsi_host->host_lock, flags);
+	spin_unlock_irqrestore(&adapter->scsi_host->host_lock, flags);
  unlock_port_units_mutex:
  	mutex_unlock(&zfcp_sysfs_port_units_mutex);
  	return in_use;
diff --git a/drivers/scsi/a2091.c b/drivers/scsi/a2091.c
index f81e53b53e20..223065529654 100644
--- a/drivers/scsi/a2091.c
+++ b/drivers/scsi/a2091.c
@@ -39,9 +39,9 @@ static irqreturn_t a2091_intr(int irq, void *data)
  	if (!(status & (ISTR_INT_F | ISTR_INT_P)) || !(status & ISTR_INTS))
  		return IRQ_NONE;

-	spin_lock_irqsave(instance->host_lock, flags);
+	spin_lock_irqsave(&instance->host_lock, flags);
  	wd33c93_intr(instance);
-	spin_unlock_irqrestore(instance->host_lock, flags);
+	spin_unlock_irqrestore(&instance->host_lock, flags);
  	return IRQ_HANDLED;
  }

diff --git a/drivers/scsi/a3000.c b/drivers/scsi/a3000.c
index bf054dd7682b..dd0924572c0e 100644
--- a/drivers/scsi/a3000.c
+++ b/drivers/scsi/a3000.c
@@ -41,9 +41,9 @@ static irqreturn_t a3000_intr(int irq, void *data)
  	if (!(status & ISTR_INT_P))
  		return IRQ_NONE;
  	if (status & ISTR_INTS) {
-		spin_lock_irqsave(instance->host_lock, flags);
+		spin_lock_irqsave(&instance->host_lock, flags);
  		wd33c93_intr(instance);
-		spin_unlock_irqrestore(instance->host_lock, flags);
+		spin_unlock_irqrestore(&instance->host_lock, flags);
  		return IRQ_HANDLED;
  	}
  	pr_warn("Non-serviced A3000 SCSI-interrupt? ISTR = %02x\n", status);
diff --git a/drivers/scsi/gvp11.c b/drivers/scsi/gvp11.c
index 79bd64e12adc..a29457f5adfb 100644
--- a/drivers/scsi/gvp11.c
+++ b/drivers/scsi/gvp11.c
@@ -42,9 +42,9 @@ static irqreturn_t gvp11_intr(int irq, void *data)
  	if (!(status & GVP11_DMAC_INT_PENDING))
  		return IRQ_NONE;

-	spin_lock_irqsave(instance->host_lock, flags);
+	spin_lock_irqsave(&instance->host_lock, flags);
  	wd33c93_intr(instance);
-	spin_unlock_irqrestore(instance->host_lock, flags);
+	spin_unlock_irqrestore(&instance->host_lock, flags);
  	return IRQ_HANDLED;
  }

diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c 
b/drivers/scsi/ibmvscsi/ibmvfc-core.c
index 4ea5d93ab1a8..594108269dec 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c
@@ -1642,9 +1642,9 @@ static void ibmvfc_locked_done(struct ibmvfc_event 
*evt)
  {
  	unsigned long flags;

-	spin_lock_irqsave(evt->vhost->host->host_lock, flags);
+	spin_lock_irqsave(&evt->vhost->host->host_lock, flags);
  	evt->_done(evt);
-	spin_unlock_irqrestore(evt->vhost->host->host_lock, flags);
+	spin_unlock_irqrestore(&evt->vhost->host->host_lock, flags);
  }

  /**
diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c 
b/drivers/scsi/ibmvscsi/ibmvscsi.c
index eeaba31b18c3..4ecd3db08875 100644
--- a/drivers/scsi/ibmvscsi/ibmvscsi.c
+++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
@@ -785,13 +785,13 @@ static void purge_requests(struct 
ibmvscsi_host_data *hostdata, int error_code)
  	struct srp_event_struct *evt;
  	unsigned long flags;

-	spin_lock_irqsave(hostdata->host->host_lock, flags);
+	spin_lock_irqsave(&hostdata->host->host_lock, flags);
  	while (!list_empty(&hostdata->sent)) {
  		evt = list_first_entry(&hostdata->sent, struct srp_event_struct, list);
  		list_del(&evt->list);
  		timer_delete(&evt->timer);

-		spin_unlock_irqrestore(hostdata->host->host_lock, flags);
+		spin_unlock_irqrestore(&hostdata->host->host_lock, flags);
  		if (evt->cmnd) {
  			evt->cmnd->result = (error_code << 16);
  			unmap_cmd_data(&evt->iu.srp.cmd, evt,
@@ -802,9 +802,9 @@ static void purge_requests(struct ibmvscsi_host_data 
*hostdata, int error_code)
  			   evt->iu.srp.login_req.opcode != SRP_LOGIN_REQ)
  			evt->done(evt);
  		free_event_struct(&evt->hostdata->pool, evt);
-		spin_lock_irqsave(hostdata->host->host_lock, flags);
+		spin_lock_irqsave(&hostdata->host->host_lock, flags);
  	}
-	spin_unlock_irqrestore(hostdata->host->host_lock, flags);
+	spin_unlock_irqrestore(&hostdata->host->host_lock, flags);
  }

  /**
@@ -818,9 +818,9 @@ static void ibmvscsi_set_request_limit(struct 
ibmvscsi_host_data *hostdata, int
  {
  	unsigned long flags;

-	spin_lock_irqsave(hostdata->host->host_lock, flags);
+	spin_lock_irqsave(&hostdata->host->host_lock, flags);
  	atomic_set(&hostdata->request_limit, limit);
-	spin_unlock_irqrestore(hostdata->host->host_lock, flags);
+	spin_unlock_irqrestore(&hostdata->host->host_lock, flags);
  }

  /**
@@ -1221,9 +1221,9 @@ static int send_srp_login(struct 
ibmvscsi_host_data *hostdata)
  	 */
  	ibmvscsi_set_request_limit(hostdata, 0);

-	spin_lock_irqsave(hostdata->host->host_lock, flags);
+	spin_lock_irqsave(&hostdata->host->host_lock, flags);
  	rc = ibmvscsi_send_srp_event(evt_struct, hostdata, login_timeout * 2);
-	spin_unlock_irqrestore(hostdata->host->host_lock, flags);
+	spin_unlock_irqrestore(&hostdata->host->host_lock, flags);
  	dev_info(hostdata->dev, "sent SRP login\n");
  	return rc;
  };
@@ -1317,10 +1317,10 @@ static void send_mad_capabilities(struct 
ibmvscsi_host_data *hostdata)
  		req->common.length = cpu_to_be16(sizeof(hostdata->caps) -
  						sizeof(hostdata->caps.reserve));

-	spin_lock_irqsave(hostdata->host->host_lock, flags);
+	spin_lock_irqsave(&hostdata->host->host_lock, flags);
  	if (ibmvscsi_send_srp_event(evt_struct, hostdata, info_timeout * 2))
  		dev_err(hostdata->dev, "couldn't send CAPABILITIES_REQ!\n");
-	spin_unlock_irqrestore(hostdata->host->host_lock, flags);
+	spin_unlock_irqrestore(&hostdata->host->host_lock, flags);
  };

  /**
@@ -1373,9 +1373,9 @@ static int enable_fast_fail(struct 
ibmvscsi_host_data *hostdata)
  	fast_fail_mad->common.type = cpu_to_be32(VIOSRP_ENABLE_FAST_FAIL);
  	fast_fail_mad->common.length = cpu_to_be16(sizeof(*fast_fail_mad));

-	spin_lock_irqsave(hostdata->host->host_lock, flags);
+	spin_lock_irqsave(&hostdata->host->host_lock, flags);
  	rc = ibmvscsi_send_srp_event(evt_struct, hostdata, info_timeout * 2);
-	spin_unlock_irqrestore(hostdata->host->host_lock, flags);
+	spin_unlock_irqrestore(&hostdata->host->host_lock, flags);
  	return rc;
  }

@@ -1454,10 +1454,10 @@ static void send_mad_adapter_info(struct 
ibmvscsi_host_data *hostdata)
  	req->common.length = cpu_to_be16(sizeof(hostdata->madapter_info));
  	req->buffer = cpu_to_be64(hostdata->adapter_info_addr);

-	spin_lock_irqsave(hostdata->host->host_lock, flags);
+	spin_lock_irqsave(&hostdata->host->host_lock, flags);
  	if (ibmvscsi_send_srp_event(evt_struct, hostdata, info_timeout * 2))
  		dev_err(hostdata->dev, "couldn't send ADAPTER_INFO_REQ!\n");
-	spin_unlock_irqrestore(hostdata->host->host_lock, flags);
+	spin_unlock_irqrestore(&hostdata->host->host_lock, flags);
  };

  /*
@@ -1502,7 +1502,7 @@ static int ibmvscsi_eh_abort_handler(struct 
scsi_cmnd *cmd)
  	/* First, find this command in our sent list so we can figure
  	 * out the correct tag
  	 */
-	spin_lock_irqsave(hostdata->host->host_lock, flags);
+	spin_lock_irqsave(&hostdata->host->host_lock, flags);
  	wait_switch = jiffies + (init_timeout * HZ);
  	do {
  		found_evt = NULL;
@@ -1514,13 +1514,13 @@ static int ibmvscsi_eh_abort_handler(struct 
scsi_cmnd *cmd)
  		}

  		if (!found_evt) {
-			spin_unlock_irqrestore(hostdata->host->host_lock, flags);
+			spin_unlock_irqrestore(&hostdata->host->host_lock, flags);
  			return SUCCESS;
  		}

  		evt = get_event_struct(&hostdata->pool);
  		if (evt == NULL) {
-			spin_unlock_irqrestore(hostdata->host->host_lock, flags);
+			spin_unlock_irqrestore(&hostdata->host->host_lock, flags);
  			sdev_printk(KERN_ERR, cmd->device,
  				"failed to allocate abort event\n");
  			return FAILED;
@@ -1548,12 +1548,12 @@ static int ibmvscsi_eh_abort_handler(struct 
scsi_cmnd *cmd)
  		if (rsp_rc != SCSI_MLQUEUE_HOST_BUSY)
  			break;

-		spin_unlock_irqrestore(hostdata->host->host_lock, flags);
+		spin_unlock_irqrestore(&hostdata->host->host_lock, flags);
  		msleep(10);
-		spin_lock_irqsave(hostdata->host->host_lock, flags);
+		spin_lock_irqsave(&hostdata->host->host_lock, flags);
  	} while (time_before(jiffies, wait_switch));

-	spin_unlock_irqrestore(hostdata->host->host_lock, flags);
+	spin_unlock_irqrestore(&hostdata->host->host_lock, flags);

  	if (rsp_rc != 0) {
  		sdev_printk(KERN_ERR, cmd->device,
@@ -1592,7 +1592,7 @@ static int ibmvscsi_eh_abort_handler(struct 
scsi_cmnd *cmd)
  	 * The event is no longer in our list.  Make sure it didn't
  	 * complete while we were aborting
  	 */
-	spin_lock_irqsave(hostdata->host->host_lock, flags);
+	spin_lock_irqsave(&hostdata->host->host_lock, flags);
  	found_evt = NULL;
  	list_for_each_entry(tmp_evt, &hostdata->sent, list) {
  		if (tmp_evt->cmnd == cmd) {
@@ -1602,7 +1602,7 @@ static int ibmvscsi_eh_abort_handler(struct 
scsi_cmnd *cmd)
  	}

  	if (found_evt == NULL) {
-		spin_unlock_irqrestore(hostdata->host->host_lock, flags);
+		spin_unlock_irqrestore(&hostdata->host->host_lock, flags);
  		sdev_printk(KERN_INFO, cmd->device, "aborted task tag 0x%llx 
completed\n",
  			    tsk_mgmt->task_tag);
  		return SUCCESS;
@@ -1616,7 +1616,7 @@ static int ibmvscsi_eh_abort_handler(struct 
scsi_cmnd *cmd)
  	unmap_cmd_data(&found_evt->iu.srp.cmd, found_evt,
  		       found_evt->hostdata->dev);
  	free_event_struct(&found_evt->hostdata->pool, found_evt);
-	spin_unlock_irqrestore(hostdata->host->host_lock, flags);
+	spin_unlock_irqrestore(&hostdata->host->host_lock, flags);
  	atomic_inc(&hostdata->request_limit);
  	return SUCCESS;
  }
@@ -1638,12 +1638,12 @@ static int 
ibmvscsi_eh_device_reset_handler(struct scsi_cmnd *cmd)
  	u16 lun = lun_from_dev(cmd->device);
  	unsigned long wait_switch = 0;

-	spin_lock_irqsave(hostdata->host->host_lock, flags);
+	spin_lock_irqsave(&hostdata->host->host_lock, flags);
  	wait_switch = jiffies + (init_timeout * HZ);
  	do {
  		evt = get_event_struct(&hostdata->pool);
  		if (evt == NULL) {
-			spin_unlock_irqrestore(hostdata->host->host_lock, flags);
+			spin_unlock_irqrestore(&hostdata->host->host_lock, flags);
  			sdev_printk(KERN_ERR, cmd->device,
  				"failed to allocate reset event\n");
  			return FAILED;
@@ -1670,12 +1670,12 @@ static int 
ibmvscsi_eh_device_reset_handler(struct scsi_cmnd *cmd)
  		if (rsp_rc != SCSI_MLQUEUE_HOST_BUSY)
  			break;

-		spin_unlock_irqrestore(hostdata->host->host_lock, flags);
+		spin_unlock_irqrestore(&hostdata->host->host_lock, flags);
  		msleep(10);
-		spin_lock_irqsave(hostdata->host->host_lock, flags);
+		spin_lock_irqsave(&hostdata->host->host_lock, flags);
  	} while (time_before(jiffies, wait_switch));

-	spin_unlock_irqrestore(hostdata->host->host_lock, flags);
+	spin_unlock_irqrestore(&hostdata->host->host_lock, flags);

  	if (rsp_rc != 0) {
  		sdev_printk(KERN_ERR, cmd->device,
@@ -1712,7 +1712,7 @@ static int ibmvscsi_eh_device_reset_handler(struct 
scsi_cmnd *cmd)
  	/* We need to find all commands for this LUN that have not yet been
  	 * responded to, and fail them with DID_RESET
  	 */
-	spin_lock_irqsave(hostdata->host->host_lock, flags);
+	spin_lock_irqsave(&hostdata->host->host_lock, flags);
  	list_for_each_entry_safe(tmp_evt, pos, &hostdata->sent, list) {
  		if ((tmp_evt->cmnd) && (tmp_evt->cmnd->device == cmd->device)) {
  			if (tmp_evt->cmnd)
@@ -1729,7 +1729,7 @@ static int ibmvscsi_eh_device_reset_handler(struct 
scsi_cmnd *cmd)
  				tmp_evt->done(tmp_evt);
  		}
  	}
-	spin_unlock_irqrestore(hostdata->host->host_lock, flags);
+	spin_unlock_irqrestore(&hostdata->host->host_lock, flags);
  	return SUCCESS;
  }

@@ -1855,10 +1855,10 @@ static void ibmvscsi_handle_crq(struct 
viosrp_crq *crq,
  	 * Lock the host_lock before messing with these structures, since we
  	 * are running in a task context
  	 */
-	spin_lock_irqsave(evt_struct->hostdata->host->host_lock, flags);
+	spin_lock_irqsave(&evt_struct->hostdata->host->host_lock, flags);
  	list_del(&evt_struct->list);
  	free_event_struct(&evt_struct->hostdata->pool, evt_struct);
-	spin_unlock_irqrestore(evt_struct->hostdata->host->host_lock, flags);
+	spin_unlock_irqrestore(&evt_struct->hostdata->host->host_lock, flags);
  }

  /**
@@ -2131,35 +2131,35 @@ static void ibmvscsi_do_work(struct 
ibmvscsi_host_data *hostdata)
  	int rc;
  	char *action = "reset";

-	spin_lock_irqsave(hostdata->host->host_lock, flags);
+	spin_lock_irqsave(&hostdata->host->host_lock, flags);
  	switch (hostdata->action) {
  	case IBMVSCSI_HOST_ACTION_UNBLOCK:
  		rc = 0;
  		break;
  	case IBMVSCSI_HOST_ACTION_RESET:
-		spin_unlock_irqrestore(hostdata->host->host_lock, flags);
+		spin_unlock_irqrestore(&hostdata->host->host_lock, flags);
  		rc = ibmvscsi_reset_crq_queue(&hostdata->queue, hostdata);
-		spin_lock_irqsave(hostdata->host->host_lock, flags);
+		spin_lock_irqsave(&hostdata->host->host_lock, flags);
  		if (!rc)
  			rc = ibmvscsi_send_crq(hostdata, 0xC001000000000000LL, 0);
  		vio_enable_interrupts(to_vio_dev(hostdata->dev));
  		break;
  	case IBMVSCSI_HOST_ACTION_REENABLE:
  		action = "enable";
-		spin_unlock_irqrestore(hostdata->host->host_lock, flags);
+		spin_unlock_irqrestore(&hostdata->host->host_lock, flags);
  		rc = ibmvscsi_reenable_crq_queue(&hostdata->queue, hostdata);
-		spin_lock_irqsave(hostdata->host->host_lock, flags);
+		spin_lock_irqsave(&hostdata->host->host_lock, flags);
  		if (!rc)
  			rc = ibmvscsi_send_crq(hostdata, 0xC001000000000000LL, 0);
  		break;
  	case IBMVSCSI_HOST_ACTION_NONE:
  	default:
-		spin_unlock_irqrestore(hostdata->host->host_lock, flags);
+		spin_unlock_irqrestore(&hostdata->host->host_lock, flags);
  		return;
  	}

  	hostdata->action = IBMVSCSI_HOST_ACTION_NONE;
-	spin_unlock_irqrestore(hostdata->host->host_lock, flags);
+	spin_unlock_irqrestore(&hostdata->host->host_lock, flags);

  	if (rc) {
  		ibmvscsi_set_request_limit(hostdata, -1);
@@ -2191,9 +2191,9 @@ static int ibmvscsi_work_to_do(struct 
ibmvscsi_host_data *hostdata)
  	unsigned long flags;
  	int rc;

-	spin_lock_irqsave(hostdata->host->host_lock, flags);
+	spin_lock_irqsave(&hostdata->host->host_lock, flags);
  	rc = __ibmvscsi_work_to_do(hostdata);
-	spin_unlock_irqrestore(hostdata->host->host_lock, flags);
+	spin_unlock_irqrestore(&hostdata->host->host_lock, flags);

  	return rc;
  }
diff --git a/drivers/scsi/mesh.c b/drivers/scsi/mesh.c
index 80779aa000d5..8655e3e3d0ac 100644
--- a/drivers/scsi/mesh.c
+++ b/drivers/scsi/mesh.c
@@ -1708,7 +1708,7 @@ static int mesh_host_reset(struct scsi_cmnd *cmd)

  	printk(KERN_DEBUG "mesh_host_reset\n");

-	spin_lock_irqsave(ms->host->host_lock, flags);
+	spin_lock_irqsave(&ms->host->host_lock, flags);

  	if (ms->dma_started)
  		halt_dma(ms);
@@ -1734,7 +1734,7 @@ static int mesh_host_reset(struct scsi_cmnd *cmd)
  	/* Complete pending commands */
  	handle_reset(ms);
  	
-	spin_unlock_irqrestore(ms->host->host_lock, flags);
+	spin_unlock_irqrestore(&ms->host->host_lock, flags);
  	return SUCCESS;
  }

@@ -1818,7 +1818,7 @@ static int mesh_shutdown(struct macio_dev *mdev)
  	unsigned long flags;

         	printk(KERN_INFO "resetting MESH scsi bus(es)\n");
-	spin_lock_irqsave(ms->host->host_lock, flags);
+	spin_lock_irqsave(&ms->host->host_lock, flags);
         	mr = ms->mesh;
  	out_8(&mr->intr_mask, 0);
  	out_8(&mr->interrupt, INT_ERROR | INT_EXCEPTION | INT_CMDDONE);
@@ -1826,7 +1826,7 @@ static int mesh_shutdown(struct macio_dev *mdev)
  	mesh_flush_io(mr);
  	udelay(30);
  	out_8(&mr->bus_status1, 0);
-	spin_unlock_irqrestore(ms->host->host_lock, flags);
+	spin_unlock_irqrestore(&ms->host->host_lock, flags);

  	return 0;
  }
diff --git a/drivers/scsi/qlogicpti.c b/drivers/scsi/qlogicpti.c
index 84cd369a5bcf..098916061835 100644
--- a/drivers/scsi/qlogicpti.c
+++ b/drivers/scsi/qlogicpti.c
@@ -1207,7 +1207,7 @@ static irqreturn_t qpti_intr(int irq, void *dev_id)
  	unsigned long flags;
  	struct scsi_cmnd *dq;

-	spin_lock_irqsave(qpti->qhost->host_lock, flags);
+	spin_lock_irqsave(&qpti->qhost->host_lock, flags);
  	dq = qlogicpti_intr_handler(qpti);

  	if (dq != NULL) {
@@ -1219,7 +1219,7 @@ static irqreturn_t qpti_intr(int irq, void *dev_id)
  			dq = next;
  		} while (dq != NULL);
  	}
-	spin_unlock_irqrestore(qpti->qhost->host_lock, flags);
+	spin_unlock_irqrestore(&qpti->qhost->host_lock, flags);

  	return IRQ_HANDLED;
  }