Thread (106 messages) 106 messages, 9 authors, 2021-09-28

Re: [PATCH 33/84] fas216: Call scsi_done() directly

From: Bart Van Assche <bvanassche@acm.org>
Date: 2021-09-19 02:26:02
Subsystem: arm/riscpc architecture, scsi subsystem, the rest · Maintainers: Russell King, "James E.J. Bottomley", "Martin K. Petersen", Linus Torvalds

On 9/17/21 17:25, Russell King (Oracle) wrote:
NAK. I don't think you've bothered to read the driver code to check
that your change is safe to make.

SCpnt->scsi_done is not always "scsi_done" but may also be
fas216_internal_done().
Thanks for the quick feedback. How about replacing this patch by the
two patches below?

[PATCH 1/2] fas216: Introduce struct fas216_cmd_priv

Introduce a structure with driver-private data per SCSI command. This data
structure will be used by the next patch to store a function pointer.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
  drivers/scsi/arm/arxescsi.c |  1 +
  drivers/scsi/arm/cumana_2.c |  1 +
  drivers/scsi/arm/eesox.c    |  1 +
  drivers/scsi/arm/fas216.h   | 10 ++++++++++
  drivers/scsi/arm/powertec.c |  2 +-
  5 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/arm/arxescsi.c b/drivers/scsi/arm/arxescsi.c
index 591414120754..7f667c198f6d 100644
--- a/drivers/scsi/arm/arxescsi.c
+++ b/drivers/scsi/arm/arxescsi.c
@@ -243,6 +243,7 @@ static struct scsi_host_template arxescsi_template = {
  	.eh_bus_reset_handler		= fas216_eh_bus_reset,
  	.eh_device_reset_handler	= fas216_eh_device_reset,
  	.eh_abort_handler		= fas216_eh_abort,
+	.cmd_size			= sizeof(struct fas216_cmd_priv),
  	.can_queue			= 0,
  	.this_id			= 7,
  	.sg_tablesize			= SG_ALL,
diff --git a/drivers/scsi/arm/cumana_2.c b/drivers/scsi/arm/cumana_2.c
index 9dcd912267e6..3c00d7773876 100644
--- a/drivers/scsi/arm/cumana_2.c
+++ b/drivers/scsi/arm/cumana_2.c
@@ -363,6 +363,7 @@ static struct scsi_host_template cumanascsi2_template = {
  	.eh_bus_reset_handler		= fas216_eh_bus_reset,
  	.eh_device_reset_handler	= fas216_eh_device_reset,
  	.eh_abort_handler		= fas216_eh_abort,
+	.cmd_size			= sizeof(struct fas216_cmd_priv),
  	.can_queue			= 1,
  	.this_id			= 7,
  	.sg_tablesize			= SG_MAX_SEGMENTS,
diff --git a/drivers/scsi/arm/eesox.c b/drivers/scsi/arm/eesox.c
index 5eb2415dda9d..1394590eecea 100644
--- a/drivers/scsi/arm/eesox.c
+++ b/drivers/scsi/arm/eesox.c
@@ -480,6 +480,7 @@ static struct scsi_host_template eesox_template = {
  	.eh_bus_reset_handler		= fas216_eh_bus_reset,
  	.eh_device_reset_handler	= fas216_eh_device_reset,
  	.eh_abort_handler		= fas216_eh_abort,
+	.cmd_size			= sizeof(struct fas216_cmd_priv),
  	.can_queue			= 1,
  	.this_id			= 7,
  	.sg_tablesize			= SG_MAX_SEGMENTS,
diff --git a/drivers/scsi/arm/fas216.h b/drivers/scsi/arm/fas216.h
index 847413ce14cf..abf960487314 100644
--- a/drivers/scsi/arm/fas216.h
+++ b/drivers/scsi/arm/fas216.h
@@ -310,6 +310,16 @@ typedef struct {
  	unsigned long		magic_end;
  } FAS216_Info;

+/* driver-private data per SCSI command. */
+struct fas216_cmd_priv {
+	void (*scsi_done)(struct scsi_cmnd *cmd);
+};
+
+static inline struct fas216_cmd_priv *fas216_cmd_priv(struct scsi_cmnd *cmd)
+{
+	return scsi_cmd_priv(cmd);
+}
+
  /* Function: int fas216_init (struct Scsi_Host *instance)
   * Purpose : initialise FAS/NCR/AMD SCSI structures.
   * Params  : instance - a driver-specific filled-out structure
diff --git a/drivers/scsi/arm/powertec.c b/drivers/scsi/arm/powertec.c
index 9cc73da4e876..8fec435cee18 100644
--- a/drivers/scsi/arm/powertec.c
+++ b/drivers/scsi/arm/powertec.c
@@ -286,7 +286,7 @@ static struct scsi_host_template powertecscsi_template = {
  	.eh_bus_reset_handler		= fas216_eh_bus_reset,
  	.eh_device_reset_handler	= fas216_eh_device_reset,
  	.eh_abort_handler		= fas216_eh_abort,
-
+	.cmd_size			= sizeof(struct fas216_cmd_priv),
  	.can_queue			= 8,
  	.this_id			= 7,
  	.sg_tablesize			= SG_MAX_SEGMENTS,





[PATCH 2/2] fas216: Stop using scsi_cmnd.scsi_done

Store the completion callback pointer in struct fas216_cmd_priv instead of in
struct scsi_cmnd. This patch prepares for removal of the scsi_done member
from struct scsi_cmnd.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
  drivers/scsi/arm/fas216.c | 8 ++++----
  1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/arm/fas216.c b/drivers/scsi/arm/fas216.c
index bbb8635782b1..9926383f79ea 100644
--- a/drivers/scsi/arm/fas216.c
+++ b/drivers/scsi/arm/fas216.c
@@ -2017,7 +2017,7 @@ static void fas216_rq_sns_done(FAS216_Info *info, struct scsi_cmnd *SCpnt,
  	 * correctly by fas216_std_done.
  	 */
  	scsi_eh_restore_cmnd(SCpnt, &info->ses);
-	SCpnt->scsi_done(SCpnt);
+	fas216_cmd_priv(SCpnt)->scsi_done(SCpnt);
  }

  /**
@@ -2088,8 +2088,8 @@ fas216_std_done(FAS216_Info *info, struct scsi_cmnd *SCpnt, unsigned int result)
  	}

  done:
-	if (SCpnt->scsi_done) {
-		SCpnt->scsi_done(SCpnt);
+	if (fas216_cmd_priv(SCpnt)->scsi_done) {
+		fas216_cmd_priv(SCpnt)->scsi_done(SCpnt);
  		return;
  	}
@@ -2205,7 +2205,7 @@ static int fas216_queue_command_lck(struct scsi_cmnd *SCpnt,
  	fas216_log_command(info, LOG_CONNECT, SCpnt,
  			   "received command (%p)", SCpnt);

-	SCpnt->scsi_done = done;
+	fas216_cmd_priv(SCpnt)->scsi_done = done;
  	SCpnt->host_scribble = (void *)fas216_std_done;
  	SCpnt->result = 0;
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help