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
