staging: rts5208: Convert kmap() to kmap_local_page()

PATCH In review Fabio M. De Francesco 3 participants 3 messages Mar 30, 2022 Staging

lore

Share & tools

lore

Fabio M. De Francesco

author

2022-03-30 14:33 UTC #0 lore raw

PATCH v4 1/1 staging: rts5208: Convert kmap() to kmap_local_page()

The use of kmap() is being deprecated in favor of kmap_local_page() where it is feasible.

With kmap_local_page(), the mapping is per thread, CPU local and not globally visible. Therefore rtsx_stor_access_xfer_buf() is a function where the use of kmap_local_page() in place of kmap() is correctly suited.

Convert to kmap_local_page() but, instead of open coding it, use the helpers memcpy_to_page() and memcpy_from_page().

Make a minor change to a comment related to scatter-gather.

Signed-off-by Fabio M. De Francesco <[email protected]>

---

v1 -> v2: Rework the commit message and use the appropriate helpers instead of open coding the use of kmap_local_page()/kunmap_local_page(). (Thanks to Ira Weiny <[email protected]>).

v2 -> v3: Use memcpy_{to,from}_page() arguments correctly. (Thanks to Dan Carpenter <[email protected]>).

v3 -> v4: According to a suggestion by Ira Weiny, change the test of a comment related to the use of scatter-gather.

 drivers/staging/rts5208/rtsx_transport.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

drivers/staging/rts5208/rtsx_transport.c +5 -7

index 805dc18fac0a..d5ad49de4c56 100644
@@ -55,9 +55,9 @@ unsigned int rtsx_stor_access_xfer_buf(unsigned char *buffer,
 		*offset += cnt;
 
 	/*
-	 * Using scatter-gather.  We have to go through the list one entry
-	 * at a time.  Each s-g entry contains some number of pages, and
-	 * each page has to be kmap()'ed separately.
+	 * Using scatter-gather. We have to go through the list one entry
+	 * at a time. Each s-g entry contains some number of pages which
+	 * have to be copied one at a time.
 	 */
 	} else {
 		struct scatterlist *sg =

@@ -92,13 +92,11 @@ unsigned int rtsx_stor_access_xfer_buf(unsigned char *buffer,
 			while (sglen > 0) {
 				unsigned int plen = min(sglen, (unsigned int)
 						PAGE_SIZE - poff);
-				unsigned char *ptr = kmap(page);
 
 				if (dir == TO_XFER_BUF)
-					memcpy(ptr + poff, buffer + cnt, plen);
+					memcpy_to_page(page, poff, buffer + cnt, plen);
 				else
-					memcpy(buffer + cnt, ptr + poff, plen);
-				kunmap(page);
+					memcpy_from_page(buffer + cnt, page, poff, plen);
 
 				/* Start at the beginning of the next page */
 				poff = 0;

Signature · 1 line

2.34.1

Dan Carpenter

Reviewed-by on v4 1/1

2022-03-30 15:33 UTC #1 lore raw

Quoted 8 lines · Fabio M. De Francesco

The use of kmap() is being deprecated in favor of kmap_local_page() where it is feasible.
With kmap_local_page(), the mapping is per thread, CPU local and not globally visible. Therefore rtsx_stor_access_xfer_buf() is a function where the use of kmap_local_page() in place of kmap() is correctly suited.

Quoted 7 lines

Convert to kmap_local_page() but, instead of open coding it, use the helpers memcpy_to_page() and memcpy_from_page().
Make a minor change to a comment related to scatter-gather.
Signed-off-by Fabio M. De Francesco <[email protected]>
---

LGTM.

Reviewed-by Dan Carpenter <[email protected]>

regards,
dan carpenter

Ira Weiny

Reviewed-by on v4 1/1

2022-03-30 21:11 UTC #2 lore raw

In reply to Fabio M. De Francesco

Quoted 8 lines · Fabio M. De Francesco

The use of kmap() is being deprecated in favor of kmap_local_page() where it is feasible.
With kmap_local_page(), the mapping is per thread, CPU local and not globally visible. Therefore rtsx_stor_access_xfer_buf() is a function where the use of kmap_local_page() in place of kmap() is correctly suited.

Quoted 6 lines

Convert to kmap_local_page() but, instead of open coding it, use the helpers memcpy_to_page() and memcpy_from_page().
Make a minor change to a comment related to scatter-gather.
Signed-off-by Fabio M. De Francesco <[email protected]>

Thanks!

Reviewed-by Ira Weiny <[email protected]>

Quoted 51 lines

---

v1 -> v2: Rework the commit message and use the appropriate helpers instead of open coding the use of kmap_local_page()/kunmap_local_page(). (Thanks to Ira Weiny <[email protected]>).

v2 -> v3: Use memcpy_{to,from}_page() arguments correctly. (Thanks to Dan Carpenter <[email protected]>).

v3 -> v4: According to a suggestion by Ira Weiny, change the test of a comment related to the use of scatter-gather.

 drivers/staging/rts5208/rtsx_transport.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

drivers/staging/rts5208/rtsx_transport.c +5 -7

index 805dc18fac0a..d5ad49de4c56 100644
@@ -55,9 +55,9 @@ unsigned int rtsx_stor_access_xfer_buf(unsigned char *buffer,
 		*offset += cnt;
 
 	/*
-	 * Using scatter-gather.  We have to go through the list one entry
-	 * at a time.  Each s-g entry contains some number of pages, and
-	 * each page has to be kmap()'ed separately.
+	 * Using scatter-gather. We have to go through the list one entry
+	 * at a time. Each s-g entry contains some number of pages which
+	 * have to be copied one at a time.
 	 */
 	} else {
 		struct scatterlist *sg =

@@ -92,13 +92,11 @@ unsigned int rtsx_stor_access_xfer_buf(unsigned char *buffer,
 			while (sglen > 0) {
 				unsigned int plen = min(sglen, (unsigned int)
 						PAGE_SIZE - poff);
-				unsigned char *ptr = kmap(page);
 
 				if (dir == TO_XFER_BUF)
-					memcpy(ptr + poff, buffer + cnt, plen);
+					memcpy_to_page(page, poff, buffer + cnt, plen);
 				else
-					memcpy(buffer + cnt, ptr + poff, plen);
-				kunmap(page);
+					memcpy_from_page(buffer + cnt, page, poff, plen);
 
 				/* Start at the beginning of the next page */
 				poff = 0;

Signature · 1 line

2.34.1