Re: [PATCH 2/2] scsi: esp: make it working on SG_CHAIN

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 4 Jun 2019, Ming Lei wrote:

> The driver supporses that there isn't sg chain, and itereate the
> list one by one. This way is obviously wrong.
> 
> Fixes it by sgl helper.
> 
> Cc: Christoph Hellwig <hch@xxxxxx>
> Cc: Bart Van Assche <bvanassche@xxxxxxx>
> Cc: Ewan D. Milne <emilne@xxxxxxxxxx>
> Cc: Hannes Reinecke <hare@xxxxxxxx>
> Cc: Guenter Roeck <linux@xxxxxxxxxxxx>
> Reported-by: Guenter Roeck <linux@xxxxxxxxxxxx>
> Signed-off-by: Ming Lei <ming.lei@xxxxxxxxxx>
> ---
>  drivers/scsi/esp_scsi.c | 32 +++++++++++++++++++++++++-------
>  1 file changed, 25 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/scsi/esp_scsi.c b/drivers/scsi/esp_scsi.c
> index 76e7ca864d6a..58b4e059dcfb 100644
> --- a/drivers/scsi/esp_scsi.c
> +++ b/drivers/scsi/esp_scsi.c
> @@ -371,6 +371,7 @@ static void esp_map_dma(struct esp *esp, struct scsi_cmnd *cmd)
>  	struct esp_cmd_priv *spriv = ESP_CMD_PRIV(cmd);
>  	struct scatterlist *sg = scsi_sglist(cmd);
>  	int total = 0, i;
> +	struct scatterlist *sgt;
>  
>  	if (cmd->sc_data_direction == DMA_NONE)
>  		return;
> @@ -381,14 +382,15 @@ static void esp_map_dma(struct esp *esp, struct scsi_cmnd *cmd)
>  		 * a dma address, so perform an identity mapping.
>  		 */
>  		spriv->num_sg = scsi_sg_count(cmd);
> -		for (i = 0; i < spriv->num_sg; i++) {
> -			sg[i].dma_address = (uintptr_t)sg_virt(&sg[i]);
> -			total += sg_dma_len(&sg[i]);
> +
> +		scsi_for_each_sg(cmd, sgt, spriv->num_sg, i) {
> +			sgt->dma_address = (uintptr_t)sg_virt(sgt);
> +			total += sg_dma_len(sgt);
>  		}
>  	} else {
>  		spriv->num_sg = scsi_dma_map(cmd);
> -		for (i = 0; i < spriv->num_sg; i++)
> -			total += sg_dma_len(&sg[i]);
> +		scsi_for_each_sg(cmd, sgt, spriv->num_sg, i)
> +			total += sg_dma_len(sgt);
>  	}
>  	spriv->cur_residue = sg_dma_len(sg);
>  	spriv->cur_sg = sg;
> @@ -444,7 +446,7 @@ static void esp_advance_dma(struct esp *esp, struct esp_cmd_entry *ent,
>  		p->tot_residue = 0;
>  	}
>  	if (!p->cur_residue && p->tot_residue) {
> -		p->cur_sg++;
> +		p->cur_sg = sg_next(p->cur_sg);
>  		p->cur_residue = sg_dma_len(p->cur_sg);
>  	}
>  }
> @@ -1610,6 +1612,22 @@ static void esp_msgin_extended(struct esp *esp)
>  	scsi_esp_cmd(esp, ESP_CMD_SATN);
>  }
>  
> +static struct scatterlist *esp_sg_prev(struct scsi_cmnd *cmd,
> +		struct scatterlist *sg)
> +{
> +	int i;
> +	struct scatterlist *tmp;
> +	struct scatterlist *prev = NULL;
> +
> +	scsi_for_each_sg(cmd, tmp, scsi_sg_count(cmd), i) {
> +		if (tmp == sg)
> +			break;
> +		prev = tmp;
> +	}
> +
> +	return prev;
> +}
> +

Did you consider recording the previous scatterlist pointer using an 
additional member in struct esp_cmd_priv, to be assigned in 
esp_advance_dma()? I think it would execute faster and require less code.

-- 

>  /* Analyze msgin bytes received from target so far.  Return non-zero
>   * if there are more bytes needed to complete the message.
>   */
> @@ -1647,7 +1665,7 @@ static int esp_msgin_process(struct esp *esp)
>  		spriv = ESP_CMD_PRIV(ent->cmd);
>  
>  		if (spriv->cur_residue == sg_dma_len(spriv->cur_sg)) {
> -			spriv->cur_sg--;
> +			spriv->cur_sg = esp_sg_prev(ent->cmd, spriv->cur_sg);
>  			spriv->cur_residue = 1;
>  		} else
>  			spriv->cur_residue++;
> 



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux