[PATCH 2/3] wd719x: use per-command private data
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
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