[PATCH 2/3] wd719x: use per-command private data

2018-11-09 Thread Christoph Hellwig
Add the SCB onto the scsi command allocation and use dma streaming
mappings for it only when in use.  This avoid possibly calling
dma_alloc_coherent under a lock or even in irq context, while also
making the code simpler.

Thanks to Ondrej Zary for testing and various bug fixes.

Signed-off-by: Christoph Hellwig 
---
 drivers/scsi/wd719x.c | 98 +++
 drivers/scsi/wd719x.h |  1 -
 2 files changed, 42 insertions(+), 57 deletions(-)

diff --git a/drivers/scsi/wd719x.c b/drivers/scsi/wd719x.c
index 7b05bbcfb186..b73e7f24a1c4 100644
--- a/drivers/scsi/wd719x.c
+++ b/drivers/scsi/wd719x.c
@@ -153,8 +153,6 @@ static int wd719x_direct_cmd(struct wd719x *wd, u8 opcode, 
u8 dev, u8 lun,
 
 static void wd719x_destroy(struct wd719x *wd)
 {
-   struct wd719x_scb *scb;
-
/* stop the RISC */
if (wd719x_direct_cmd(wd, WD719X_CMD_SLEEP, 0, 0, 0, 0,
  WD719X_WAIT_FOR_RISC))
@@ -164,10 +162,6 @@ static void wd719x_destroy(struct wd719x *wd)
 
WARN_ON_ONCE(!list_empty(>active_scbs));
 
-   /* free all SCBs */
-   list_for_each_entry(scb, >free_scbs, list)
-   pci_free_consistent(wd->pdev, sizeof(struct wd719x_scb), scb,
-   scb->phys);
/* free internal buffers */
pci_free_consistent(wd->pdev, wd->fw_size, wd->fw_virt, wd->fw_phys);
wd->fw_virt = NULL;
@@ -180,18 +174,20 @@ static void wd719x_destroy(struct wd719x *wd)
free_irq(wd->pdev->irq, wd);
 }
 
-/* finish a SCSI command, mark SCB (if any) as free, unmap buffers */
-static void wd719x_finish_cmd(struct scsi_cmnd *cmd, int result)
+/* finish a SCSI command, unmap buffers */
+static void wd719x_finish_cmd(struct wd719x_scb *scb, int result)
 {
+   struct scsi_cmnd *cmd = scb->cmd;
struct wd719x *wd = shost_priv(cmd->device->host);
-   struct wd719x_scb *scb = (struct wd719x_scb *) cmd->host_scribble;
 
-   if (scb) {
-   list_move(>list, >free_scbs);
-   dma_unmap_single(>pdev->dev, cmd->SCp.dma_handle,
-SCSI_SENSE_BUFFERSIZE, DMA_FROM_DEVICE);
-   scsi_dma_unmap(cmd);
-   }
+   list_del(>list);
+
+   dma_unmap_single(>pdev->dev, scb->phys,
+   sizeof(struct wd719x_scb), DMA_BIDIRECTIONAL);
+   scsi_dma_unmap(cmd);
+   dma_unmap_single(>pdev->dev, cmd->SCp.dma_handle,
+SCSI_SENSE_BUFFERSIZE, DMA_FROM_DEVICE);
+
cmd->result = result << 16;
cmd->scsi_done(cmd);
 }
@@ -201,36 +197,10 @@ static int wd719x_queuecommand(struct Scsi_Host *sh, 
struct scsi_cmnd *cmd)
 {
int i, count_sg;
unsigned long flags;
-   struct wd719x_scb *scb;
+   struct wd719x_scb *scb = scsi_cmd_priv(cmd);
struct wd719x *wd = shost_priv(sh);
-   dma_addr_t phys;
-
-   cmd->host_scribble = NULL;
-
-   /* get a free SCB - either from existing ones or allocate a new one */
-   spin_lock_irqsave(wd->sh->host_lock, flags);
-   scb = list_first_entry_or_null(>free_scbs, struct wd719x_scb, list);
-   if (scb) {
-   list_del(>list);
-   phys = scb->phys;
-   } else {
-   spin_unlock_irqrestore(wd->sh->host_lock, flags);
-   scb = pci_alloc_consistent(wd->pdev, sizeof(struct wd719x_scb),
-  );
-   spin_lock_irqsave(wd->sh->host_lock, flags);
-   if (!scb) {
-   dev_err(>pdev->dev, "unable to allocate SCB\n");
-   wd719x_finish_cmd(cmd, DID_ERROR);
-   spin_unlock_irqrestore(wd->sh->host_lock, flags);
-   return 0;
-   }
-   }
-   memset(scb, 0, sizeof(struct wd719x_scb));
-   list_add(>list, >active_scbs);
 
-   scb->phys = phys;
scb->cmd = cmd;
-   cmd->host_scribble = (char *) scb;
 
scb->CDB_tag = 0;   /* Tagged queueing not supported yet */
scb->devid = cmd->device->id;
@@ -239,10 +209,19 @@ static int wd719x_queuecommand(struct Scsi_Host *sh, 
struct scsi_cmnd *cmd)
/* copy the command */
memcpy(scb->CDB, cmd->cmnd, cmd->cmd_len);
 
+   /* map SCB */
+   scb->phys = dma_map_single(>pdev->dev, scb, sizeof(*scb),
+  DMA_BIDIRECTIONAL);
+
+   if (dma_mapping_error(>pdev->dev, scb->phys))
+   goto out_error;
+
/* map sense buffer */
scb->sense_buf_length = SCSI_SENSE_BUFFERSIZE;
cmd->SCp.dma_handle = dma_map_single(>pdev->dev, cmd->sense_buffer,
SCSI_SENSE_BUFFERSIZE, DMA_FROM_DEVICE);
+   if (dma_mapping_error(>pdev->dev, cmd->SCp.dma_handle))
+   goto out_unmap_scb;
scb->sense_buf = cpu_to_le32(cmd->SCp.dma_handle);
 
/* request autosense */
@@ -257,11 +236,8 @@ static int wd719x_queuecommand(struct Scsi_Host 

[PATCH 2/3] wd719x: use per-command private data

2018-10-18 Thread Christoph Hellwig
Add the SCB onto the scsi command allocation and use dma streaming
mappings for it only when in use.  This avoid possibly calling
dma_alloc_coherent under a lock or even in irq context, while also
making the code simpler.

Signed-off-by: Christoph Hellwig 
---
 drivers/scsi/wd719x.c | 95 ++-
 drivers/scsi/wd719x.h |  1 -
 2 files changed, 39 insertions(+), 57 deletions(-)

diff --git a/drivers/scsi/wd719x.c b/drivers/scsi/wd719x.c
index 7b05bbcfb186..d47190f08ed6 100644
--- a/drivers/scsi/wd719x.c
+++ b/drivers/scsi/wd719x.c
@@ -153,8 +153,6 @@ static int wd719x_direct_cmd(struct wd719x *wd, u8 opcode, 
u8 dev, u8 lun,
 
 static void wd719x_destroy(struct wd719x *wd)
 {
-   struct wd719x_scb *scb;
-
/* stop the RISC */
if (wd719x_direct_cmd(wd, WD719X_CMD_SLEEP, 0, 0, 0, 0,
  WD719X_WAIT_FOR_RISC))
@@ -164,10 +162,6 @@ static void wd719x_destroy(struct wd719x *wd)
 
WARN_ON_ONCE(!list_empty(>active_scbs));
 
-   /* free all SCBs */
-   list_for_each_entry(scb, >free_scbs, list)
-   pci_free_consistent(wd->pdev, sizeof(struct wd719x_scb), scb,
-   scb->phys);
/* free internal buffers */
pci_free_consistent(wd->pdev, wd->fw_size, wd->fw_virt, wd->fw_phys);
wd->fw_virt = NULL;
@@ -180,18 +174,20 @@ static void wd719x_destroy(struct wd719x *wd)
free_irq(wd->pdev->irq, wd);
 }
 
-/* finish a SCSI command, mark SCB (if any) as free, unmap buffers */
-static void wd719x_finish_cmd(struct scsi_cmnd *cmd, int result)
+/* finish a SCSI command, unmap buffers */
+static void wd719x_finish_cmd(struct wd719x_scb *scb, int result)
 {
+   struct scsi_cmnd *cmd = scb->cmd;
struct wd719x *wd = shost_priv(cmd->device->host);
-   struct wd719x_scb *scb = (struct wd719x_scb *) cmd->host_scribble;
 
-   if (scb) {
-   list_move(>list, >free_scbs);
-   dma_unmap_single(>pdev->dev, cmd->SCp.dma_handle,
-SCSI_SENSE_BUFFERSIZE, DMA_FROM_DEVICE);
-   scsi_dma_unmap(cmd);
-   }
+   list_del(>list);
+
+   dma_unmap_single(>pdev->dev, scb->phys,
+   sizeof(struct wd719x_scb), DMA_TO_DEVICE);
+   scsi_dma_unmap(cmd);
+   dma_unmap_single(>pdev->dev, cmd->SCp.dma_handle,
+SCSI_SENSE_BUFFERSIZE, DMA_FROM_DEVICE);
+
cmd->result = result << 16;
cmd->scsi_done(cmd);
 }
@@ -201,36 +197,10 @@ static int wd719x_queuecommand(struct Scsi_Host *sh, 
struct scsi_cmnd *cmd)
 {
int i, count_sg;
unsigned long flags;
-   struct wd719x_scb *scb;
+   struct wd719x_scb *scb = scsi_cmd_priv(cmd);
struct wd719x *wd = shost_priv(sh);
-   dma_addr_t phys;
-
-   cmd->host_scribble = NULL;
 
-   /* get a free SCB - either from existing ones or allocate a new one */
-   spin_lock_irqsave(wd->sh->host_lock, flags);
-   scb = list_first_entry_or_null(>free_scbs, struct wd719x_scb, list);
-   if (scb) {
-   list_del(>list);
-   phys = scb->phys;
-   } else {
-   spin_unlock_irqrestore(wd->sh->host_lock, flags);
-   scb = pci_alloc_consistent(wd->pdev, sizeof(struct wd719x_scb),
-  );
-   spin_lock_irqsave(wd->sh->host_lock, flags);
-   if (!scb) {
-   dev_err(>pdev->dev, "unable to allocate SCB\n");
-   wd719x_finish_cmd(cmd, DID_ERROR);
-   spin_unlock_irqrestore(wd->sh->host_lock, flags);
-   return 0;
-   }
-   }
-   memset(scb, 0, sizeof(struct wd719x_scb));
-   list_add(>list, >active_scbs);
-
-   scb->phys = phys;
scb->cmd = cmd;
-   cmd->host_scribble = (char *) scb;
 
scb->CDB_tag = 0;   /* Tagged queueing not supported yet */
scb->devid = cmd->device->id;
@@ -243,6 +213,8 @@ static int wd719x_queuecommand(struct Scsi_Host *sh, struct 
scsi_cmnd *cmd)
scb->sense_buf_length = SCSI_SENSE_BUFFERSIZE;
cmd->SCp.dma_handle = dma_map_single(>pdev->dev, cmd->sense_buffer,
SCSI_SENSE_BUFFERSIZE, DMA_FROM_DEVICE);
+   if (dma_mapping_error(>pdev->dev, cmd->SCp.dma_handle))
+   goto out_error;
scb->sense_buf = cpu_to_le32(cmd->SCp.dma_handle);
 
/* request autosense */
@@ -257,11 +229,8 @@ static int wd719x_queuecommand(struct Scsi_Host *sh, 
struct scsi_cmnd *cmd)
 
/* Scather/gather */
count_sg = scsi_dma_map(cmd);
-   if (count_sg < 0) {
-   wd719x_finish_cmd(cmd, DID_ERROR);
-   spin_unlock_irqrestore(wd->sh->host_lock, flags);
-   return 0;
-   }
+   if (count_sg < 0)
+   goto out_unmap_sense;
BUG_ON(count_sg > WD719X_SG);
 
if (count_sg) {
@@ -277,24