Linux-USB Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH V4 00/16] use sg helper to operate scatterlist
@ 2019-06-17  3:03 Ming Lei
  2019-06-17  3:03 ` [PATCH V4 01/16] scsi: vmw_pscsi: " Ming Lei
                   ` (16 more replies)
  0 siblings, 17 replies; 40+ messages in thread
From: Ming Lei @ 2019-06-17  3:03 UTC (permalink / raw)
  To: linux-scsi, Martin K . Petersen
  Cc: James Bottomley, Bart Van Assche, Hannes Reinecke,
	Christoph Hellwig, Jim Gill, Cathy Avery, Ewan D . Milne,
	Brian King, James Smart, Juergen E . Fischer, Michael Schmitz,
	Finn Thain, Greg Kroah-Hartman, devel, linux-usb, Dan Carpenter,
	Benjamin Block, Ming Lei

Hi,

Scsi MQ makes a large static allocation for the first scatter gather
list chunk for the driver to use.  This is a performance headache we'd
like to fix by reducing the size of the allocation to a 2 element
array.  Doing this will break the current guarantee that any driver
using SG_ALL doesn't need to use the scatterlist iterators and can get
away with directly dereferencing the array.  Thus we need to update all
drivers to use the scatterlist iterators and remove direct indexing of
the scatterlist array before reducing the initial scatterlist
allocation size in SCSI.

So convert drivers to use scatterlist helper.

There are two types of direct access on scatterlist in SCSI drivers:

1) operate on the scatterlist via scsi_sglist(scmd) directly, then one
local variable of 'struct scatterlist *' is involved.

2) scsi_sglist(scmd) is stored to cmd->SCp.buffer and the scatterlist is
used via cmd->SCp.buffer.

The following coccinelle semantic patch is developed for finding the
above two types of direct scatterlist uses:

	@@ struct scatterlist *p; @@
	(
	- ++p
	+ p = sg_next(p)
	|
	- p++
	+ p = sg_next(p)
	|
	- p = p + 1
	+ p = sg_next(p)
	|
	- p += 1
	+ p = sg_next(p)
	|
	- --p
	+ p = sg_non_exist_prev(p)
	|
	- p--
	+ p = sg_non_exist_prev(p)
	|
	- p = p - 1
	+ p = sg_non_exist_prev(p)
	|
	- p -= 1
	+ p = sg_non_exist_prev(p)
	)
	
	@@
	struct scatterlist *p;
	expression data != 0;
	@@
	- p[data]
	+ '!!!!!!use sg iterator helper!!!!!!'
	
	@@
	struct scatterlist[] p;
	expression data != 0;
	@@
	- p[data]
	+ '!!!!!!use sg iterator helper!!!!!!'
	
	
	@@ struct scsi_cmnd *scmd; @@
	(
	-	scmd->SCp.buffer++
	+	scmd->SCp.buffer = sg_next(scmd->SCp.buffer)
	|
	-	++scmd->SCp.buffer
	+	scmd->SCp.buffer = sg_next(scmd->SCp.buffer)
	|
	-	scmd->SCp.buffer += 1
	+	scmd->SCp.buffer = sg_next(scmd->SCp.buffer)
	|
	-	scmd->SCp.buffer = scmd->SCp.buffer + 1
	+	scmd->SCp.buffer = sg_next(scmd->SCp.buffer)
	|
	-	scmd->SCp.buffer--
	+	scmd->SCp.buffer = sg_no_exit_prev(scmd->SCp.buffer)
	|
	-	--scmd->SCp.buffer
	+	scmd->SCp.buffer = sg_no_exit_prev(scmd->SCp.buffer)
	|
	-	scmd->SCp.buffer -= 1
	+	scmd->SCp.buffer = sg_no_exit_prev(scmd->SCp.buffer)
	|
	-	scmd->SCp.buffer = scmd->SCp.buffer - 1
	+	scmd->SCp.buffer = sg_no_exit_prev(scmd->SCp.buffer)
	)
	
	@@
	struct scsi_cmnd *scmd;
	expression data != 0;
	@@
	- scmd->SCp.buffer[data]
	+ '!!!!!!use sg iterator helper!!!!!!'


The 1st 10 patches are for handling type #1, and the other 6 patches
for handling type #2, and all the 16 are found by the above coccinelle
semantic patch.

V4:
	- fix building failure on pmcraid's conversion 
	- improve the coccinelle semantic patch to cover both two types of
	scatterlist direct use
	- driver 'staging: rtsx' is covered

V3:
	- update commit log and cover letter, most of words are from
	James Bottomley	

V2:
	- use coccinelle semantic patch for finding direct sgl uses from
	scsi command(9 drivers found)
	- run 'git grep -E "SCp.buffer"' to find direct sgl uses
	from SCp.buffer(6 drivers are found)


Finn Thain (1):
  NCR5380: Support chained sg lists

Ming Lei (15):
  scsi: vmw_pscsi: use sg helper to operate scatterlist
  scsi: advansys: use sg helper to operate scatterlist
  scsi: lpfc: use sg helper to operate scatterlist
  scsi: mvumi: use sg helper to operate scatterlist
  scsi: ipr: use sg helper to operate scatterlist
  scsi: pmcraid: use sg helper to operate scatterlist
  usb: image: microtek: use sg helper to operate scatterlist
  staging: unisys: visorhba: use sg helper to operate scatterlist
  staging: rtsx: use sg helper to operate scatterlist
  s390: zfcp_fc: use sg helper to operate scatterlist
  scsi: aha152x: use sg helper to operate scatterlist
  scsi: imm: use sg helper to operate scatterlist
  scsi: pcmcia: nsp_cs: use sg helper to operate scatterlist
  scsi: ppa: use sg helper to operate scatterlist
  scsi: wd33c93: use sg helper to operate scatterlist

 drivers/s390/scsi/zfcp_fc.c                   |  4 +-
 drivers/scsi/NCR5380.c                        | 41 ++++++++----------
 drivers/scsi/advansys.c                       |  2 +-
 drivers/scsi/aha152x.c                        | 42 +++++++++----------
 drivers/scsi/imm.c                            |  2 +-
 drivers/scsi/ipr.c                            | 28 +++++++------
 drivers/scsi/lpfc/lpfc_nvmet.c                |  3 +-
 drivers/scsi/mvumi.c                          |  9 ++--
 drivers/scsi/pcmcia/nsp_cs.c                  |  4 +-
 drivers/scsi/pmcraid.c                        | 14 +++----
 drivers/scsi/ppa.c                            |  2 +-
 drivers/scsi/vmw_pvscsi.c                     |  2 +-
 drivers/scsi/wd33c93.c                        |  2 +-
 drivers/staging/rts5208/rtsx_transport.c      |  4 +-
 .../staging/unisys/visorhba/visorhba_main.c   |  9 ++--
 drivers/usb/image/microtek.c                  | 20 ++++-----
 drivers/usb/image/microtek.h                  |  2 +-
 17 files changed, 90 insertions(+), 100 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH V4 01/16] scsi: vmw_pscsi: use sg helper to operate scatterlist
  2019-06-17  3:03 [PATCH V4 00/16] use sg helper to operate scatterlist Ming Lei
@ 2019-06-17  3:03 ` " Ming Lei
  2019-06-17  8:22   ` Christoph Hellwig
  2019-06-17  3:03 ` [PATCH V4 02/16] scsi: advansys: " Ming Lei
                   ` (15 subsequent siblings)
  16 siblings, 1 reply; 40+ messages in thread
From: Ming Lei @ 2019-06-17  3:03 UTC (permalink / raw)
  To: linux-scsi, Martin K . Petersen
  Cc: James Bottomley, Bart Van Assche, Hannes Reinecke,
	Christoph Hellwig, Jim Gill, Cathy Avery, Ewan D . Milne,
	Brian King, James Smart, Juergen E . Fischer, Michael Schmitz,
	Finn Thain, Greg Kroah-Hartman, devel, linux-usb, Dan Carpenter,
	Benjamin Block, Ming Lei

Use the scatterlist iterators and remove direct indexing of the scatterlist
array.

This way allows us to pre-allocate one small scatterlist, which can be chained
with one runtime allocated scatterlist if the pre-allocated one isn't enough
for the whole request.

Reviewed-by: Ewan D. Milne <emilne@redhat.com>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 drivers/scsi/vmw_pvscsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/vmw_pvscsi.c b/drivers/scsi/vmw_pvscsi.c
index ecee4b3ff073..d71abd416eb4 100644
--- a/drivers/scsi/vmw_pvscsi.c
+++ b/drivers/scsi/vmw_pvscsi.c
@@ -335,7 +335,7 @@ static void pvscsi_create_sg(struct pvscsi_ctx *ctx,
 	BUG_ON(count > PVSCSI_MAX_NUM_SG_ENTRIES_PER_SEGMENT);
 
 	sge = &ctx->sgl->sge[0];
-	for (i = 0; i < count; i++, sg++) {
+	for (i = 0; i < count; i++, sg = sg_next(sg)) {
 		sge[i].addr   = sg_dma_address(sg);
 		sge[i].length = sg_dma_len(sg);
 		sge[i].flags  = 0;
-- 
2.20.1


^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH V4 02/16] scsi: advansys: use sg helper to operate scatterlist
  2019-06-17  3:03 [PATCH V4 00/16] use sg helper to operate scatterlist Ming Lei
  2019-06-17  3:03 ` [PATCH V4 01/16] scsi: vmw_pscsi: " Ming Lei
@ 2019-06-17  3:03 ` " Ming Lei
  2019-06-17  8:22   ` Christoph Hellwig
  2019-06-17  3:03 ` [PATCH V4 03/16] scsi: lpfc: " Ming Lei
                   ` (14 subsequent siblings)
  16 siblings, 1 reply; 40+ messages in thread
From: Ming Lei @ 2019-06-17  3:03 UTC (permalink / raw)
  To: linux-scsi, Martin K . Petersen
  Cc: James Bottomley, Bart Van Assche, Hannes Reinecke,
	Christoph Hellwig, Jim Gill, Cathy Avery, Ewan D . Milne,
	Brian King, James Smart, Juergen E . Fischer, Michael Schmitz,
	Finn Thain, Greg Kroah-Hartman, devel, linux-usb, Dan Carpenter,
	Benjamin Block, Ming Lei

Use the scatterlist iterators and remove direct indexing of the
scatterlist array.

This way allows us to pre-allocate one small scatterlist, which can be
chained with one runtime allocated scatterlist if the pre-allocated one
isn't enough for the whole request.

Reviewed-by: Ewan D. Milne <emilne@redhat.com>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 drivers/scsi/advansys.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/advansys.c b/drivers/scsi/advansys.c
index d37584403c33..b87de8d3d844 100644
--- a/drivers/scsi/advansys.c
+++ b/drivers/scsi/advansys.c
@@ -7714,7 +7714,7 @@ adv_get_sglist(struct asc_board *boardp, adv_req_t *reqp,
 				sg_block->sg_ptr = 0L; /* Last ADV_SG_BLOCK in list. */
 				return ADV_SUCCESS;
 			}
-			slp++;
+			slp = sg_next(slp);
 		}
 		sg_block->sg_cnt = NO_OF_SG_PER_BLOCK;
 		prev_sg_block = sg_block;
-- 
2.20.1


^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH V4 03/16] scsi: lpfc: use sg helper to operate scatterlist
  2019-06-17  3:03 [PATCH V4 00/16] use sg helper to operate scatterlist Ming Lei
  2019-06-17  3:03 ` [PATCH V4 01/16] scsi: vmw_pscsi: " Ming Lei
  2019-06-17  3:03 ` [PATCH V4 02/16] scsi: advansys: " Ming Lei
@ 2019-06-17  3:03 ` " Ming Lei
  2019-06-17  8:22   ` Christoph Hellwig
  2019-06-17  3:03 ` [PATCH V4 04/16] scsi: mvumi: " Ming Lei
                   ` (13 subsequent siblings)
  16 siblings, 1 reply; 40+ messages in thread
From: Ming Lei @ 2019-06-17  3:03 UTC (permalink / raw)
  To: linux-scsi, Martin K . Petersen
  Cc: James Bottomley, Bart Van Assche, Hannes Reinecke,
	Christoph Hellwig, Jim Gill, Cathy Avery, Ewan D . Milne,
	Brian King, James Smart, Juergen E . Fischer, Michael Schmitz,
	Finn Thain, Greg Kroah-Hartman, devel, linux-usb, Dan Carpenter,
	Benjamin Block, Ming Lei

Use the scatterlist iterators and remove direct indexing of the
scatterlist array.

This way allows us to pre-allocate one small scatterlist, which can be
chained with one runtime allocated scatterlist if the pre-allocated one isn't
enough for the whole request.

Reviewed by: Ewan D. Milne <emilne@redhat.com>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 drivers/scsi/lpfc/lpfc_nvmet.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_nvmet.c b/drivers/scsi/lpfc/lpfc_nvmet.c
index f3d9a5545164..3f803982bd1e 100644
--- a/drivers/scsi/lpfc/lpfc_nvmet.c
+++ b/drivers/scsi/lpfc/lpfc_nvmet.c
@@ -2887,8 +2887,7 @@ lpfc_nvmet_prep_fcp_wqe(struct lpfc_hba *phba,
 	nvmewqe->drvrTimeout = (phba->fc_ratov * 3) + LPFC_DRVR_TIMEOUT;
 	nvmewqe->context1 = ndlp;
 
-	for (i = 0; i < rsp->sg_cnt; i++) {
-		sgel = &rsp->sg[i];
+	for_each_sg(rsp->sg, sgel, rsp->sg_cnt, i) {
 		physaddr = sg_dma_address(sgel);
 		cnt = sg_dma_len(sgel);
 		sgl->addr_hi = putPaddrHigh(physaddr);
-- 
2.20.1


^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH V4 04/16] scsi: mvumi: use sg helper to operate scatterlist
  2019-06-17  3:03 [PATCH V4 00/16] use sg helper to operate scatterlist Ming Lei
                   ` (2 preceding siblings ...)
  2019-06-17  3:03 ` [PATCH V4 03/16] scsi: lpfc: " Ming Lei
@ 2019-06-17  3:03 ` " Ming Lei
  2019-06-17  8:22   ` Christoph Hellwig
  2019-06-17  3:03 ` [PATCH V4 05/16] scsi: ipr: " Ming Lei
                   ` (12 subsequent siblings)
  16 siblings, 1 reply; 40+ messages in thread
From: Ming Lei @ 2019-06-17  3:03 UTC (permalink / raw)
  To: linux-scsi, Martin K . Petersen
  Cc: James Bottomley, Bart Van Assche, Hannes Reinecke,
	Christoph Hellwig, Jim Gill, Cathy Avery, Ewan D . Milne,
	Brian King, James Smart, Juergen E . Fischer, Michael Schmitz,
	Finn Thain, Greg Kroah-Hartman, devel, linux-usb, Dan Carpenter,
	Benjamin Block, Ming Lei

Use the scatterlist iterators and remove direct indexing of the
scatterlist array.

This way allows us to pre-allocate one small scatterlist, which can be
chained with one runtime allocated scatterlist if the pre-allocated one
isn't enough for the whole request.

Reviewed-by: Ewan D. Milne <emilne@redhat.com>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 drivers/scsi/mvumi.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/mvumi.c b/drivers/scsi/mvumi.c
index a5410615edac..0022cd31500a 100644
--- a/drivers/scsi/mvumi.c
+++ b/drivers/scsi/mvumi.c
@@ -211,8 +211,7 @@ static int mvumi_make_sgl(struct mvumi_hba *mhba, struct scsi_cmnd *scmd,
 	unsigned int sgnum = scsi_sg_count(scmd);
 	dma_addr_t busaddr;
 
-	sg = scsi_sglist(scmd);
-	*sg_count = dma_map_sg(&mhba->pdev->dev, sg, sgnum,
+	*sg_count = dma_map_sg(&mhba->pdev->dev, scsi_sglist(scmd), sgnum,
 			       scmd->sc_data_direction);
 	if (*sg_count > mhba->max_sge) {
 		dev_err(&mhba->pdev->dev,
@@ -222,12 +221,12 @@ static int mvumi_make_sgl(struct mvumi_hba *mhba, struct scsi_cmnd *scmd,
 			     scmd->sc_data_direction);
 		return -1;
 	}
-	for (i = 0; i < *sg_count; i++) {
-		busaddr = sg_dma_address(&sg[i]);
+	scsi_for_each_sg(scmd, sg, *sg_count, i) {
+		busaddr = sg_dma_address(sg);
 		m_sg->baseaddr_l = cpu_to_le32(lower_32_bits(busaddr));
 		m_sg->baseaddr_h = cpu_to_le32(upper_32_bits(busaddr));
 		m_sg->flags = 0;
-		sgd_setsz(mhba, m_sg, cpu_to_le32(sg_dma_len(&sg[i])));
+		sgd_setsz(mhba, m_sg, cpu_to_le32(sg_dma_len(sg)));
 		if ((i + 1) == *sg_count)
 			m_sg->flags |= 1U << mhba->eot_flag;
 
-- 
2.20.1


^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH V4 05/16] scsi: ipr: use sg helper to operate scatterlist
  2019-06-17  3:03 [PATCH V4 00/16] use sg helper to operate scatterlist Ming Lei
                   ` (3 preceding siblings ...)
  2019-06-17  3:03 ` [PATCH V4 04/16] scsi: mvumi: " Ming Lei
@ 2019-06-17  3:03 ` " Ming Lei
  2019-06-17  8:24   ` Christoph Hellwig
  2019-06-17  3:03 ` [PATCH V4 06/16] scsi: pmcraid: " Ming Lei
                   ` (11 subsequent siblings)
  16 siblings, 1 reply; 40+ messages in thread
From: Ming Lei @ 2019-06-17  3:03 UTC (permalink / raw)
  To: linux-scsi, Martin K . Petersen
  Cc: James Bottomley, Bart Van Assche, Hannes Reinecke,
	Christoph Hellwig, Jim Gill, Cathy Avery, Ewan D . Milne,
	Brian King, James Smart, Juergen E . Fischer, Michael Schmitz,
	Finn Thain, Greg Kroah-Hartman, devel, linux-usb, Dan Carpenter,
	Benjamin Block, Ming Lei

Use the scatterlist iterators and remove direct indexing of the
scatterlist array.

This way allows us to pre-allocate one small scatterlist, which can be
chained with one runtime allocated scatterlist if the pre-allocated one
isn't enough for the whole request.

Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 drivers/scsi/ipr.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c
index 6d053e220153..383603973937 100644
--- a/drivers/scsi/ipr.c
+++ b/drivers/scsi/ipr.c
@@ -3915,22 +3915,22 @@ static int ipr_copy_ucode_buffer(struct ipr_sglist *sglist,
 				 u8 *buffer, u32 len)
 {
 	int bsize_elem, i, result = 0;
-	struct scatterlist *scatterlist;
+	struct scatterlist *sg;
 	void *kaddr;
 
 	/* Determine the actual number of bytes per element */
 	bsize_elem = PAGE_SIZE * (1 << sglist->order);
 
-	scatterlist = sglist->scatterlist;
+	sg = sglist->scatterlist;
 
-	for (i = 0; i < (len / bsize_elem); i++, buffer += bsize_elem) {
-		struct page *page = sg_page(&scatterlist[i]);
+	for (i = 0; i < (len / bsize_elem); i++, sg = sg_next(sg), buffer += bsize_elem) {
+		struct page *page = sg_page(sg);
 
 		kaddr = kmap(page);
 		memcpy(kaddr, buffer, bsize_elem);
 		kunmap(page);
 
-		scatterlist[i].length = bsize_elem;
+		sg->length = bsize_elem;
 
 		if (result != 0) {
 			ipr_trace;
@@ -3939,13 +3939,13 @@ static int ipr_copy_ucode_buffer(struct ipr_sglist *sglist,
 	}
 
 	if (len % bsize_elem) {
-		struct page *page = sg_page(&scatterlist[i]);
+		struct page *page = sg_page(sg);
 
 		kaddr = kmap(page);
 		memcpy(kaddr, buffer, len % bsize_elem);
 		kunmap(page);
 
-		scatterlist[i].length = len % bsize_elem;
+		sg->length = len % bsize_elem;
 	}
 
 	sglist->buffer_len = len;
@@ -3966,6 +3966,7 @@ static void ipr_build_ucode_ioadl64(struct ipr_cmnd *ipr_cmd,
 	struct ipr_ioarcb *ioarcb = &ipr_cmd->ioarcb;
 	struct ipr_ioadl64_desc *ioadl64 = ipr_cmd->i.ioadl64;
 	struct scatterlist *scatterlist = sglist->scatterlist;
+	struct scatterlist *sg;
 	int i;
 
 	ipr_cmd->dma_use_sg = sglist->num_dma_sg;
@@ -3974,10 +3975,10 @@ static void ipr_build_ucode_ioadl64(struct ipr_cmnd *ipr_cmd,
 
 	ioarcb->ioadl_len =
 		cpu_to_be32(sizeof(struct ipr_ioadl64_desc) * ipr_cmd->dma_use_sg);
-	for (i = 0; i < ipr_cmd->dma_use_sg; i++) {
+	for_each_sg(scatterlist, sg, ipr_cmd->dma_use_sg, i) {
 		ioadl64[i].flags = cpu_to_be32(IPR_IOADL_FLAGS_WRITE);
-		ioadl64[i].data_len = cpu_to_be32(sg_dma_len(&scatterlist[i]));
-		ioadl64[i].address = cpu_to_be64(sg_dma_address(&scatterlist[i]));
+		ioadl64[i].data_len = cpu_to_be32(sg_dma_len(sg));
+		ioadl64[i].address = cpu_to_be64(sg_dma_address(sg));
 	}
 
 	ioadl64[i-1].flags |= cpu_to_be32(IPR_IOADL_FLAGS_LAST);
@@ -3997,6 +3998,7 @@ static void ipr_build_ucode_ioadl(struct ipr_cmnd *ipr_cmd,
 	struct ipr_ioarcb *ioarcb = &ipr_cmd->ioarcb;
 	struct ipr_ioadl_desc *ioadl = ipr_cmd->i.ioadl;
 	struct scatterlist *scatterlist = sglist->scatterlist;
+	struct scatterlist *sg;
 	int i;
 
 	ipr_cmd->dma_use_sg = sglist->num_dma_sg;
@@ -4006,11 +4008,11 @@ static void ipr_build_ucode_ioadl(struct ipr_cmnd *ipr_cmd,
 	ioarcb->ioadl_len =
 		cpu_to_be32(sizeof(struct ipr_ioadl_desc) * ipr_cmd->dma_use_sg);
 
-	for (i = 0; i < ipr_cmd->dma_use_sg; i++) {
+	for_each_sg(scatterlist, sg, ipr_cmd->dma_use_sg, i) {
 		ioadl[i].flags_and_data_len =
-			cpu_to_be32(IPR_IOADL_FLAGS_WRITE | sg_dma_len(&scatterlist[i]));
+			cpu_to_be32(IPR_IOADL_FLAGS_WRITE | sg_dma_len(sg));
 		ioadl[i].address =
-			cpu_to_be32(sg_dma_address(&scatterlist[i]));
+			cpu_to_be32(sg_dma_address(sg));
 	}
 
 	ioadl[i-1].flags_and_data_len |=
-- 
2.20.1


^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH V4 06/16] scsi: pmcraid: use sg helper to operate scatterlist
  2019-06-17  3:03 [PATCH V4 00/16] use sg helper to operate scatterlist Ming Lei
                   ` (4 preceding siblings ...)
  2019-06-17  3:03 ` [PATCH V4 05/16] scsi: ipr: " Ming Lei
@ 2019-06-17  3:03 ` " Ming Lei
  2019-06-17  8:24   ` Christoph Hellwig
  2019-06-17  3:03 ` [PATCH V4 07/16] usb: image: microtek: " Ming Lei
                   ` (10 subsequent siblings)
  16 siblings, 1 reply; 40+ messages in thread
From: Ming Lei @ 2019-06-17  3:03 UTC (permalink / raw)
  To: linux-scsi, Martin K . Petersen
  Cc: James Bottomley, Bart Van Assche, Hannes Reinecke,
	Christoph Hellwig, Jim Gill, Cathy Avery, Ewan D . Milne,
	Brian King, James Smart, Juergen E . Fischer, Michael Schmitz,
	Finn Thain, Greg Kroah-Hartman, devel, linux-usb, Dan Carpenter,
	Benjamin Block, Ming Lei

Use the scatterlist iterators and remove direct indexing of the
scatterlist array.

This way allows us to pre-allocate one small scatterlist, which can be
chained with one runtime allocated scatterlist if the pre-allocated one
isn't enough for the whole request.

Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 drivers/scsi/pmcraid.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/pmcraid.c b/drivers/scsi/pmcraid.c
index e338d7a4f571..286cac59cb5f 100644
--- a/drivers/scsi/pmcraid.c
+++ b/drivers/scsi/pmcraid.c
@@ -3270,7 +3270,7 @@ static int pmcraid_copy_sglist(
 	int direction
 )
 {
-	struct scatterlist *scatterlist;
+	struct scatterlist *sg;
 	void *kaddr;
 	int bsize_elem;
 	int i;
@@ -3279,10 +3279,10 @@ static int pmcraid_copy_sglist(
 	/* Determine the actual number of bytes per element */
 	bsize_elem = PAGE_SIZE * (1 << sglist->order);
 
-	scatterlist = sglist->scatterlist;
+	sg = sglist->scatterlist;
 
-	for (i = 0; i < (len / bsize_elem); i++, buffer += bsize_elem) {
-		struct page *page = sg_page(&scatterlist[i]);
+	for (i = 0; i < (len / bsize_elem); i++, sg = sg_next(sg), buffer += bsize_elem) {
+		struct page *page = sg_page(sg);
 
 		kaddr = kmap(page);
 		if (direction == DMA_TO_DEVICE)
@@ -3297,11 +3297,11 @@ static int pmcraid_copy_sglist(
 			return -EFAULT;
 		}
 
-		scatterlist[i].length = bsize_elem;
+		sg->length = bsize_elem;
 	}
 
 	if (len % bsize_elem) {
-		struct page *page = sg_page(&scatterlist[i]);
+		struct page *page = sg_page(sg);
 
 		kaddr = kmap(page);
 
@@ -3312,7 +3312,7 @@ static int pmcraid_copy_sglist(
 
 		kunmap(page);
 
-		scatterlist[i].length = len % bsize_elem;
+		sg->length = len % bsize_elem;
 	}
 
 	if (rc) {
-- 
2.20.1


^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH V4 07/16] usb: image: microtek: use sg helper to operate scatterlist
  2019-06-17  3:03 [PATCH V4 00/16] use sg helper to operate scatterlist Ming Lei
                   ` (5 preceding siblings ...)
  2019-06-17  3:03 ` [PATCH V4 06/16] scsi: pmcraid: " Ming Lei
@ 2019-06-17  3:03 ` " Ming Lei
  2019-06-17  8:25   ` Christoph Hellwig
  2019-06-17  3:03 ` [PATCH V4 08/16] staging: unisys: visorhba: " Ming Lei
                   ` (9 subsequent siblings)
  16 siblings, 1 reply; 40+ messages in thread
From: Ming Lei @ 2019-06-17  3:03 UTC (permalink / raw)
  To: linux-scsi, Martin K . Petersen
  Cc: James Bottomley, Bart Van Assche, Hannes Reinecke,
	Christoph Hellwig, Jim Gill, Cathy Avery, Ewan D . Milne,
	Brian King, James Smart, Juergen E . Fischer, Michael Schmitz,
	Finn Thain, Greg Kroah-Hartman, devel, linux-usb, Dan Carpenter,
	Benjamin Block, Ming Lei, Oliver Neukum

Use the scatterlist iterators and remove direct indexing of the
scatterlist array.

This way allows us to pre-allocate one small scatterlist, which can be
chained with one runtime allocated scatterlist if the pre-allocated one
isn't enough for the whole request.

Cc: Oliver Neukum <oliver@neukum.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 drivers/usb/image/microtek.c | 20 ++++++++------------
 drivers/usb/image/microtek.h |  2 +-
 2 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/image/microtek.c b/drivers/usb/image/microtek.c
index 607be1f4fe27..0a57c2cc8e5a 100644
--- a/drivers/usb/image/microtek.c
+++ b/drivers/usb/image/microtek.c
@@ -488,7 +488,6 @@ static void mts_command_done( struct urb *transfer )
 
 static void mts_do_sg (struct urb* transfer)
 {
-	struct scatterlist * sg;
 	int status = transfer->status;
 	MTS_INT_INIT();
 
@@ -500,13 +499,12 @@ static void mts_do_sg (struct urb* transfer)
 		mts_transfer_cleanup(transfer);
         }
 
-	sg = scsi_sglist(context->srb);
-	context->fragment++;
+	context->curr_sg = sg_next(context->curr_sg);
 	mts_int_submit_urb(transfer,
 			   context->data_pipe,
-			   sg_virt(&sg[context->fragment]),
-			   sg[context->fragment].length,
-			   context->fragment + 1 == scsi_sg_count(context->srb) ?
+			   sg_virt(context->curr_sg),
+			   context->curr_sg->length,
+			   sg_is_last(context->curr_sg) ?
 			   mts_data_done : mts_do_sg);
 }
 
@@ -526,22 +524,20 @@ static void
 mts_build_transfer_context(struct scsi_cmnd *srb, struct mts_desc* desc)
 {
 	int pipe;
-	struct scatterlist * sg;
-	
+
 	MTS_DEBUG_GOT_HERE();
 
 	desc->context.instance = desc;
 	desc->context.srb = srb;
-	desc->context.fragment = 0;
 
 	if (!scsi_bufflen(srb)) {
 		desc->context.data = NULL;
 		desc->context.data_length = 0;
 		return;
 	} else {
-		sg = scsi_sglist(srb);
-		desc->context.data = sg_virt(&sg[0]);
-		desc->context.data_length = sg[0].length;
+		desc->context.curr_sg = scsi_sglist(srb);
+		desc->context.data = sg_virt(desc->context.curr_sg);
+		desc->context.data_length = desc->context.curr_sg->length;
 	}
 
 
diff --git a/drivers/usb/image/microtek.h b/drivers/usb/image/microtek.h
index 66685e59241a..7bd5f4639c4a 100644
--- a/drivers/usb/image/microtek.h
+++ b/drivers/usb/image/microtek.h
@@ -21,7 +21,7 @@ struct mts_transfer_context
 	void *data;
 	unsigned data_length;
 	int data_pipe;
-	int fragment;
+	struct scatterlist *curr_sg;
 
 	u8 *scsi_status; /* status returned from ep_response after command completion */
 };
-- 
2.20.1


^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH V4 08/16] staging: unisys: visorhba: use sg helper to operate scatterlist
  2019-06-17  3:03 [PATCH V4 00/16] use sg helper to operate scatterlist Ming Lei
                   ` (6 preceding siblings ...)
  2019-06-17  3:03 ` [PATCH V4 07/16] usb: image: microtek: " Ming Lei
@ 2019-06-17  3:03 ` " Ming Lei
  2019-06-17  8:25   ` Christoph Hellwig
  2019-06-17  3:03 ` [PATCH V4 09/16] staging: rtsx: " Ming Lei
                   ` (8 subsequent siblings)
  16 siblings, 1 reply; 40+ messages in thread
From: Ming Lei @ 2019-06-17  3:03 UTC (permalink / raw)
  To: linux-scsi, Martin K . Petersen
  Cc: James Bottomley, Bart Van Assche, Hannes Reinecke,
	Christoph Hellwig, Jim Gill, Cathy Avery, Ewan D . Milne,
	Brian King, James Smart, Juergen E . Fischer, Michael Schmitz,
	Finn Thain, Greg Kroah-Hartman, devel, linux-usb, Dan Carpenter,
	Benjamin Block, Ming Lei

Use the scatterlist iterators and remove direct indexing of the
scatterlist array.

This way allows us to pre-allocate one small scatterlist, which can be
chained with one runtime allocated scatterlist if the pre-allocated one
isn't enough for the whole request.

Cc: devel@driverdev.osuosl.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 drivers/staging/unisys/visorhba/visorhba_main.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/unisys/visorhba/visorhba_main.c b/drivers/staging/unisys/visorhba/visorhba_main.c
index 2dad36a05518..dd979ee4dcf1 100644
--- a/drivers/staging/unisys/visorhba/visorhba_main.c
+++ b/drivers/staging/unisys/visorhba/visorhba_main.c
@@ -871,12 +871,11 @@ static void do_scsi_nolinuxstat(struct uiscmdrsp *cmdrsp,
 			return;
 		}
 
-		sg = scsi_sglist(scsicmd);
-		for (i = 0; i < scsi_sg_count(scsicmd); i++) {
-			this_page_orig = kmap_atomic(sg_page(sg + i));
+		scsi_for_each_sg(scsicmd, sg, scsi_sg_count(scsicmd), i) {
+			this_page_orig = kmap_atomic(sg_page(sg));
 			this_page = (void *)((unsigned long)this_page_orig |
-					     sg[i].offset);
-			memcpy(this_page, buf + bufind, sg[i].length);
+					     sg->offset);
+			memcpy(this_page, buf + bufind, sg->length);
 			kunmap_atomic(this_page_orig);
 		}
 		kfree(buf);
-- 
2.20.1


^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH V4 09/16] staging: rtsx: use sg helper to operate scatterlist
  2019-06-17  3:03 [PATCH V4 00/16] use sg helper to operate scatterlist Ming Lei
                   ` (7 preceding siblings ...)
  2019-06-17  3:03 ` [PATCH V4 08/16] staging: unisys: visorhba: " Ming Lei
@ 2019-06-17  3:03 ` " Ming Lei
  2019-06-17  8:27   ` Christoph Hellwig
  2019-06-17  3:03 ` [PATCH V4 10/16] s390: zfcp_fc: " Ming Lei
                   ` (7 subsequent siblings)
  16 siblings, 1 reply; 40+ messages in thread
From: Ming Lei @ 2019-06-17  3:03 UTC (permalink / raw)
  To: linux-scsi, Martin K . Petersen
  Cc: James Bottomley, Bart Van Assche, Hannes Reinecke,
	Christoph Hellwig, Jim Gill, Cathy Avery, Ewan D . Milne,
	Brian King, James Smart, Juergen E . Fischer, Michael Schmitz,
	Finn Thain, Greg Kroah-Hartman, devel, linux-usb, Dan Carpenter,
	Benjamin Block, Ming Lei, Kim Bradley

Use the scatterlist iterators and remove direct indexing of the
scatterlist array.

This way allows us to pre-allocate one small scatterlist, which can be
chained with one runtime allocated scatterlist if the pre-allocated one
isn't enough for the whole request.

Cc: Kim Bradley <kim.jamie.bradley@gmail.com>
Cc: devel@driverdev.osuosl.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 drivers/staging/rts5208/rtsx_transport.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/rts5208/rtsx_transport.c b/drivers/staging/rts5208/rtsx_transport.c
index 8277d7895608..407c9079b052 100644
--- a/drivers/staging/rts5208/rtsx_transport.c
+++ b/drivers/staging/rts5208/rtsx_transport.c
@@ -63,6 +63,8 @@ unsigned int rtsx_stor_access_xfer_buf(unsigned char *buffer,
 		struct scatterlist *sg =
 				(struct scatterlist *)scsi_sglist(srb)
 				+ *index;
+		if (sg_is_chain(sg))
+			sg = sg_chain_ptr(sg);
 
 		/*
 		 * This loop handles a single s-g list entry, which may
@@ -86,7 +88,7 @@ unsigned int rtsx_stor_access_xfer_buf(unsigned char *buffer,
 				/* Transfer continues to next s-g entry */
 				*offset = 0;
 				++*index;
-				++sg;
+				sg = sg_next(sg);
 			}
 
 			while (sglen > 0) {
-- 
2.20.1


^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH V4 10/16] s390: zfcp_fc: use sg helper to operate scatterlist
  2019-06-17  3:03 [PATCH V4 00/16] use sg helper to operate scatterlist Ming Lei
                   ` (8 preceding siblings ...)
  2019-06-17  3:03 ` [PATCH V4 09/16] staging: rtsx: " Ming Lei
@ 2019-06-17  3:03 ` " Ming Lei
  2019-06-17  8:27   ` Christoph Hellwig
  2019-06-17  3:03 ` [PATCH V4 11/16] scsi: aha152x: " Ming Lei
                   ` (6 subsequent siblings)
  16 siblings, 1 reply; 40+ messages in thread
From: Ming Lei @ 2019-06-17  3:03 UTC (permalink / raw)
  To: linux-scsi, Martin K . Petersen
  Cc: James Bottomley, Bart Van Assche, Hannes Reinecke,
	Christoph Hellwig, Jim Gill, Cathy Avery, Ewan D . Milne,
	Brian King, James Smart, Juergen E . Fischer, Michael Schmitz,
	Finn Thain, Greg Kroah-Hartman, devel, linux-usb, Dan Carpenter,
	Benjamin Block, Ming Lei, Steffen Maier, Martin Schwidefsky,
	Heiko Carstens, linux-s390

Use the scatterlist iterators and remove direct indexing of the
scatterlist array.

This way allows us to pre-allocate one small scatterlist, which can be
chained with one runtime allocated scatterlist if the pre-allocated one
isn't enough for the whole request.

Cc: Steffen Maier <maier@linux.ibm.com>
Cc: Benjamin Block <bblock@linux.ibm.com>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: linux-s390@vger.kernel.org
Acked-by: Benjamin Block <bblock@linux.ibm.com>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 drivers/s390/scsi/zfcp_fc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/s390/scsi/zfcp_fc.c b/drivers/s390/scsi/zfcp_fc.c
index 33eddb02ee30..b018b61bd168 100644
--- a/drivers/s390/scsi/zfcp_fc.c
+++ b/drivers/s390/scsi/zfcp_fc.c
@@ -620,7 +620,7 @@ static void zfcp_fc_sg_free_table(struct scatterlist *sg, int count)
 {
 	int i;
 
-	for (i = 0; i < count; i++, sg++)
+	for (i = 0; i < count; i++, sg = sg_next(sg))
 		if (sg)
 			free_page((unsigned long) sg_virt(sg));
 		else
@@ -641,7 +641,7 @@ static int zfcp_fc_sg_setup_table(struct scatterlist *sg, int count)
 	int i;
 
 	sg_init_table(sg, count);
-	for (i = 0; i < count; i++, sg++) {
+	for (i = 0; i < count; i++, sg = sg_next(sg)) {
 		addr = (void *) get_zeroed_page(GFP_KERNEL);
 		if (!addr) {
 			zfcp_fc_sg_free_table(sg, i);
-- 
2.20.1


^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH V4 11/16] scsi: aha152x: use sg helper to operate scatterlist
  2019-06-17  3:03 [PATCH V4 00/16] use sg helper to operate scatterlist Ming Lei
                   ` (9 preceding siblings ...)
  2019-06-17  3:03 ` [PATCH V4 10/16] s390: zfcp_fc: " Ming Lei
@ 2019-06-17  3:03 ` " Ming Lei
  2019-06-17  3:35   ` Finn Thain
  2019-06-17  8:28   ` Christoph Hellwig
  2019-06-17  3:03 ` [PATCH V4 12/16] scsi: imm: " Ming Lei
                   ` (5 subsequent siblings)
  16 siblings, 2 replies; 40+ messages in thread
From: Ming Lei @ 2019-06-17  3:03 UTC (permalink / raw)
  To: linux-scsi, Martin K . Petersen
  Cc: James Bottomley, Bart Van Assche, Hannes Reinecke,
	Christoph Hellwig, Jim Gill, Cathy Avery, Ewan D . Milne,
	Brian King, James Smart, Juergen E . Fischer, Michael Schmitz,
	Finn Thain, Greg Kroah-Hartman, devel, linux-usb, Dan Carpenter,
	Benjamin Block, Ming Lei

Use the scatterlist iterators and remove direct indexing of the
scatterlist array.

This way allows us to pre-allocate one small scatterlist, which can be
chained with one runtime allocated scatterlist if the pre-allocated one
isn't enough for the whole request.

Finn added the change to replace SCp.buffers_residual with sg_is_last()
for fixing updating it, and the similar change has been applied on
NCR5380.c

Cc: Finn Thain <fthain@telegraphics.com.au>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 drivers/scsi/aha152x.c | 42 ++++++++++++++++++++----------------------
 1 file changed, 20 insertions(+), 22 deletions(-)

diff --git a/drivers/scsi/aha152x.c b/drivers/scsi/aha152x.c
index 97872838b983..6d0518f616cb 100644
--- a/drivers/scsi/aha152x.c
+++ b/drivers/scsi/aha152x.c
@@ -948,7 +948,6 @@ static int aha152x_internal_queue(struct scsi_cmnd *SCpnt,
 	   SCp.ptr              : buffer pointer
 	   SCp.this_residual    : buffer length
 	   SCp.buffer           : next buffer
-	   SCp.buffers_residual : left buffers in list
 	   SCp.phase            : current state of the command */
 
 	if ((phase & resetting) || !scsi_sglist(SCpnt)) {
@@ -956,13 +955,11 @@ static int aha152x_internal_queue(struct scsi_cmnd *SCpnt,
 		SCpnt->SCp.this_residual = 0;
 		scsi_set_resid(SCpnt, 0);
 		SCpnt->SCp.buffer           = NULL;
-		SCpnt->SCp.buffers_residual = 0;
 	} else {
 		scsi_set_resid(SCpnt, scsi_bufflen(SCpnt));
 		SCpnt->SCp.buffer           = scsi_sglist(SCpnt);
 		SCpnt->SCp.ptr              = SG_ADDRESS(SCpnt->SCp.buffer);
 		SCpnt->SCp.this_residual    = SCpnt->SCp.buffer->length;
-		SCpnt->SCp.buffers_residual = scsi_sg_count(SCpnt) - 1;
 	}
 
 	DO_LOCK(flags);
@@ -2030,10 +2027,9 @@ static void datai_run(struct Scsi_Host *shpnt)
 				}
 
 				if (CURRENT_SC->SCp.this_residual == 0 &&
-				    CURRENT_SC->SCp.buffers_residual > 0) {
+				    !sg_is_last(CURRENT_SC->SCp.buffer)) {
 					/* advance to next buffer */
-					CURRENT_SC->SCp.buffers_residual--;
-					CURRENT_SC->SCp.buffer++;
+					CURRENT_SC->SCp.buffer = sg_next(CURRENT_SC->SCp.buffer);
 					CURRENT_SC->SCp.ptr           = SG_ADDRESS(CURRENT_SC->SCp.buffer);
 					CURRENT_SC->SCp.this_residual = CURRENT_SC->SCp.buffer->length;
 				}
@@ -2136,10 +2132,10 @@ static void datao_run(struct Scsi_Host *shpnt)
 			CMD_INC_RESID(CURRENT_SC, -2 * data_count);
 		}
 
-		if(CURRENT_SC->SCp.this_residual==0 && CURRENT_SC->SCp.buffers_residual>0) {
+		if(CURRENT_SC->SCp.this_residual==0 &&
+		   !sg_is_last(CURRENT_SC->SCp.buffer)) {
 			/* advance to next buffer */
-			CURRENT_SC->SCp.buffers_residual--;
-			CURRENT_SC->SCp.buffer++;
+			CURRENT_SC->SCp.buffer = sg_next(CURRENT_SC->SCp.buffer);
 			CURRENT_SC->SCp.ptr           = SG_ADDRESS(CURRENT_SC->SCp.buffer);
 			CURRENT_SC->SCp.this_residual = CURRENT_SC->SCp.buffer->length;
 		}
@@ -2158,22 +2154,24 @@ static void datao_run(struct Scsi_Host *shpnt)
 static void datao_end(struct Scsi_Host *shpnt)
 {
 	if(TESTLO(DMASTAT, DFIFOEMP)) {
-		int data_count = (DATA_LEN - scsi_get_resid(CURRENT_SC)) -
-			GETSTCNT();
+		int done = GETSTCNT();
+		int data_count = (DATA_LEN - scsi_get_resid(CURRENT_SC)) - done;
+		struct scatterlist *sg = scsi_sglist(CURRENT_SC);
 
 		CMD_INC_RESID(CURRENT_SC, data_count);
 
-		data_count -= CURRENT_SC->SCp.ptr -
-			SG_ADDRESS(CURRENT_SC->SCp.buffer);
-		while(data_count>0) {
-			CURRENT_SC->SCp.buffer--;
-			CURRENT_SC->SCp.buffers_residual++;
-			data_count -= CURRENT_SC->SCp.buffer->length;
+		/* Locate the first SG entry not yet sent */
+		while (done > 0 && !sg_is_last(sg)) {
+			if (done < sg->length)
+				break;
+			done -= sg->length;
+			sg = sg_next(sg);
 		}
-		CURRENT_SC->SCp.ptr = SG_ADDRESS(CURRENT_SC->SCp.buffer) -
-			data_count;
-		CURRENT_SC->SCp.this_residual = CURRENT_SC->SCp.buffer->length +
-			data_count;
+
+		CURRENT_SC->SCp.buffer = sg;
+		CURRENT_SC->SCp.ptr = SG_ADDRESS(CURRENT_SC->SCp.buffer) + done;
+		CURRENT_SC->SCp.this_residual = CURRENT_SC->SCp.buffer->length -
+			done;
 	}
 
 	SETPORT(SXFRCTL0, CH1|CLRCH1|CLRSTCNT);
@@ -2501,7 +2499,7 @@ static void get_command(struct seq_file *m, struct scsi_cmnd * ptr)
 
 	seq_printf(m, "); resid=%d; residual=%d; buffers=%d; phase |",
 		scsi_get_resid(ptr), ptr->SCp.this_residual,
-		ptr->SCp.buffers_residual);
+		sg_nents(ptr->SCp.buffer) - 1);
 
 	if (ptr->SCp.phase & not_issued)
 		seq_puts(m, "not issued|");
-- 
2.20.1


^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH V4 12/16] scsi: imm: use sg helper to operate scatterlist
  2019-06-17  3:03 [PATCH V4 00/16] use sg helper to operate scatterlist Ming Lei
                   ` (10 preceding siblings ...)
  2019-06-17  3:03 ` [PATCH V4 11/16] scsi: aha152x: " Ming Lei
@ 2019-06-17  3:03 ` " Ming Lei
  2019-06-17  8:28   ` Christoph Hellwig
  2019-06-17  3:03 ` [PATCH V4 13/16] scsi: pcmcia: nsp_cs: " Ming Lei
                   ` (4 subsequent siblings)
  16 siblings, 1 reply; 40+ messages in thread
From: Ming Lei @ 2019-06-17  3:03 UTC (permalink / raw)
  To: linux-scsi, Martin K . Petersen
  Cc: James Bottomley, Bart Van Assche, Hannes Reinecke,
	Christoph Hellwig, Jim Gill, Cathy Avery, Ewan D . Milne,
	Brian King, James Smart, Juergen E . Fischer, Michael Schmitz,
	Finn Thain, Greg Kroah-Hartman, devel, linux-usb, Dan Carpenter,
	Benjamin Block, Ming Lei

Use the scatterlist iterators and remove direct indexing of the
scatterlist array.

This way allows us to pre-allocate one small scatterlist, which can be
chained with one runtime allocated scatterlist if the pre-allocated one
isn't enough for the whole request.

Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 drivers/scsi/imm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/imm.c b/drivers/scsi/imm.c
index 64ae418d29f3..56d29f157749 100644
--- a/drivers/scsi/imm.c
+++ b/drivers/scsi/imm.c
@@ -686,7 +686,7 @@ static int imm_completion(struct scsi_cmnd *cmd)
 		if (cmd->SCp.buffer && !cmd->SCp.this_residual) {
 			/* if scatter/gather, advance to the next segment */
 			if (cmd->SCp.buffers_residual--) {
-				cmd->SCp.buffer++;
+				cmd->SCp.buffer = sg_next(cmd->SCp.buffer);
 				cmd->SCp.this_residual =
 				    cmd->SCp.buffer->length;
 				cmd->SCp.ptr = sg_virt(cmd->SCp.buffer);
-- 
2.20.1


^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH V4 13/16] scsi: pcmcia: nsp_cs: use sg helper to operate scatterlist
  2019-06-17  3:03 [PATCH V4 00/16] use sg helper to operate scatterlist Ming Lei
                   ` (11 preceding siblings ...)
  2019-06-17  3:03 ` [PATCH V4 12/16] scsi: imm: " Ming Lei
@ 2019-06-17  3:03 ` " Ming Lei
  2019-06-17  8:29   ` Christoph Hellwig
  2019-06-17  3:03 ` [PATCH V4 14/16] scsi: ppa: " Ming Lei
                   ` (3 subsequent siblings)
  16 siblings, 1 reply; 40+ messages in thread
From: Ming Lei @ 2019-06-17  3:03 UTC (permalink / raw)
  To: linux-scsi, Martin K . Petersen
  Cc: James Bottomley, Bart Van Assche, Hannes Reinecke,
	Christoph Hellwig, Jim Gill, Cathy Avery, Ewan D . Milne,
	Brian King, James Smart, Juergen E . Fischer, Michael Schmitz,
	Finn Thain, Greg Kroah-Hartman, devel, linux-usb, Dan Carpenter,
	Benjamin Block, Ming Lei

Use the scatterlist iterators and remove direct indexing of the
scatterlist array.

This way allows us to pre-allocate one small scatterlist, which can be
chained with one runtime allocated scatterlist if the pre-allocated one
isn't enough for the whole request.

Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 drivers/scsi/pcmcia/nsp_cs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/pcmcia/nsp_cs.c b/drivers/scsi/pcmcia/nsp_cs.c
index a81748e6e8fb..97416e1dcc5b 100644
--- a/drivers/scsi/pcmcia/nsp_cs.c
+++ b/drivers/scsi/pcmcia/nsp_cs.c
@@ -789,7 +789,7 @@ static void nsp_pio_read(struct scsi_cmnd *SCpnt)
 		    SCpnt->SCp.buffers_residual != 0 ) {
 			//nsp_dbg(NSP_DEBUG_DATA_IO, "scatterlist next timeout=%d", time_out);
 			SCpnt->SCp.buffers_residual--;
-			SCpnt->SCp.buffer++;
+			SCpnt->SCp.buffer = sg_next(SCpnt->SCp.buffer);
 			SCpnt->SCp.ptr		 = BUFFER_ADDR;
 			SCpnt->SCp.this_residual = SCpnt->SCp.buffer->length;
 			time_out = 1000;
@@ -887,7 +887,7 @@ static void nsp_pio_write(struct scsi_cmnd *SCpnt)
 		    SCpnt->SCp.buffers_residual != 0 ) {
 			//nsp_dbg(NSP_DEBUG_DATA_IO, "scatterlist next");
 			SCpnt->SCp.buffers_residual--;
-			SCpnt->SCp.buffer++;
+			SCpnt->SCp.buffer = sg_next(SCpnt->SCp.buffer);
 			SCpnt->SCp.ptr		 = BUFFER_ADDR;
 			SCpnt->SCp.this_residual = SCpnt->SCp.buffer->length;
 			time_out = 1000;
-- 
2.20.1


^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH V4 14/16] scsi: ppa: use sg helper to operate scatterlist
  2019-06-17  3:03 [PATCH V4 00/16] use sg helper to operate scatterlist Ming Lei
                   ` (12 preceding siblings ...)
  2019-06-17  3:03 ` [PATCH V4 13/16] scsi: pcmcia: nsp_cs: " Ming Lei
@ 2019-06-17  3:03 ` " Ming Lei
  2019-06-17  8:29   ` Christoph Hellwig
  2019-06-17  3:03 ` [PATCH V4 15/16] scsi: wd33c93: " Ming Lei
                   ` (2 subsequent siblings)
  16 siblings, 1 reply; 40+ messages in thread
From: Ming Lei @ 2019-06-17  3:03 UTC (permalink / raw)
  To: linux-scsi, Martin K . Petersen
  Cc: James Bottomley, Bart Van Assche, Hannes Reinecke,
	Christoph Hellwig, Jim Gill, Cathy Avery, Ewan D . Milne,
	Brian King, James Smart, Juergen E . Fischer, Michael Schmitz,
	Finn Thain, Greg Kroah-Hartman, devel, linux-usb, Dan Carpenter,
	Benjamin Block, Ming Lei

Use the scatterlist iterators and remove direct indexing of the
scatterlist array.

This way allows us to pre-allocate one small scatterlist, which can be
chained with one runtime allocated scatterlist if the pre-allocated one
isn't enough for the whole request.

Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 drivers/scsi/ppa.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/ppa.c b/drivers/scsi/ppa.c
index 35213082e933..a406cc825426 100644
--- a/drivers/scsi/ppa.c
+++ b/drivers/scsi/ppa.c
@@ -590,7 +590,7 @@ static int ppa_completion(struct scsi_cmnd *cmd)
 		if (cmd->SCp.buffer && !cmd->SCp.this_residual) {
 			/* if scatter/gather, advance to the next segment */
 			if (cmd->SCp.buffers_residual--) {
-				cmd->SCp.buffer++;
+				cmd->SCp.buffer = sg_next(cmd->SCp.buffer);
 				cmd->SCp.this_residual =
 				    cmd->SCp.buffer->length;
 				cmd->SCp.ptr = sg_virt(cmd->SCp.buffer);
-- 
2.20.1


^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH V4 15/16] scsi: wd33c93: use sg helper to operate scatterlist
  2019-06-17  3:03 [PATCH V4 00/16] use sg helper to operate scatterlist Ming Lei
                   ` (13 preceding siblings ...)
  2019-06-17  3:03 ` [PATCH V4 14/16] scsi: ppa: " Ming Lei
@ 2019-06-17  3:03 ` " Ming Lei
  2019-06-17  8:29   ` Christoph Hellwig
  2019-06-17  3:03 ` [PATCH V4 16/16] NCR5380: Support chained sg lists Ming Lei
  2019-06-17 23:52 ` [PATCH V4 00/16] use sg helper to operate scatterlist Bart Van Assche
  16 siblings, 1 reply; 40+ messages in thread
From: Ming Lei @ 2019-06-17  3:03 UTC (permalink / raw)
  To: linux-scsi, Martin K . Petersen
  Cc: James Bottomley, Bart Van Assche, Hannes Reinecke,
	Christoph Hellwig, Jim Gill, Cathy Avery, Ewan D . Milne,
	Brian King, James Smart, Juergen E . Fischer, Michael Schmitz,
	Finn Thain, Greg Kroah-Hartman, devel, linux-usb, Dan Carpenter,
	Benjamin Block, Ming Lei

Use the scatterlist iterators and remove direct indexing of the
scatterlist array.

This way allows us to pre-allocate one small scatterlist, which can be
chained with one runtime allocated scatterlist if the pre-allocated one
isn't enough for the whole request.

Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 drivers/scsi/wd33c93.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/wd33c93.c b/drivers/scsi/wd33c93.c
index 74be04f2357c..ae5935c0a149 100644
--- a/drivers/scsi/wd33c93.c
+++ b/drivers/scsi/wd33c93.c
@@ -744,7 +744,7 @@ transfer_bytes(const wd33c93_regs regs, struct scsi_cmnd *cmd,
  * source or destination for THIS transfer.
  */
 	if (!cmd->SCp.this_residual && cmd->SCp.buffers_residual) {
-		++cmd->SCp.buffer;
+		cmd->SCp.buffer = sg_next(cmd->SCp.buffer);
 		--cmd->SCp.buffers_residual;
 		cmd->SCp.this_residual = cmd->SCp.buffer->length;
 		cmd->SCp.ptr = sg_virt(cmd->SCp.buffer);
-- 
2.20.1


^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH V4 16/16] NCR5380: Support chained sg lists
  2019-06-17  3:03 [PATCH V4 00/16] use sg helper to operate scatterlist Ming Lei
                   ` (14 preceding siblings ...)
  2019-06-17  3:03 ` [PATCH V4 15/16] scsi: wd33c93: " Ming Lei
@ 2019-06-17  3:03 ` Ming Lei
  2019-06-17  8:30   ` Christoph Hellwig
  2019-06-17 23:52 ` [PATCH V4 00/16] use sg helper to operate scatterlist Bart Van Assche
  16 siblings, 1 reply; 40+ messages in thread
From: Ming Lei @ 2019-06-17  3:03 UTC (permalink / raw)
  To: linux-scsi, Martin K . Petersen
  Cc: James Bottomley, Bart Van Assche, Hannes Reinecke,
	Christoph Hellwig, Jim Gill, Cathy Avery, Ewan D . Milne,
	Brian King, James Smart, Juergen E . Fischer, Michael Schmitz,
	Finn Thain, Greg Kroah-Hartman, devel, linux-usb, Dan Carpenter,
	Benjamin Block

From: Finn Thain <fthain@telegraphics.com.au>

My understanding is that support for chained scatterlists is to
become mandatory for LLDs.

Use the scatterlist iterators and remove direct indexing of the
scatterlist array.

This way allows us to pre-allocate one small scatterlist, which can be
chained with one runtime allocated scatterlist if the pre-allocated one
isn't enough for the whole request.

Cc: Michael Schmitz <schmitzmic@gmail.com>
Reviewed-by: Michael Schmitz <schmitzmic@gmail.com>
Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
 drivers/scsi/NCR5380.c | 41 ++++++++++++++++++-----------------------
 1 file changed, 18 insertions(+), 23 deletions(-)

diff --git a/drivers/scsi/NCR5380.c b/drivers/scsi/NCR5380.c
index fe0535affc14..4ef44fafe6ca 100644
--- a/drivers/scsi/NCR5380.c
+++ b/drivers/scsi/NCR5380.c
@@ -149,12 +149,10 @@ static inline void initialize_SCp(struct scsi_cmnd *cmd)
 
 	if (scsi_bufflen(cmd)) {
 		cmd->SCp.buffer = scsi_sglist(cmd);
-		cmd->SCp.buffers_residual = scsi_sg_count(cmd) - 1;
 		cmd->SCp.ptr = sg_virt(cmd->SCp.buffer);
 		cmd->SCp.this_residual = cmd->SCp.buffer->length;
 	} else {
 		cmd->SCp.buffer = NULL;
-		cmd->SCp.buffers_residual = 0;
 		cmd->SCp.ptr = NULL;
 		cmd->SCp.this_residual = 0;
 	}
@@ -163,6 +161,17 @@ static inline void initialize_SCp(struct scsi_cmnd *cmd)
 	cmd->SCp.Message = 0;
 }
 
+static inline void advance_sg_buffer(struct scsi_cmnd *cmd)
+{
+	struct scatterlist *s = cmd->SCp.buffer;
+
+	if (!cmd->SCp.this_residual && s && !sg_is_last(s)) {
+		cmd->SCp.buffer = sg_next(s);
+		cmd->SCp.ptr = sg_virt(cmd->SCp.buffer);
+		cmd->SCp.this_residual = cmd->SCp.buffer->length;
+	}
+}
+
 /**
  * NCR5380_poll_politely2 - wait for two chip register values
  * @hostdata: host private data
@@ -1672,12 +1681,7 @@ static void NCR5380_information_transfer(struct Scsi_Host *instance)
 			    sun3_dma_setup_done != cmd) {
 				int count;
 
-				if (!cmd->SCp.this_residual && cmd->SCp.buffers_residual) {
-					++cmd->SCp.buffer;
-					--cmd->SCp.buffers_residual;
-					cmd->SCp.this_residual = cmd->SCp.buffer->length;
-					cmd->SCp.ptr = sg_virt(cmd->SCp.buffer);
-				}
+				advance_sg_buffer(cmd);
 
 				count = sun3scsi_dma_xfer_len(hostdata, cmd);
 
@@ -1727,15 +1731,11 @@ static void NCR5380_information_transfer(struct Scsi_Host *instance)
 				 * scatter-gather list, move onto the next one.
 				 */
 
-				if (!cmd->SCp.this_residual && cmd->SCp.buffers_residual) {
-					++cmd->SCp.buffer;
-					--cmd->SCp.buffers_residual;
-					cmd->SCp.this_residual = cmd->SCp.buffer->length;
-					cmd->SCp.ptr = sg_virt(cmd->SCp.buffer);
-					dsprintk(NDEBUG_INFORMATION, instance, "%d bytes and %d buffers left\n",
-					         cmd->SCp.this_residual,
-					         cmd->SCp.buffers_residual);
-				}
+				advance_sg_buffer(cmd);
+				dsprintk(NDEBUG_INFORMATION, instance,
+					"this residual %d, sg ents %d\n",
+					cmd->SCp.this_residual,
+					sg_nents(cmd->SCp.buffer));
 
 				/*
 				 * The preferred transfer method is going to be
@@ -2136,12 +2136,7 @@ static void NCR5380_reselect(struct Scsi_Host *instance)
 	if (sun3_dma_setup_done != tmp) {
 		int count;
 
-		if (!tmp->SCp.this_residual && tmp->SCp.buffers_residual) {
-			++tmp->SCp.buffer;
-			--tmp->SCp.buffers_residual;
-			tmp->SCp.this_residual = tmp->SCp.buffer->length;
-			tmp->SCp.ptr = sg_virt(tmp->SCp.buffer);
-		}
+		advance_sg_buffer(tmp);
 
 		count = sun3scsi_dma_xfer_len(hostdata, tmp);
 
-- 
2.20.1


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 11/16] scsi: aha152x: use sg helper to operate scatterlist
  2019-06-17  3:03 ` [PATCH V4 11/16] scsi: aha152x: " Ming Lei
@ 2019-06-17  3:35   ` Finn Thain
  2019-06-17 23:35     ` Finn Thain
  2019-06-17  8:28   ` Christoph Hellwig
  1 sibling, 1 reply; 40+ messages in thread
From: Finn Thain @ 2019-06-17  3:35 UTC (permalink / raw)
  To: Ming Lei
  Cc: linux-scsi, Martin K . Petersen, James Bottomley,
	Bart Van Assche, Hannes Reinecke, Christoph Hellwig, Jim Gill,
	Cathy Avery, Ewan D . Milne, Brian King, James Smart,
	Juergen E . Fischer, Michael Schmitz, Greg Kroah-Hartman, devel,
	linux-usb, Dan Carpenter, Benjamin Block

On Mon, 17 Jun 2019, Ming Lei wrote:

> Use the scatterlist iterators and remove direct indexing of the
> scatterlist array.
> 
> This way allows us to pre-allocate one small scatterlist, which can be
> chained with one runtime allocated scatterlist if the pre-allocated one
> isn't enough for the whole request.
> 
> Finn added the change to replace SCp.buffers_residual with sg_is_last()
> for fixing updating it, and the similar change has been applied on
> NCR5380.c
> 
> Cc: Finn Thain <fthain@telegraphics.com.au>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>

Reviewed-by: Finn Thain <fthain@telegraphics.com.au>

-- 

> ---
>  drivers/scsi/aha152x.c | 42 ++++++++++++++++++++----------------------
>  1 file changed, 20 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/scsi/aha152x.c b/drivers/scsi/aha152x.c
> index 97872838b983..6d0518f616cb 100644
> --- a/drivers/scsi/aha152x.c
> +++ b/drivers/scsi/aha152x.c
> @@ -948,7 +948,6 @@ static int aha152x_internal_queue(struct scsi_cmnd *SCpnt,
>  	   SCp.ptr              : buffer pointer
>  	   SCp.this_residual    : buffer length
>  	   SCp.buffer           : next buffer
> -	   SCp.buffers_residual : left buffers in list
>  	   SCp.phase            : current state of the command */
>  
>  	if ((phase & resetting) || !scsi_sglist(SCpnt)) {
> @@ -956,13 +955,11 @@ static int aha152x_internal_queue(struct scsi_cmnd *SCpnt,
>  		SCpnt->SCp.this_residual = 0;
>  		scsi_set_resid(SCpnt, 0);
>  		SCpnt->SCp.buffer           = NULL;
> -		SCpnt->SCp.buffers_residual = 0;
>  	} else {
>  		scsi_set_resid(SCpnt, scsi_bufflen(SCpnt));
>  		SCpnt->SCp.buffer           = scsi_sglist(SCpnt);
>  		SCpnt->SCp.ptr              = SG_ADDRESS(SCpnt->SCp.buffer);
>  		SCpnt->SCp.this_residual    = SCpnt->SCp.buffer->length;
> -		SCpnt->SCp.buffers_residual = scsi_sg_count(SCpnt) - 1;
>  	}
>  
>  	DO_LOCK(flags);
> @@ -2030,10 +2027,9 @@ static void datai_run(struct Scsi_Host *shpnt)
>  				}
>  
>  				if (CURRENT_SC->SCp.this_residual == 0 &&
> -				    CURRENT_SC->SCp.buffers_residual > 0) {
> +				    !sg_is_last(CURRENT_SC->SCp.buffer)) {
>  					/* advance to next buffer */
> -					CURRENT_SC->SCp.buffers_residual--;
> -					CURRENT_SC->SCp.buffer++;
> +					CURRENT_SC->SCp.buffer = sg_next(CURRENT_SC->SCp.buffer);
>  					CURRENT_SC->SCp.ptr           = SG_ADDRESS(CURRENT_SC->SCp.buffer);
>  					CURRENT_SC->SCp.this_residual = CURRENT_SC->SCp.buffer->length;
>  				}
> @@ -2136,10 +2132,10 @@ static void datao_run(struct Scsi_Host *shpnt)
>  			CMD_INC_RESID(CURRENT_SC, -2 * data_count);
>  		}
>  
> -		if(CURRENT_SC->SCp.this_residual==0 && CURRENT_SC->SCp.buffers_residual>0) {
> +		if(CURRENT_SC->SCp.this_residual==0 &&
> +		   !sg_is_last(CURRENT_SC->SCp.buffer)) {
>  			/* advance to next buffer */
> -			CURRENT_SC->SCp.buffers_residual--;
> -			CURRENT_SC->SCp.buffer++;
> +			CURRENT_SC->SCp.buffer = sg_next(CURRENT_SC->SCp.buffer);
>  			CURRENT_SC->SCp.ptr           = SG_ADDRESS(CURRENT_SC->SCp.buffer);
>  			CURRENT_SC->SCp.this_residual = CURRENT_SC->SCp.buffer->length;
>  		}
> @@ -2158,22 +2154,24 @@ static void datao_run(struct Scsi_Host *shpnt)
>  static void datao_end(struct Scsi_Host *shpnt)
>  {
>  	if(TESTLO(DMASTAT, DFIFOEMP)) {
> -		int data_count = (DATA_LEN - scsi_get_resid(CURRENT_SC)) -
> -			GETSTCNT();
> +		int done = GETSTCNT();
> +		int data_count = (DATA_LEN - scsi_get_resid(CURRENT_SC)) - done;
> +		struct scatterlist *sg = scsi_sglist(CURRENT_SC);
>  
>  		CMD_INC_RESID(CURRENT_SC, data_count);
>  
> -		data_count -= CURRENT_SC->SCp.ptr -
> -			SG_ADDRESS(CURRENT_SC->SCp.buffer);
> -		while(data_count>0) {
> -			CURRENT_SC->SCp.buffer--;
> -			CURRENT_SC->SCp.buffers_residual++;
> -			data_count -= CURRENT_SC->SCp.buffer->length;
> +		/* Locate the first SG entry not yet sent */
> +		while (done > 0 && !sg_is_last(sg)) {
> +			if (done < sg->length)
> +				break;
> +			done -= sg->length;
> +			sg = sg_next(sg);
>  		}
> -		CURRENT_SC->SCp.ptr = SG_ADDRESS(CURRENT_SC->SCp.buffer) -
> -			data_count;
> -		CURRENT_SC->SCp.this_residual = CURRENT_SC->SCp.buffer->length +
> -			data_count;
> +
> +		CURRENT_SC->SCp.buffer = sg;
> +		CURRENT_SC->SCp.ptr = SG_ADDRESS(CURRENT_SC->SCp.buffer) + done;
> +		CURRENT_SC->SCp.this_residual = CURRENT_SC->SCp.buffer->length -
> +			done;
>  	}
>  
>  	SETPORT(SXFRCTL0, CH1|CLRCH1|CLRSTCNT);
> @@ -2501,7 +2499,7 @@ static void get_command(struct seq_file *m, struct scsi_cmnd * ptr)
>  
>  	seq_printf(m, "); resid=%d; residual=%d; buffers=%d; phase |",
>  		scsi_get_resid(ptr), ptr->SCp.this_residual,
> -		ptr->SCp.buffers_residual);
> +		sg_nents(ptr->SCp.buffer) - 1);
>  
>  	if (ptr->SCp.phase & not_issued)
>  		seq_puts(m, "not issued|");
> 

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 01/16] scsi: vmw_pscsi: use sg helper to operate scatterlist
  2019-06-17  3:03 ` [PATCH V4 01/16] scsi: vmw_pscsi: " Ming Lei
@ 2019-06-17  8:22   ` Christoph Hellwig
  0 siblings, 0 replies; 40+ messages in thread
From: Christoph Hellwig @ 2019-06-17  8:22 UTC (permalink / raw)
  To: Ming Lei
  Cc: linux-scsi, Martin K . Petersen, James Bottomley,
	Bart Van Assche, Hannes Reinecke, Christoph Hellwig, Jim Gill,
	Cathy Avery, Ewan D . Milne, Brian King, James Smart,
	Juergen E . Fischer, Michael Schmitz, Finn Thain,
	Greg Kroah-Hartman, devel, linux-usb, Dan Carpenter,
	Benjamin Block

On Mon, Jun 17, 2019 at 11:03:34AM +0800, Ming Lei wrote:
> Use the scatterlist iterators and remove direct indexing of the scatterlist
> array.
> 
> This way allows us to pre-allocate one small scatterlist, which can be chained
> with one runtime allocated scatterlist if the pre-allocated one isn't enough
> for the whole request.
> 
> Reviewed-by: Ewan D. Milne <emilne@redhat.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 02/16] scsi: advansys: use sg helper to operate scatterlist
  2019-06-17  3:03 ` [PATCH V4 02/16] scsi: advansys: " Ming Lei
@ 2019-06-17  8:22   ` Christoph Hellwig
  0 siblings, 0 replies; 40+ messages in thread
From: Christoph Hellwig @ 2019-06-17  8:22 UTC (permalink / raw)
  To: Ming Lei
  Cc: linux-scsi, Martin K . Petersen, James Bottomley,
	Bart Van Assche, Hannes Reinecke, Christoph Hellwig, Jim Gill,
	Cathy Avery, Ewan D . Milne, Brian King, James Smart,
	Juergen E . Fischer, Michael Schmitz, Finn Thain,
	Greg Kroah-Hartman, devel, linux-usb, Dan Carpenter,
	Benjamin Block

On Mon, Jun 17, 2019 at 11:03:35AM +0800, Ming Lei wrote:
> Use the scatterlist iterators and remove direct indexing of the
> scatterlist array.
> 
> This way allows us to pre-allocate one small scatterlist, which can be
> chained with one runtime allocated scatterlist if the pre-allocated one
> isn't enough for the whole request.
> 
> Reviewed-by: Ewan D. Milne <emilne@redhat.com>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 03/16] scsi: lpfc: use sg helper to operate scatterlist
  2019-06-17  3:03 ` [PATCH V4 03/16] scsi: lpfc: " Ming Lei
@ 2019-06-17  8:22   ` Christoph Hellwig
  0 siblings, 0 replies; 40+ messages in thread
From: Christoph Hellwig @ 2019-06-17  8:22 UTC (permalink / raw)
  To: Ming Lei
  Cc: linux-scsi, Martin K . Petersen, James Bottomley,
	Bart Van Assche, Hannes Reinecke, Christoph Hellwig, Jim Gill,
	Cathy Avery, Ewan D . Milne, Brian King, James Smart,
	Juergen E . Fischer, Michael Schmitz, Finn Thain,
	Greg Kroah-Hartman, devel, linux-usb, Dan Carpenter,
	Benjamin Block

On Mon, Jun 17, 2019 at 11:03:36AM +0800, Ming Lei wrote:
> Use the scatterlist iterators and remove direct indexing of the
> scatterlist array.
> 
> This way allows us to pre-allocate one small scatterlist, which can be
> chained with one runtime allocated scatterlist if the pre-allocated one isn't
> enough for the whole request.
> 
> Reviewed by: Ewan D. Milne <emilne@redhat.com>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 04/16] scsi: mvumi: use sg helper to operate scatterlist
  2019-06-17  3:03 ` [PATCH V4 04/16] scsi: mvumi: " Ming Lei
@ 2019-06-17  8:22   ` Christoph Hellwig
  0 siblings, 0 replies; 40+ messages in thread
From: Christoph Hellwig @ 2019-06-17  8:22 UTC (permalink / raw)
  To: Ming Lei
  Cc: linux-scsi, Martin K . Petersen, James Bottomley,
	Bart Van Assche, Hannes Reinecke, Christoph Hellwig, Jim Gill,
	Cathy Avery, Ewan D . Milne, Brian King, James Smart,
	Juergen E . Fischer, Michael Schmitz, Finn Thain,
	Greg Kroah-Hartman, devel, linux-usb, Dan Carpenter,
	Benjamin Block

On Mon, Jun 17, 2019 at 11:03:37AM +0800, Ming Lei wrote:
> Use the scatterlist iterators and remove direct indexing of the
> scatterlist array.
> 
> This way allows us to pre-allocate one small scatterlist, which can be
> chained with one runtime allocated scatterlist if the pre-allocated one
> isn't enough for the whole request.
> 
> Reviewed-by: Ewan D. Milne <emilne@redhat.com>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 05/16] scsi: ipr: use sg helper to operate scatterlist
  2019-06-17  3:03 ` [PATCH V4 05/16] scsi: ipr: " Ming Lei
@ 2019-06-17  8:24   ` Christoph Hellwig
  2019-06-17  8:50     ` Ming Lei
  0 siblings, 1 reply; 40+ messages in thread
From: Christoph Hellwig @ 2019-06-17  8:24 UTC (permalink / raw)
  To: Ming Lei
  Cc: linux-scsi, Martin K . Petersen, James Bottomley,
	Bart Van Assche, Hannes Reinecke, Christoph Hellwig, Jim Gill,
	Cathy Avery, Ewan D . Milne, Brian King, James Smart,
	Juergen E . Fischer, Michael Schmitz, Finn Thain,
	Greg Kroah-Hartman, devel, linux-usb, Dan Carpenter,
	Benjamin Block

> -	for (i = 0; i < (len / bsize_elem); i++, buffer += bsize_elem) {
> -		struct page *page = sg_page(&scatterlist[i]);
> +	for (i = 0; i < (len / bsize_elem); i++, sg = sg_next(sg), buffer += bsize_elem) {

Please split the overly long line.

> +		struct page *page = sg_page(sg);
>  
>  		kaddr = kmap(page);
>  		memcpy(kaddr, buffer, bsize_elem);
>  		kunmap(page);

Not new in this patch, but this is buggy as scatterlists could have
offsets.  This should probably use the scatterlist.c copy helper
eventually.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 06/16] scsi: pmcraid: use sg helper to operate scatterlist
  2019-06-17  3:03 ` [PATCH V4 06/16] scsi: pmcraid: " Ming Lei
@ 2019-06-17  8:24   ` Christoph Hellwig
  0 siblings, 0 replies; 40+ messages in thread
From: Christoph Hellwig @ 2019-06-17  8:24 UTC (permalink / raw)
  To: Ming Lei
  Cc: linux-scsi, Martin K . Petersen, James Bottomley,
	Bart Van Assche, Hannes Reinecke, Christoph Hellwig, Jim Gill,
	Cathy Avery, Ewan D . Milne, Brian King, James Smart,
	Juergen E . Fischer, Michael Schmitz, Finn Thain,
	Greg Kroah-Hartman, devel, linux-usb, Dan Carpenter,
	Benjamin Block

Same kmap issue here that will need addressing.  Otherwise looks
good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 07/16] usb: image: microtek: use sg helper to operate scatterlist
  2019-06-17  3:03 ` [PATCH V4 07/16] usb: image: microtek: " Ming Lei
@ 2019-06-17  8:25   ` Christoph Hellwig
  0 siblings, 0 replies; 40+ messages in thread
From: Christoph Hellwig @ 2019-06-17  8:25 UTC (permalink / raw)
  To: Ming Lei
  Cc: linux-scsi, Martin K . Petersen, James Bottomley,
	Bart Van Assche, Hannes Reinecke, Christoph Hellwig, Jim Gill,
	Cathy Avery, Ewan D . Milne, Brian King, James Smart,
	Juergen E . Fischer, Michael Schmitz, Finn Thain,
	Greg Kroah-Hartman, devel, linux-usb, Dan Carpenter,
	Benjamin Block, Oliver Neukum

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 08/16] staging: unisys: visorhba: use sg helper to operate scatterlist
  2019-06-17  3:03 ` [PATCH V4 08/16] staging: unisys: visorhba: " Ming Lei
@ 2019-06-17  8:25   ` Christoph Hellwig
  0 siblings, 0 replies; 40+ messages in thread
From: Christoph Hellwig @ 2019-06-17  8:25 UTC (permalink / raw)
  To: Ming Lei
  Cc: linux-scsi, Martin K . Petersen, James Bottomley,
	Bart Van Assche, Hannes Reinecke, Christoph Hellwig, Jim Gill,
	Cathy Avery, Ewan D . Milne, Brian King, James Smart,
	Juergen E . Fischer, Michael Schmitz, Finn Thain,
	Greg Kroah-Hartman, devel, linux-usb, Dan Carpenter,
	Benjamin Block

The patch itsel looks good, but another case of buggy kmap it seems:

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 09/16] staging: rtsx: use sg helper to operate scatterlist
  2019-06-17  3:03 ` [PATCH V4 09/16] staging: rtsx: " Ming Lei
@ 2019-06-17  8:27   ` Christoph Hellwig
  2019-06-17  9:15     ` Ming Lei
  0 siblings, 1 reply; 40+ messages in thread
From: Christoph Hellwig @ 2019-06-17  8:27 UTC (permalink / raw)
  To: Ming Lei
  Cc: linux-scsi, Martin K . Petersen, James Bottomley,
	Bart Van Assche, Hannes Reinecke, Christoph Hellwig, Jim Gill,
	Cathy Avery, Ewan D . Milne, Brian King, James Smart,
	Juergen E . Fischer, Michael Schmitz, Finn Thain,
	Greg Kroah-Hartman, devel, linux-usb, Dan Carpenter,
	Benjamin Block, Kim Bradley

On Mon, Jun 17, 2019 at 11:03:42AM +0800, Ming Lei wrote:
> Use the scatterlist iterators and remove direct indexing of the
> scatterlist array.
> 
> This way allows us to pre-allocate one small scatterlist, which can be
> chained with one runtime allocated scatterlist if the pre-allocated one
> isn't enough for the whole request.

The scatterlist handling here looks completely bogus, it really
needs to stop using the index and switch to proper sg_next-based
iteration.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 10/16] s390: zfcp_fc: use sg helper to operate scatterlist
  2019-06-17  3:03 ` [PATCH V4 10/16] s390: zfcp_fc: " Ming Lei
@ 2019-06-17  8:27   ` Christoph Hellwig
  0 siblings, 0 replies; 40+ messages in thread
From: Christoph Hellwig @ 2019-06-17  8:27 UTC (permalink / raw)
  To: Ming Lei
  Cc: linux-scsi, Martin K . Petersen, James Bottomley,
	Bart Van Assche, Hannes Reinecke, Christoph Hellwig, Jim Gill,
	Cathy Avery, Ewan D . Milne, Brian King, James Smart,
	Juergen E . Fischer, Michael Schmitz, Finn Thain,
	Greg Kroah-Hartman, devel, linux-usb, Dan Carpenter,
	Benjamin Block, Steffen Maier, Martin Schwidefsky,
	Heiko Carstens, linux-s390

On Mon, Jun 17, 2019 at 11:03:43AM +0800, Ming Lei wrote:
> Use the scatterlist iterators and remove direct indexing of the
> scatterlist array.
> 
> This way allows us to pre-allocate one small scatterlist, which can be
> chained with one runtime allocated scatterlist if the pre-allocated one
> isn't enough for the whole request.

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 11/16] scsi: aha152x: use sg helper to operate scatterlist
  2019-06-17  3:03 ` [PATCH V4 11/16] scsi: aha152x: " Ming Lei
  2019-06-17  3:35   ` Finn Thain
@ 2019-06-17  8:28   ` Christoph Hellwig
  1 sibling, 0 replies; 40+ messages in thread
From: Christoph Hellwig @ 2019-06-17  8:28 UTC (permalink / raw)
  To: Ming Lei
  Cc: linux-scsi, Martin K . Petersen, James Bottomley,
	Bart Van Assche, Hannes Reinecke, Christoph Hellwig, Jim Gill,
	Cathy Avery, Ewan D . Milne, Brian King, James Smart,
	Juergen E . Fischer, Michael Schmitz, Finn Thain,
	Greg Kroah-Hartman, devel, linux-usb, Dan Carpenter,
	Benjamin Block

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 12/16] scsi: imm: use sg helper to operate scatterlist
  2019-06-17  3:03 ` [PATCH V4 12/16] scsi: imm: " Ming Lei
@ 2019-06-17  8:28   ` Christoph Hellwig
  0 siblings, 0 replies; 40+ messages in thread
From: Christoph Hellwig @ 2019-06-17  8:28 UTC (permalink / raw)
  To: Ming Lei
  Cc: linux-scsi, Martin K . Petersen, James Bottomley,
	Bart Van Assche, Hannes Reinecke, Christoph Hellwig, Jim Gill,
	Cathy Avery, Ewan D . Milne, Brian King, James Smart,
	Juergen E . Fischer, Michael Schmitz, Finn Thain,
	Greg Kroah-Hartman, devel, linux-usb, Dan Carpenter,
	Benjamin Block

On Mon, Jun 17, 2019 at 11:03:45AM +0800, Ming Lei wrote:
> Use the scatterlist iterators and remove direct indexing of the
> scatterlist array.
> 
> This way allows us to pre-allocate one small scatterlist, which can be
> chained with one runtime allocated scatterlist if the pre-allocated one
> isn't enough for the whole request.
> 
> Signed-off-by: Ming Lei <ming.lei@redhat.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 13/16] scsi: pcmcia: nsp_cs: use sg helper to operate scatterlist
  2019-06-17  3:03 ` [PATCH V4 13/16] scsi: pcmcia: nsp_cs: " Ming Lei
@ 2019-06-17  8:29   ` Christoph Hellwig
  0 siblings, 0 replies; 40+ messages in thread
From: Christoph Hellwig @ 2019-06-17  8:29 UTC (permalink / raw)
  To: Ming Lei
  Cc: linux-scsi, Martin K . Petersen, James Bottomley,
	Bart Van Assche, Hannes Reinecke, Christoph Hellwig, Jim Gill,
	Cathy Avery, Ewan D . Milne, Brian King, James Smart,
	Juergen E . Fischer, Michael Schmitz, Finn Thain,
	Greg Kroah-Hartman, devel, linux-usb, Dan Carpenter,
	Benjamin Block

On Mon, Jun 17, 2019 at 11:03:46AM +0800, Ming Lei wrote:
> Use the scatterlist iterators and remove direct indexing of the
> scatterlist array.
> 
> This way allows us to pre-allocate one small scatterlist, which can be
> chained with one runtime allocated scatterlist if the pre-allocated one
> isn't enough for the whole request.
> 
> Signed-off-by: Ming Lei <ming.lei@redhat.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 14/16] scsi: ppa: use sg helper to operate scatterlist
  2019-06-17  3:03 ` [PATCH V4 14/16] scsi: ppa: " Ming Lei
@ 2019-06-17  8:29   ` Christoph Hellwig
  0 siblings, 0 replies; 40+ messages in thread
From: Christoph Hellwig @ 2019-06-17  8:29 UTC (permalink / raw)
  To: Ming Lei
  Cc: linux-scsi, Martin K . Petersen, James Bottomley,
	Bart Van Assche, Hannes Reinecke, Christoph Hellwig, Jim Gill,
	Cathy Avery, Ewan D . Milne, Brian King, James Smart,
	Juergen E . Fischer, Michael Schmitz, Finn Thain,
	Greg Kroah-Hartman, devel, linux-usb, Dan Carpenter,
	Benjamin Block

On Mon, Jun 17, 2019 at 11:03:47AM +0800, Ming Lei wrote:
> Use the scatterlist iterators and remove direct indexing of the
> scatterlist array.
> 
> This way allows us to pre-allocate one small scatterlist, which can be
> chained with one runtime allocated scatterlist if the pre-allocated one
> isn't enough for the whole request.
> 
> Signed-off-by: Ming Lei <ming.lei@redhat.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 15/16] scsi: wd33c93: use sg helper to operate scatterlist
  2019-06-17  3:03 ` [PATCH V4 15/16] scsi: wd33c93: " Ming Lei
@ 2019-06-17  8:29   ` Christoph Hellwig
  0 siblings, 0 replies; 40+ messages in thread
From: Christoph Hellwig @ 2019-06-17  8:29 UTC (permalink / raw)
  To: Ming Lei
  Cc: linux-scsi, Martin K . Petersen, James Bottomley,
	Bart Van Assche, Hannes Reinecke, Christoph Hellwig, Jim Gill,
	Cathy Avery, Ewan D . Milne, Brian King, James Smart,
	Juergen E . Fischer, Michael Schmitz, Finn Thain,
	Greg Kroah-Hartman, devel, linux-usb, Dan Carpenter,
	Benjamin Block

On Mon, Jun 17, 2019 at 11:03:48AM +0800, Ming Lei wrote:
> Use the scatterlist iterators and remove direct indexing of the
> scatterlist array.
> 
> This way allows us to pre-allocate one small scatterlist, which can be
> chained with one runtime allocated scatterlist if the pre-allocated one
> isn't enough for the whole request.
> 
> Signed-off-by: Ming Lei <ming.lei@redhat.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 16/16] NCR5380: Support chained sg lists
  2019-06-17  3:03 ` [PATCH V4 16/16] NCR5380: Support chained sg lists Ming Lei
@ 2019-06-17  8:30   ` Christoph Hellwig
  0 siblings, 0 replies; 40+ messages in thread
From: Christoph Hellwig @ 2019-06-17  8:30 UTC (permalink / raw)
  To: Ming Lei
  Cc: linux-scsi, Martin K . Petersen, James Bottomley,
	Bart Van Assche, Hannes Reinecke, Christoph Hellwig, Jim Gill,
	Cathy Avery, Ewan D . Milne, Brian King, James Smart,
	Juergen E . Fischer, Michael Schmitz, Finn Thain,
	Greg Kroah-Hartman, devel, linux-usb, Dan Carpenter,
	Benjamin Block

On Mon, Jun 17, 2019 at 11:03:49AM +0800, Ming Lei wrote:
> From: Finn Thain <fthain@telegraphics.com.au>
> 
> My understanding is that support for chained scatterlists is to
> become mandatory for LLDs.
> 
> Use the scatterlist iterators and remove direct indexing of the
> scatterlist array.
> 
> This way allows us to pre-allocate one small scatterlist, which can be
> chained with one runtime allocated scatterlist if the pre-allocated one
> isn't enough for the whole request.

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 05/16] scsi: ipr: use sg helper to operate scatterlist
  2019-06-17  8:24   ` Christoph Hellwig
@ 2019-06-17  8:50     ` Ming Lei
  0 siblings, 0 replies; 40+ messages in thread
From: Ming Lei @ 2019-06-17  8:50 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-scsi, Martin K . Petersen, James Bottomley,
	Bart Van Assche, Hannes Reinecke, Jim Gill, Cathy Avery,
	Ewan D . Milne, Brian King, James Smart, Juergen E . Fischer,
	Michael Schmitz, Finn Thain, Greg Kroah-Hartman, devel,
	linux-usb, Dan Carpenter, Benjamin Block

On Mon, Jun 17, 2019 at 10:24:23AM +0200, Christoph Hellwig wrote:
> > -	for (i = 0; i < (len / bsize_elem); i++, buffer += bsize_elem) {
> > -		struct page *page = sg_page(&scatterlist[i]);
> > +	for (i = 0; i < (len / bsize_elem); i++, sg = sg_next(sg), buffer += bsize_elem) {
> 
> Please split the overly long line.

Fine.

> 
> > +		struct page *page = sg_page(sg);
> >  
> >  		kaddr = kmap(page);
> >  		memcpy(kaddr, buffer, bsize_elem);
> >  		kunmap(page);
> 
> Not new in this patch, but this is buggy as scatterlists could have
> offsets.  This should probably use the scatterlist.c copy helper
> eventually.

This sglist is allocated by driver, see ipr_copy_ucode_buffer(), so
offset for any element is zero.

Thanks,
Ming

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 09/16] staging: rtsx: use sg helper to operate scatterlist
  2019-06-17  8:27   ` Christoph Hellwig
@ 2019-06-17  9:15     ` Ming Lei
  2019-06-17  9:24       ` Christoph Hellwig
  0 siblings, 1 reply; 40+ messages in thread
From: Ming Lei @ 2019-06-17  9:15 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-scsi, Martin K . Petersen, James Bottomley,
	Bart Van Assche, Hannes Reinecke, Jim Gill, Cathy Avery,
	Ewan D . Milne, Brian King, James Smart, Juergen E . Fischer,
	Michael Schmitz, Finn Thain, Greg Kroah-Hartman, devel,
	linux-usb, Dan Carpenter, Benjamin Block, Kim Bradley

On Mon, Jun 17, 2019 at 10:27:06AM +0200, Christoph Hellwig wrote:
> On Mon, Jun 17, 2019 at 11:03:42AM +0800, Ming Lei wrote:
> > Use the scatterlist iterators and remove direct indexing of the
> > scatterlist array.
> > 
> > This way allows us to pre-allocate one small scatterlist, which can be
> > chained with one runtime allocated scatterlist if the pre-allocated one
> > isn't enough for the whole request.
> 
> The scatterlist handling here looks completely bogus, it really
> needs to stop using the index and switch to proper sg_next-based
> iteration.

Yeah, I agree, looks we may convert in the following way by
storing current 'sg', and the 'offset' already stores the actual offset
on the current 'sg'.

diff --git a/drivers/staging/rts5208/rtsx_transport.c b/drivers/staging/rts5208/rtsx_transport.c
index 8277d7895608..861b4cc4562e 100644
--- a/drivers/staging/rts5208/rtsx_transport.c
+++ b/drivers/staging/rts5208/rtsx_transport.c
@@ -32,7 +32,7 @@
 unsigned int rtsx_stor_access_xfer_buf(unsigned char *buffer,
 				       unsigned int buflen,
 				       struct scsi_cmnd *srb,
-				       unsigned int *index,
+				       struct scatterlist **cur_sg,
 				       unsigned int *offset,
 				       enum xfer_buf_dir dir)
 {
@@ -60,18 +60,18 @@ unsigned int rtsx_stor_access_xfer_buf(unsigned char *buffer,
 	 * each page has to be kmap()'ed separately.
 	 */
 	} else {
-		struct scatterlist *sg =
-				(struct scatterlist *)scsi_sglist(srb)
-				+ *index;
+		struct scatterlist *sg = *cur_sg ?:
+				(struct scatterlist *)scsi_sglist(srb);
+
 
 		/*
 		 * This loop handles a single s-g list entry, which may
 		 * include multiple pages.  Find the initial page structure
 		 * and the starting offset within the page, and update
-		 * the *offset and *index values for the next loop.
+		 * the *offset values for the next loop.
 		 */
 		cnt = 0;
-		while (cnt < buflen && *index < scsi_sg_count(srb)) {
+		while (cnt < buflen && sg) {
 			struct page *page = sg_page(sg) +
 					((sg->offset + *offset) >> PAGE_SHIFT);
 			unsigned int poff = (sg->offset + *offset) &
@@ -85,8 +85,7 @@ unsigned int rtsx_stor_access_xfer_buf(unsigned char *buffer,
 			} else {
 				/* Transfer continues to next s-g entry */
 				*offset = 0;
-				++*index;
-				++sg;
+				sg = sg_next(sg);
 			}
 
 			while (sglen > 0) {
@@ -120,9 +119,10 @@ unsigned int rtsx_stor_access_xfer_buf(unsigned char *buffer,
 void rtsx_stor_set_xfer_buf(unsigned char *buffer,
 			    unsigned int buflen, struct scsi_cmnd *srb)
 {
-	unsigned int index = 0, offset = 0;
+	unsigned int offset = 0;
+	struct scatterlist *sg = NULL;
 
-	rtsx_stor_access_xfer_buf(buffer, buflen, srb, &index, &offset,
+	rtsx_stor_access_xfer_buf(buffer, buflen, srb, &sg, &offset,
 				  TO_XFER_BUF);
 	if (buflen < scsi_bufflen(srb))
 		scsi_set_resid(srb, scsi_bufflen(srb) - buflen);
@@ -131,9 +131,10 @@ void rtsx_stor_set_xfer_buf(unsigned char *buffer,
 void rtsx_stor_get_xfer_buf(unsigned char *buffer,
 			    unsigned int buflen, struct scsi_cmnd *srb)
 {
-	unsigned int index = 0, offset = 0;
+	unsigned int offset = 0;
+	struct scatterlist *sg = NULL;
 
-	rtsx_stor_access_xfer_buf(buffer, buflen, srb, &index, &offset,
+	rtsx_stor_access_xfer_buf(buffer, buflen, srb, &sg, &offset,
 				  FROM_XFER_BUF);
 	if (buflen < scsi_bufflen(srb))
 		scsi_set_resid(srb, scsi_bufflen(srb) - buflen);
diff --git a/drivers/staging/rts5208/spi.c b/drivers/staging/rts5208/spi.c
index f1e9e80044ed..6ad4278de531 100644
--- a/drivers/staging/rts5208/spi.c
+++ b/drivers/staging/rts5208/spi.c
@@ -554,7 +554,8 @@ int spi_read_flash_id(struct scsi_cmnd *srb, struct rtsx_chip *chip)
 int spi_read_flash(struct scsi_cmnd *srb, struct rtsx_chip *chip)
 {
 	int retval;
-	unsigned int index = 0, offset = 0;
+	unsigned int offset = 0;
+	struct scatterlist *sg = NULL;
 	u8 ins, slow_read;
 	u32 addr;
 	u16 len;
@@ -631,7 +632,7 @@ int spi_read_flash(struct scsi_cmnd *srb, struct rtsx_chip *chip)
 			return STATUS_FAIL;
 		}
 
-		rtsx_stor_access_xfer_buf(buf, pagelen, srb, &index, &offset,
+		rtsx_stor_access_xfer_buf(buf, pagelen, srb, &sg, &offset,
 					  TO_XFER_BUF);
 
 		addr += pagelen;
@@ -651,7 +652,8 @@ int spi_write_flash(struct scsi_cmnd *srb, struct rtsx_chip *chip)
 	u32 addr;
 	u16 len;
 	u8 *buf;
-	unsigned int index = 0, offset = 0;
+	unsigned int offset = 0;
+	struct scatterlist *sg = NULL;
 
 	spi_set_err_code(chip, SPI_NO_ERR);
 
@@ -679,7 +681,7 @@ int spi_write_flash(struct scsi_cmnd *srb, struct rtsx_chip *chip)
 				return STATUS_FAIL;
 			}
 
-			rtsx_stor_access_xfer_buf(buf, 1, srb, &index, &offset,
+			rtsx_stor_access_xfer_buf(buf, 1, srb, &sg, &offset,
 						  FROM_XFER_BUF);
 
 			rtsx_init_cmd(chip);
@@ -722,7 +724,7 @@ int spi_write_flash(struct scsi_cmnd *srb, struct rtsx_chip *chip)
 			return STATUS_ERROR;
 
 		while (len) {
-			rtsx_stor_access_xfer_buf(buf, 1, srb, &index, &offset,
+			rtsx_stor_access_xfer_buf(buf, 1, srb, &sg, &offset,
 						  FROM_XFER_BUF);
 
 			rtsx_init_cmd(chip);
@@ -788,7 +790,7 @@ int spi_write_flash(struct scsi_cmnd *srb, struct rtsx_chip *chip)
 
 			rtsx_send_cmd_no_wait(chip);
 
-			rtsx_stor_access_xfer_buf(buf, pagelen, srb, &index,
+			rtsx_stor_access_xfer_buf(buf, pagelen, srb, &sg,
 						  &offset, FROM_XFER_BUF);
 
 			retval = rtsx_transfer_data(chip, 0, buf, pagelen, 0,

Thanks,
Ming

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 09/16] staging: rtsx: use sg helper to operate scatterlist
  2019-06-17  9:15     ` Ming Lei
@ 2019-06-17  9:24       ` Christoph Hellwig
  0 siblings, 0 replies; 40+ messages in thread
From: Christoph Hellwig @ 2019-06-17  9:24 UTC (permalink / raw)
  To: Ming Lei
  Cc: Christoph Hellwig, linux-scsi, Martin K . Petersen,
	James Bottomley, Bart Van Assche, Hannes Reinecke, Jim Gill,
	Cathy Avery, Ewan D . Milne, Brian King, James Smart,
	Juergen E . Fischer, Michael Schmitz, Finn Thain,
	Greg Kroah-Hartman, devel, linux-usb, Dan Carpenter,
	Benjamin Block, Kim Bradley

> +		struct scatterlist *sg = *cur_sg ?:
> +				(struct scatterlist *)scsi_sglist(srb);
> +

No need for the cast here.  And I have to say I hate that GNU C
non-standard shortshut in ? :.

Why not simply:

		struct scatterlist *sg = *cur_sg;

		if (!sg)
			sg = scsi_sglist(srb);

Which is a little more verbose, but much more readabe.

The rest of the patch looks fine to me.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 11/16] scsi: aha152x: use sg helper to operate scatterlist
  2019-06-17  3:35   ` Finn Thain
@ 2019-06-17 23:35     ` Finn Thain
  2019-06-18  0:15       ` Ming Lei
  0 siblings, 1 reply; 40+ messages in thread
From: Finn Thain @ 2019-06-17 23:35 UTC (permalink / raw)
  To: Juergen E . Fischer, Ming Lei
  Cc: linux-scsi, Martin K . Petersen, James Bottomley,
	Bart Van Assche, Hannes Reinecke, Christoph Hellwig, Jim Gill,
	Cathy Avery, Ewan D . Milne, Brian King, James Smart,
	Michael Schmitz, Greg Kroah-Hartman, devel, linux-usb,
	Dan Carpenter, Benjamin Block

On Mon, 17 Jun 2019, Finn Thain wrote:

> On Mon, 17 Jun 2019, Ming Lei wrote:
> 
> > Use the scatterlist iterators and remove direct indexing of the
> > scatterlist array.
> > 
> > This way allows us to pre-allocate one small scatterlist, which can be
> > chained with one runtime allocated scatterlist if the pre-allocated one
> > isn't enough for the whole request.
> > 
> > Finn added the change to replace SCp.buffers_residual with sg_is_last()
> > for fixing updating it, and the similar change has been applied on
> > NCR5380.c
> > 
> > Cc: Finn Thain <fthain@telegraphics.com.au>
> > Signed-off-by: Ming Lei <ming.lei@redhat.com>
> 
> Reviewed-by: Finn Thain <fthain@telegraphics.com.au>
> 

I have to retract that. I think this patch is still wrong.

GETSTCNT() appears to be the number of bytes sent since datao_init() and 
not the number of bytes sent since the start of the command. (Note the 
CLRSTCNT in datao_init() which appears to clear the transfer counter.) I 
don't see how the existing datao_end() could work otherwise. (Juergen?)

So here's another attempt. Ming, I'd be happy to take the blame/credit (in 
the form of a From tag etc.) in case you don't want to spend more time on 
this.

diff --git a/drivers/scsi/aha152x.c b/drivers/scsi/aha152x.c
index 97872838b983..f07f3fa9b58d 100644
--- a/drivers/scsi/aha152x.c
+++ b/drivers/scsi/aha152x.c
@@ -948,7 +948,6 @@ static int aha152x_internal_queue(struct scsi_cmnd *SCpnt,
 	   SCp.ptr              : buffer pointer
 	   SCp.this_residual    : buffer length
 	   SCp.buffer           : next buffer
-	   SCp.buffers_residual : left buffers in list
 	   SCp.phase            : current state of the command */
 
 	if ((phase & resetting) || !scsi_sglist(SCpnt)) {
@@ -956,13 +955,11 @@ static int aha152x_internal_queue(struct scsi_cmnd *SCpnt,
 		SCpnt->SCp.this_residual = 0;
 		scsi_set_resid(SCpnt, 0);
 		SCpnt->SCp.buffer           = NULL;
-		SCpnt->SCp.buffers_residual = 0;
 	} else {
 		scsi_set_resid(SCpnt, scsi_bufflen(SCpnt));
 		SCpnt->SCp.buffer           = scsi_sglist(SCpnt);
 		SCpnt->SCp.ptr              = SG_ADDRESS(SCpnt->SCp.buffer);
 		SCpnt->SCp.this_residual    = SCpnt->SCp.buffer->length;
-		SCpnt->SCp.buffers_residual = scsi_sg_count(SCpnt) - 1;
 	}
 
 	DO_LOCK(flags);
@@ -2030,10 +2027,9 @@ static void datai_run(struct Scsi_Host *shpnt)
 				}
 
 				if (CURRENT_SC->SCp.this_residual == 0 &&
-				    CURRENT_SC->SCp.buffers_residual > 0) {
+				    !sg_is_last(CURRENT_SC->SCp.buffer)) {
 					/* advance to next buffer */
-					CURRENT_SC->SCp.buffers_residual--;
-					CURRENT_SC->SCp.buffer++;
+					CURRENT_SC->SCp.buffer = sg_next(CURRENT_SC->SCp.buffer);
 					CURRENT_SC->SCp.ptr           = SG_ADDRESS(CURRENT_SC->SCp.buffer);
 					CURRENT_SC->SCp.this_residual = CURRENT_SC->SCp.buffer->length;
 				}
@@ -2136,10 +2132,10 @@ static void datao_run(struct Scsi_Host *shpnt)
 			CMD_INC_RESID(CURRENT_SC, -2 * data_count);
 		}
 
-		if(CURRENT_SC->SCp.this_residual==0 && CURRENT_SC->SCp.buffers_residual>0) {
+		if (CURRENT_SC->SCp.this_residual == 0 &&
+		    !sg_is_last(CURRENT_SC->SCp.buffer)) {
 			/* advance to next buffer */
-			CURRENT_SC->SCp.buffers_residual--;
-			CURRENT_SC->SCp.buffer++;
+			CURRENT_SC->SCp.buffer = sg_next(CURRENT_SC->SCp.buffer);
 			CURRENT_SC->SCp.ptr           = SG_ADDRESS(CURRENT_SC->SCp.buffer);
 			CURRENT_SC->SCp.this_residual = CURRENT_SC->SCp.buffer->length;
 		}
@@ -2158,22 +2154,26 @@ static void datao_run(struct Scsi_Host *shpnt)
 static void datao_end(struct Scsi_Host *shpnt)
 {
 	if(TESTLO(DMASTAT, DFIFOEMP)) {
-		int data_count = (DATA_LEN - scsi_get_resid(CURRENT_SC)) -
-			GETSTCNT();
+		u32 datao_cnt = GETSTCNT();
+		int datao_out = DATA_LEN - scsi_get_resid(CURRENT_SC);
+		int done;
+		struct scatterlist *sg = scsi_sglist(CURRENT_SC);
 
-		CMD_INC_RESID(CURRENT_SC, data_count);
+		CMD_INC_RESID(CURRENT_SC, datao_out - datao_cnt);
 
-		data_count -= CURRENT_SC->SCp.ptr -
-			SG_ADDRESS(CURRENT_SC->SCp.buffer);
-		while(data_count>0) {
-			CURRENT_SC->SCp.buffer--;
-			CURRENT_SC->SCp.buffers_residual++;
-			data_count -= CURRENT_SC->SCp.buffer->length;
+		done = scsi_bufflen(CURRENT_SC) - scsi_get_resid(CURRENT_SC);
+		/* Locate the first SG entry not yet sent */
+		while (done > 0 && !sg_is_last(sg)) {
+			if (done < sg->length)
+				break;
+			done -= sg->length;
+			sg = sg_next(sg);
 		}
-		CURRENT_SC->SCp.ptr = SG_ADDRESS(CURRENT_SC->SCp.buffer) -
-			data_count;
-		CURRENT_SC->SCp.this_residual = CURRENT_SC->SCp.buffer->length +
-			data_count;
+
+		CURRENT_SC->SCp.buffer = sg;
+		CURRENT_SC->SCp.ptr = SG_ADDRESS(CURRENT_SC->SCp.buffer) + done;
+		CURRENT_SC->SCp.this_residual = CURRENT_SC->SCp.buffer->length -
+			done;
 	}
 
 	SETPORT(SXFRCTL0, CH1|CLRCH1|CLRSTCNT);
@@ -2501,7 +2501,7 @@ static void get_command(struct seq_file *m, struct scsi_cmnd * ptr)
 
 	seq_printf(m, "); resid=%d; residual=%d; buffers=%d; phase |",
 		scsi_get_resid(ptr), ptr->SCp.this_residual,
-		ptr->SCp.buffers_residual);
+		sg_nents(ptr->SCp.buffer) - 1);
 
 	if (ptr->SCp.phase & not_issued)
 		seq_puts(m, "not issued|");

-- 

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 00/16] use sg helper to operate scatterlist
  2019-06-17  3:03 [PATCH V4 00/16] use sg helper to operate scatterlist Ming Lei
                   ` (15 preceding siblings ...)
  2019-06-17  3:03 ` [PATCH V4 16/16] NCR5380: Support chained sg lists Ming Lei
@ 2019-06-17 23:52 ` Bart Van Assche
  16 siblings, 0 replies; 40+ messages in thread
From: Bart Van Assche @ 2019-06-17 23:52 UTC (permalink / raw)
  To: Ming Lei, linux-scsi, Martin K . Petersen
  Cc: James Bottomley, Hannes Reinecke, Christoph Hellwig, Jim Gill,
	Cathy Avery, Ewan D . Milne, Brian King, James Smart,
	Juergen E . Fischer, Michael Schmitz, Finn Thain,
	Greg Kroah-Hartman, devel, linux-usb, Dan Carpenter,
	Benjamin Block

On 6/16/19 8:03 PM, Ming Lei wrote:
> Scsi MQ makes a large static allocation for the first scatter gather
> list chunk for the driver to use.  This is a performance headache we'd
> like to fix by reducing the size of the allocation to a 2 element
> array.  Doing this will break the current guarantee that any driver
> using SG_ALL doesn't need to use the scatterlist iterators and can get
> away with directly dereferencing the array.  Thus we need to update all
> drivers to use the scatterlist iterators and remove direct indexing of
> the scatterlist array before reducing the initial scatterlist
> allocation size in SCSI.

For all patches in this series except 9/16 and 11/16, please add the 
following:

Reviewed-by: Bart Van Assche <bvanassche@acm.org>

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH V4 11/16] scsi: aha152x: use sg helper to operate scatterlist
  2019-06-17 23:35     ` Finn Thain
@ 2019-06-18  0:15       ` Ming Lei
  0 siblings, 0 replies; 40+ messages in thread
From: Ming Lei @ 2019-06-18  0:15 UTC (permalink / raw)
  To: Finn Thain
  Cc: Juergen E . Fischer, linux-scsi, Martin K . Petersen,
	James Bottomley, Bart Van Assche, Hannes Reinecke,
	Christoph Hellwig, Jim Gill, Cathy Avery, Ewan D . Milne,
	Brian King, James Smart, Michael Schmitz, Greg Kroah-Hartman,
	devel, linux-usb, Dan Carpenter, Benjamin Block

On Tue, Jun 18, 2019 at 09:35:48AM +1000, Finn Thain wrote:
> On Mon, 17 Jun 2019, Finn Thain wrote:
> 
> > On Mon, 17 Jun 2019, Ming Lei wrote:
> > 
> > > Use the scatterlist iterators and remove direct indexing of the
> > > scatterlist array.
> > > 
> > > This way allows us to pre-allocate one small scatterlist, which can be
> > > chained with one runtime allocated scatterlist if the pre-allocated one
> > > isn't enough for the whole request.
> > > 
> > > Finn added the change to replace SCp.buffers_residual with sg_is_last()
> > > for fixing updating it, and the similar change has been applied on
> > > NCR5380.c
> > > 
> > > Cc: Finn Thain <fthain@telegraphics.com.au>
> > > Signed-off-by: Ming Lei <ming.lei@redhat.com>
> > 
> > Reviewed-by: Finn Thain <fthain@telegraphics.com.au>
> > 
> 
> I have to retract that. I think this patch is still wrong.
> 
> GETSTCNT() appears to be the number of bytes sent since datao_init() and 
> not the number of bytes sent since the start of the command. (Note the 
> CLRSTCNT in datao_init() which appears to clear the transfer counter.) I 
> don't see how the existing datao_end() could work otherwise. (Juergen?)
> 
> So here's another attempt. Ming, I'd be happy to take the blame/credit (in 
> the form of a From tag etc.) in case you don't want to spend more time on 
> this.

Sure, will do that in V5.

Thanks,
Ming

^ permalink raw reply	[flat|nested] 40+ messages in thread

end of thread, back to index

Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-17  3:03 [PATCH V4 00/16] use sg helper to operate scatterlist Ming Lei
2019-06-17  3:03 ` [PATCH V4 01/16] scsi: vmw_pscsi: " Ming Lei
2019-06-17  8:22   ` Christoph Hellwig
2019-06-17  3:03 ` [PATCH V4 02/16] scsi: advansys: " Ming Lei
2019-06-17  8:22   ` Christoph Hellwig
2019-06-17  3:03 ` [PATCH V4 03/16] scsi: lpfc: " Ming Lei
2019-06-17  8:22   ` Christoph Hellwig
2019-06-17  3:03 ` [PATCH V4 04/16] scsi: mvumi: " Ming Lei
2019-06-17  8:22   ` Christoph Hellwig
2019-06-17  3:03 ` [PATCH V4 05/16] scsi: ipr: " Ming Lei
2019-06-17  8:24   ` Christoph Hellwig
2019-06-17  8:50     ` Ming Lei
2019-06-17  3:03 ` [PATCH V4 06/16] scsi: pmcraid: " Ming Lei
2019-06-17  8:24   ` Christoph Hellwig
2019-06-17  3:03 ` [PATCH V4 07/16] usb: image: microtek: " Ming Lei
2019-06-17  8:25   ` Christoph Hellwig
2019-06-17  3:03 ` [PATCH V4 08/16] staging: unisys: visorhba: " Ming Lei
2019-06-17  8:25   ` Christoph Hellwig
2019-06-17  3:03 ` [PATCH V4 09/16] staging: rtsx: " Ming Lei
2019-06-17  8:27   ` Christoph Hellwig
2019-06-17  9:15     ` Ming Lei
2019-06-17  9:24       ` Christoph Hellwig
2019-06-17  3:03 ` [PATCH V4 10/16] s390: zfcp_fc: " Ming Lei
2019-06-17  8:27   ` Christoph Hellwig
2019-06-17  3:03 ` [PATCH V4 11/16] scsi: aha152x: " Ming Lei
2019-06-17  3:35   ` Finn Thain
2019-06-17 23:35     ` Finn Thain
2019-06-18  0:15       ` Ming Lei
2019-06-17  8:28   ` Christoph Hellwig
2019-06-17  3:03 ` [PATCH V4 12/16] scsi: imm: " Ming Lei
2019-06-17  8:28   ` Christoph Hellwig
2019-06-17  3:03 ` [PATCH V4 13/16] scsi: pcmcia: nsp_cs: " Ming Lei
2019-06-17  8:29   ` Christoph Hellwig
2019-06-17  3:03 ` [PATCH V4 14/16] scsi: ppa: " Ming Lei
2019-06-17  8:29   ` Christoph Hellwig
2019-06-17  3:03 ` [PATCH V4 15/16] scsi: wd33c93: " Ming Lei
2019-06-17  8:29   ` Christoph Hellwig
2019-06-17  3:03 ` [PATCH V4 16/16] NCR5380: Support chained sg lists Ming Lei
2019-06-17  8:30   ` Christoph Hellwig
2019-06-17 23:52 ` [PATCH V4 00/16] use sg helper to operate scatterlist Bart Van Assche

Linux-USB Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-usb/0 linux-usb/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-usb linux-usb/ https://lore.kernel.org/linux-usb \
		linux-usb@vger.kernel.org linux-usb@archiver.kernel.org
	public-inbox-index linux-usb

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-usb


AGPL code for this site: git clone https://public-inbox.org/ public-inbox