remove opencoded kmap of bio_vecs

remove opencoded kmap of bio_vecs

[Christoph Hellwig]

Hi all,

this series replaces various open coded kmaps of bio_vecs with higher
level helpers that use kmap_local_page underneath.

Diffstat:
 arch/xtensa/platforms/iss/simdisk.c |    4 ++--
 drivers/block/aoe/aoecmd.c          |    2 +-
 drivers/block/drbd/drbd_receiver.c  |    4 ++--
 drivers/block/drbd/drbd_worker.c    |    6 +++---
 drivers/block/floppy.c              |    6 ++----
 drivers/block/zram/zram_drv.c       |    9 ++-------
 drivers/md/bcache/request.c         |    4 ++--
 drivers/nvdimm/blk.c                |    7 +++----
 drivers/nvdimm/btt.c                |   10 ++++------
 9 files changed, 21 insertions(+), 31 deletions(-)

[PATCH 01/10] iss-simdisk: use bvec_kmap_local in simdisk_submit_bio

[Christoph Hellwig]

Using local kmaps slightly reduces the chances to stray writes, and
the bvec interface cleans up the code a little bit.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/xtensa/platforms/iss/simdisk.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/xtensa/platforms/iss/simdisk.c b/arch/xtensa/platforms/iss/simdisk.c
index 8eb6ad1a3a1de..0f0e0724397f4 100644
--- a/arch/xtensa/platforms/iss/simdisk.c
+++ b/arch/xtensa/platforms/iss/simdisk.c
@@ -108,13 +108,13 @@ static void simdisk_submit_bio(struct bio *bio)
 	sector_t sector = bio->bi_iter.bi_sector;
 
 	bio_for_each_segment(bvec, bio, iter) {
-		char *buffer = kmap_atomic(bvec.bv_page) + bvec.bv_offset;
+		char *buffer = bvec_kmap_local(&bvec);
 		unsigned len = bvec.bv_len >> SECTOR_SHIFT;
 
 		simdisk_transfer(dev, sector, len, buffer,
 				bio_data_dir(bio) == WRITE);
 		sector += len;
-		kunmap_atomic(buffer);
+		kunmap_local(buffer);
 	}
 
 	bio_endio(bio);
-- 
2.30.2

[PATCH 02/10] aoe: use bvec_kmap_local in bvcpy

[Christoph Hellwig]

Using local kmaps slightly reduces the chances to stray writes, and
the bvec interface cleans up the code a little bit.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/block/aoe/aoecmd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/block/aoe/aoecmd.c b/drivers/block/aoe/aoecmd.c
index cc11f89a0928f..093996961d452 100644
--- a/drivers/block/aoe/aoecmd.c
+++ b/drivers/block/aoe/aoecmd.c
@@ -1018,7 +1018,7 @@ bvcpy(struct sk_buff *skb, struct bio *bio, struct bvec_iter iter, long cnt)
 	iter.bi_size = cnt;
 
 	__bio_for_each_segment(bv, bio, iter, iter) {
-		char *p = kmap_atomic(bv.bv_page) + bv.bv_offset;
+		char *p = bvec_kmap_local(&bv);
 		skb_copy_bits(skb, soff, p, bv.bv_len);
 		kunmap_atomic(p);
 		soff += bv.bv_len;
-- 
2.30.2

[PATCH 03/10] zram: use memcpy_to_bvec in zram_bvec_read

[Christoph Hellwig]

Use the proper helper instead of open coding the copy.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/block/zram/zram_drv.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index a3a5e1e713268..14becdf2815df 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -1331,12 +1331,10 @@ static int zram_bvec_read(struct zram *zram, struct bio_vec *bvec,
 		goto out;
 
 	if (is_partial_io(bvec)) {
-		void *dst = kmap_atomic(bvec->bv_page);
 		void *src = kmap_atomic(page);
 
-		memcpy(dst + bvec->bv_offset, src + offset, bvec->bv_len);
+		memcpy_to_bvec(bvec, src + offset);
 		kunmap_atomic(src);
-		kunmap_atomic(dst);
 	}
 out:
 	if (is_partial_io(bvec))
-- 
2.30.2

[PATCH 04/10] zram: use memcpy_from_bvec in zram_bvec_write

[Christoph Hellwig]

Use memcpy_from_bvec instead of open coding the logic.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/block/zram/zram_drv.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 14becdf2815df..e9474b02012de 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -1465,7 +1465,6 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec,
 {
 	int ret;
 	struct page *page = NULL;
-	void *src;
 	struct bio_vec vec;
 
 	vec = *bvec;
@@ -1483,11 +1482,9 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec,
 		if (ret)
 			goto out;
 
-		src = kmap_atomic(bvec->bv_page);
 		dst = kmap_atomic(page);
-		memcpy(dst + offset, src + bvec->bv_offset, bvec->bv_len);
+		memcpy_from_bvec(dst + offset, bvec);
 		kunmap_atomic(dst);
-		kunmap_atomic(src);
 
 		vec.bv_page = page;
 		vec.bv_len = PAGE_SIZE;
-- 
2.30.2

[PATCH 05/10] nvdimm-blk: use bvec_kmap_local in nd_blk_rw_integrity

[Christoph Hellwig]

Using local kmaps slightly reduces the chances to stray writes, and
the bvec interface cleans up the code a little bit.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/nvdimm/blk.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/nvdimm/blk.c b/drivers/nvdimm/blk.c
index c1db43524d755..0a38738335941 100644
--- a/drivers/nvdimm/blk.c
+++ b/drivers/nvdimm/blk.c
@@ -88,10 +88,9 @@ static int nd_blk_rw_integrity(struct nd_namespace_blk *nsblk,
 		 */
 
 		cur_len = min(len, bv.bv_len);
-		iobuf = kmap_atomic(bv.bv_page);
-		err = ndbr->do_io(ndbr, dev_offset, iobuf + bv.bv_offset,
-				cur_len, rw);
-		kunmap_atomic(iobuf);
+		iobuf = bvec_kmap_local(&bv);
+		err = ndbr->do_io(ndbr, dev_offset, iobuf, cur_len, rw);
+		kunmap_local(iobuf);
 		if (err)
 			return err;
 
-- 
2.30.2

[PATCH 06/10] nvdimm-btt: use bvec_kmap_local in btt_rw_integrity

[Christoph Hellwig]

Using local kmaps slightly reduces the chances to stray writes, and
the bvec interface cleans up the code a little bit.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/nvdimm/btt.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c
index cbd994f7f1fe6..9613e54c7a675 100644
--- a/drivers/nvdimm/btt.c
+++ b/drivers/nvdimm/btt.c
@@ -1163,17 +1163,15 @@ static int btt_rw_integrity(struct btt *btt, struct bio_integrity_payload *bip,
 		 */
 
 		cur_len = min(len, bv.bv_len);
-		mem = kmap_atomic(bv.bv_page);
+		mem = bvec_kmap_local(&bv);
 		if (rw)
-			ret = arena_write_bytes(arena, meta_nsoff,
-					mem + bv.bv_offset, cur_len,
+			ret = arena_write_bytes(arena, meta_nsoff, mem, cur_len,
 					NVDIMM_IO_ATOMIC);
 		else
-			ret = arena_read_bytes(arena, meta_nsoff,
-					mem + bv.bv_offset, cur_len,
+			ret = arena_read_bytes(arena, meta_nsoff, mem, cur_len,
 					NVDIMM_IO_ATOMIC);
 
-		kunmap_atomic(mem);
+		kunmap_local(mem);
 		if (ret)
 			return ret;
 
-- 
2.30.2

[PATCH 07/10] bcache: use bvec_kmap_local in bio_csum

[Christoph Hellwig]

Using local kmaps slightly reduces the chances to stray writes, and
the bvec interface cleans up the code a little bit.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/md/bcache/request.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/md/bcache/request.c b/drivers/md/bcache/request.c
index 6869e010475a3..4e55ca8ca67ff 100644
--- a/drivers/md/bcache/request.c
+++ b/drivers/md/bcache/request.c
@@ -44,10 +44,10 @@ static void bio_csum(struct bio *bio, struct bkey *k)
 	uint64_t csum = 0;
 
 	bio_for_each_segment(bv, bio, iter) {
-		void *d = kmap(bv.bv_page) + bv.bv_offset;
+		void *d = bvec_kmap_local(&bv);
 
 		csum = crc64_be(csum, d, bv.bv_len);
-		kunmap(bv.bv_page);
+		kunmap(d);
 	}
 
 	k->ptr[KEY_PTRS(k)] = csum & (~0ULL >> 1);
-- 
2.30.2

[PATCH 08/10] drbd: use bvec_kmap_local in drbd_csum_bio

[Christoph Hellwig]

Using local kmaps slightly reduces the chances to stray writes, and
the bvec interface cleans up the code a little bit.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/block/drbd/drbd_worker.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/block/drbd/drbd_worker.c b/drivers/block/drbd/drbd_worker.c
index a5e04b38006b6..1b48c8172a077 100644
--- a/drivers/block/drbd/drbd_worker.c
+++ b/drivers/block/drbd/drbd_worker.c
@@ -326,9 +326,9 @@ void drbd_csum_bio(struct crypto_shash *tfm, struct bio *bio, void *digest)
 	bio_for_each_segment(bvec, bio, iter) {
 		u8 *src;
 
-		src = kmap_atomic(bvec.bv_page);
-		crypto_shash_update(desc, src + bvec.bv_offset, bvec.bv_len);
-		kunmap_atomic(src);
+		src = bvec_kmap_local(&bvec);
+		crypto_shash_update(desc, src, bvec.bv_len);
+		kunmap_local(src);
 
 		/* REQ_OP_WRITE_SAME has only one segment,
 		 * checksum the payload only once. */
-- 
2.30.2

[PATCH 09/10] drbd: use bvec_kmap_local in recv_dless_read

[Christoph Hellwig]

Using local kmaps slightly reduces the chances to stray writes, and
the bvec interface cleans up the code a little bit.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/block/drbd/drbd_receiver.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/block/drbd/drbd_receiver.c b/drivers/block/drbd/drbd_receiver.c
index 04e3ec12d8b49..fa00cf2ea9529 100644
--- a/drivers/block/drbd/drbd_receiver.c
+++ b/drivers/block/drbd/drbd_receiver.c
@@ -2017,10 +2017,10 @@ static int recv_dless_read(struct drbd_peer_device *peer_device, struct drbd_req
 	D_ASSERT(peer_device->device, sector == bio->bi_iter.bi_sector);
 
 	bio_for_each_segment(bvec, bio, iter) {
-		void *mapped = kmap(bvec.bv_page) + bvec.bv_offset;
+		void *mapped = bvec_kmap_local(&bvec);
 		expect = min_t(int, data_size, bvec.bv_len);
 		err = drbd_recv_all_warn(peer_device->connection, mapped, expect);
-		kunmap(bvec.bv_page);
+		kunmap_local(mapped);
 		if (err)
 			return err;
 		data_size -= expect;
-- 
2.30.2

[PATCH 10/10] floppy: use memcpy_{to,from}_bvec

[Christoph Hellwig]

Use the helpers instead of open coding them.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/block/floppy.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index 19c2d0327e157..8c647532e3ce9 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -2485,11 +2485,9 @@ static void copy_buffer(int ssize, int max_sector, int max_sector_2)
 		}
 
 		if (CT(raw_cmd->cmd[COMMAND]) == FD_READ)
-			memcpy_to_page(bv.bv_page, bv.bv_offset, dma_buffer,
-				       size);
+			memcpy_to_bvec(&bv, dma_buffer);
 		else
-			memcpy_from_page(dma_buffer, bv.bv_page, bv.bv_offset,
-					 size);
+			memcpy_from_bvec(dma_buffer, &bv);
 
 		remaining -= size;
 		dma_buffer += size;
-- 
2.30.2

Re: remove opencoded kmap of bio_vecs

[Chaitanya Kulkarni]

On 2/22/22 07:51, Christoph Hellwig wrote:
> Hi all,
> 
> this series replaces various open coded kmaps of bio_vecs with higher
> level helpers that use kmap_local_page underneath.
> 


this series looks good to me, looks good.

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

Re: [PATCH 01/10] iss-simdisk: use bvec_kmap_local in simdisk_submit_bio

[Ira Weiny]

On Tue, Feb 22, 2022 at 04:51:47PM +0100, Christoph Hellwig wrote:
> Using local kmaps slightly reduces the chances to stray writes, and
> the bvec interface cleans up the code a little bit.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Ira Weiny <ira.weiny@intel.com>

> ---
>  arch/xtensa/platforms/iss/simdisk.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/xtensa/platforms/iss/simdisk.c b/arch/xtensa/platforms/iss/simdisk.c
> index 8eb6ad1a3a1de..0f0e0724397f4 100644
> --- a/arch/xtensa/platforms/iss/simdisk.c
> +++ b/arch/xtensa/platforms/iss/simdisk.c
> @@ -108,13 +108,13 @@ static void simdisk_submit_bio(struct bio *bio)
>  	sector_t sector = bio->bi_iter.bi_sector;
>  
>  	bio_for_each_segment(bvec, bio, iter) {
> -		char *buffer = kmap_atomic(bvec.bv_page) + bvec.bv_offset;
> +		char *buffer = bvec_kmap_local(&bvec);
>  		unsigned len = bvec.bv_len >> SECTOR_SHIFT;
>  
>  		simdisk_transfer(dev, sector, len, buffer,
>  				bio_data_dir(bio) == WRITE);
>  		sector += len;
> -		kunmap_atomic(buffer);
> +		kunmap_local(buffer);
>  	}
>  
>  	bio_endio(bio);
> -- 
> 2.30.2
> 
> 

Re: [PATCH 02/10] aoe: use bvec_kmap_local in bvcpy

[Ira Weiny]

On Tue, Feb 22, 2022 at 04:51:48PM +0100, Christoph Hellwig wrote:
> Using local kmaps slightly reduces the chances to stray writes, and
> the bvec interface cleans up the code a little bit.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/block/aoe/aoecmd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/block/aoe/aoecmd.c b/drivers/block/aoe/aoecmd.c
> index cc11f89a0928f..093996961d452 100644
> --- a/drivers/block/aoe/aoecmd.c
> +++ b/drivers/block/aoe/aoecmd.c
> @@ -1018,7 +1018,7 @@ bvcpy(struct sk_buff *skb, struct bio *bio, struct bvec_iter iter, long cnt)
>  	iter.bi_size = cnt;
>  
>  	__bio_for_each_segment(bv, bio, iter, iter) {
> -		char *p = kmap_atomic(bv.bv_page) + bv.bv_offset;
> +		char *p = bvec_kmap_local(&bv);
>  		skb_copy_bits(skb, soff, p, bv.bv_len);
>  		kunmap_atomic(p);

kunmap_local()

Ira

>  		soff += bv.bv_len;
> -- 
> 2.30.2
> 
> 

Re: [PATCH 03/10] zram: use memcpy_to_bvec in zram_bvec_read

[Ira Weiny]

On Tue, Feb 22, 2022 at 04:51:49PM +0100, Christoph Hellwig wrote:
> Use the proper helper instead of open coding the copy.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks fine but I don't see a reason to keep the other operation atomic.  Could
the src map also use kmap_local_page()?

Reviewed-by: Ira Weiny <ira.weiny@intel.com>

> ---
>  drivers/block/zram/zram_drv.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index a3a5e1e713268..14becdf2815df 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -1331,12 +1331,10 @@ static int zram_bvec_read(struct zram *zram, struct bio_vec *bvec,
>  		goto out;
>  
>  	if (is_partial_io(bvec)) {
> -		void *dst = kmap_atomic(bvec->bv_page);
>  		void *src = kmap_atomic(page);
>  
> -		memcpy(dst + bvec->bv_offset, src + offset, bvec->bv_len);
> +		memcpy_to_bvec(bvec, src + offset);
>  		kunmap_atomic(src);
> -		kunmap_atomic(dst);
>  	}
>  out:
>  	if (is_partial_io(bvec))
> -- 
> 2.30.2
> 
> 

Re: [PATCH 04/10] zram: use memcpy_from_bvec in zram_bvec_write

[Ira Weiny]

On Tue, Feb 22, 2022 at 04:51:50PM +0100, Christoph Hellwig wrote:
> Use memcpy_from_bvec instead of open coding the logic.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Same comment regarding the dst map.  Does it need to be atomic?

Regardless,
Reviewed-by: Ira Weiny <ira.weiny@intel.com>

> ---
>  drivers/block/zram/zram_drv.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index 14becdf2815df..e9474b02012de 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -1465,7 +1465,6 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec,
>  {
>  	int ret;
>  	struct page *page = NULL;
> -	void *src;
>  	struct bio_vec vec;
>  
>  	vec = *bvec;
> @@ -1483,11 +1482,9 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec,
>  		if (ret)
>  			goto out;
>  
> -		src = kmap_atomic(bvec->bv_page);
>  		dst = kmap_atomic(page);
> -		memcpy(dst + offset, src + bvec->bv_offset, bvec->bv_len);
> +		memcpy_from_bvec(dst + offset, bvec);
>  		kunmap_atomic(dst);
> -		kunmap_atomic(src);
>  
>  		vec.bv_page = page;
>  		vec.bv_len = PAGE_SIZE;
> -- 
> 2.30.2
> 
> 

Re: [PATCH 05/10] nvdimm-blk: use bvec_kmap_local in nd_blk_rw_integrity

[Ira Weiny]

On Tue, Feb 22, 2022 at 04:51:51PM +0100, Christoph Hellwig wrote:
> Using local kmaps slightly reduces the chances to stray writes, and
> the bvec interface cleans up the code a little bit.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Ira Weiny <ira.weiny@intel.com>

> ---
>  drivers/nvdimm/blk.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/nvdimm/blk.c b/drivers/nvdimm/blk.c
> index c1db43524d755..0a38738335941 100644
> --- a/drivers/nvdimm/blk.c
> +++ b/drivers/nvdimm/blk.c
> @@ -88,10 +88,9 @@ static int nd_blk_rw_integrity(struct nd_namespace_blk *nsblk,
>  		 */
>  
>  		cur_len = min(len, bv.bv_len);
> -		iobuf = kmap_atomic(bv.bv_page);
> -		err = ndbr->do_io(ndbr, dev_offset, iobuf + bv.bv_offset,
> -				cur_len, rw);
> -		kunmap_atomic(iobuf);
> +		iobuf = bvec_kmap_local(&bv);
> +		err = ndbr->do_io(ndbr, dev_offset, iobuf, cur_len, rw);
> +		kunmap_local(iobuf);
>  		if (err)
>  			return err;
>  
> -- 
> 2.30.2
> 
> 

Re: [PATCH 06/10] nvdimm-btt: use bvec_kmap_local in btt_rw_integrity

[Ira Weiny]

On Tue, Feb 22, 2022 at 04:51:52PM +0100, Christoph Hellwig wrote:
> Using local kmaps slightly reduces the chances to stray writes, and
> the bvec interface cleans up the code a little bit.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/nvdimm/btt.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c
> index cbd994f7f1fe6..9613e54c7a675 100644
> --- a/drivers/nvdimm/btt.c
> +++ b/drivers/nvdimm/btt.c
> @@ -1163,17 +1163,15 @@ static int btt_rw_integrity(struct btt *btt, struct bio_integrity_payload *bip,
>  		 */
>  
>  		cur_len = min(len, bv.bv_len);
> -		mem = kmap_atomic(bv.bv_page);
> +		mem = bvec_kmap_local(&bv);
>  		if (rw)
> -			ret = arena_write_bytes(arena, meta_nsoff,
> -					mem + bv.bv_offset, cur_len,
> +			ret = arena_write_bytes(arena, meta_nsoff, mem, cur_len,

Why drop bv.bv_offset here and below?

Ira

>  					NVDIMM_IO_ATOMIC);
>  		else
> -			ret = arena_read_bytes(arena, meta_nsoff,
> -					mem + bv.bv_offset, cur_len,
> +			ret = arena_read_bytes(arena, meta_nsoff, mem, cur_len,
>  					NVDIMM_IO_ATOMIC);
>  
> -		kunmap_atomic(mem);
> +		kunmap_local(mem);
>  		if (ret)
>  			return ret;
>  
> -- 
> 2.30.2
> 
> 

Re: [PATCH 07/10] bcache: use bvec_kmap_local in bio_csum

[Ira Weiny]

On Tue, Feb 22, 2022 at 04:51:53PM +0100, Christoph Hellwig wrote:
> Using local kmaps slightly reduces the chances to stray writes, and
> the bvec interface cleans up the code a little bit.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/md/bcache/request.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/md/bcache/request.c b/drivers/md/bcache/request.c
> index 6869e010475a3..4e55ca8ca67ff 100644
> --- a/drivers/md/bcache/request.c
> +++ b/drivers/md/bcache/request.c
> @@ -44,10 +44,10 @@ static void bio_csum(struct bio *bio, struct bkey *k)
>  	uint64_t csum = 0;
>  
>  	bio_for_each_segment(bv, bio, iter) {
> -		void *d = kmap(bv.bv_page) + bv.bv_offset;
> +		void *d = bvec_kmap_local(&bv);
>  
>  		csum = crc64_be(csum, d, bv.bv_len);
> -		kunmap(bv.bv_page);
> +		kunmap(d);

kunmap_local()

Ira

>  	}
>  
>  	k->ptr[KEY_PTRS(k)] = csum & (~0ULL >> 1);
> -- 
> 2.30.2
> 
> 

Re: [PATCH 08/10] drbd: use bvec_kmap_local in drbd_csum_bio

[Ira Weiny]

On Tue, Feb 22, 2022 at 04:51:54PM +0100, Christoph Hellwig wrote:
> Using local kmaps slightly reduces the chances to stray writes, and
> the bvec interface cleans up the code a little bit.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Ira Weiny <ira.weiny@intel.com>

> ---
>  drivers/block/drbd/drbd_worker.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/block/drbd/drbd_worker.c b/drivers/block/drbd/drbd_worker.c
> index a5e04b38006b6..1b48c8172a077 100644
> --- a/drivers/block/drbd/drbd_worker.c
> +++ b/drivers/block/drbd/drbd_worker.c
> @@ -326,9 +326,9 @@ void drbd_csum_bio(struct crypto_shash *tfm, struct bio *bio, void *digest)
>  	bio_for_each_segment(bvec, bio, iter) {
>  		u8 *src;
>  
> -		src = kmap_atomic(bvec.bv_page);
> -		crypto_shash_update(desc, src + bvec.bv_offset, bvec.bv_len);
> -		kunmap_atomic(src);
> +		src = bvec_kmap_local(&bvec);
> +		crypto_shash_update(desc, src, bvec.bv_len);
> +		kunmap_local(src);
>  
>  		/* REQ_OP_WRITE_SAME has only one segment,
>  		 * checksum the payload only once. */
> -- 
> 2.30.2
> 
> 

Re: [PATCH 09/10] drbd: use bvec_kmap_local in recv_dless_read

[Ira Weiny]

On Tue, Feb 22, 2022 at 04:51:55PM +0100, Christoph Hellwig wrote:
> Using local kmaps slightly reduces the chances to stray writes, and
> the bvec interface cleans up the code a little bit.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Ira Weiny <ira.weiny@intel.com>

> ---
>  drivers/block/drbd/drbd_receiver.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/block/drbd/drbd_receiver.c b/drivers/block/drbd/drbd_receiver.c
> index 04e3ec12d8b49..fa00cf2ea9529 100644
> --- a/drivers/block/drbd/drbd_receiver.c
> +++ b/drivers/block/drbd/drbd_receiver.c
> @@ -2017,10 +2017,10 @@ static int recv_dless_read(struct drbd_peer_device *peer_device, struct drbd_req
>  	D_ASSERT(peer_device->device, sector == bio->bi_iter.bi_sector);
>  
>  	bio_for_each_segment(bvec, bio, iter) {
> -		void *mapped = kmap(bvec.bv_page) + bvec.bv_offset;
> +		void *mapped = bvec_kmap_local(&bvec);
>  		expect = min_t(int, data_size, bvec.bv_len);
>  		err = drbd_recv_all_warn(peer_device->connection, mapped, expect);
> -		kunmap(bvec.bv_page);
> +		kunmap_local(mapped);
>  		if (err)
>  			return err;
>  		data_size -= expect;
> -- 
> 2.30.2
> 
> 

Re: [PATCH 10/10] floppy: use memcpy_{to,from}_bvec

[Ira Weiny]

On Tue, Feb 22, 2022 at 04:51:56PM +0100, Christoph Hellwig wrote:
> Use the helpers instead of open coding them.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Ira Weiny <ira.weiny@intel.com>

> ---
>  drivers/block/floppy.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
> index 19c2d0327e157..8c647532e3ce9 100644
> --- a/drivers/block/floppy.c
> +++ b/drivers/block/floppy.c
> @@ -2485,11 +2485,9 @@ static void copy_buffer(int ssize, int max_sector, int max_sector_2)
>  		}
>  
>  		if (CT(raw_cmd->cmd[COMMAND]) == FD_READ)
> -			memcpy_to_page(bv.bv_page, bv.bv_offset, dma_buffer,
> -				       size);
> +			memcpy_to_bvec(&bv, dma_buffer);
>  		else
> -			memcpy_from_page(dma_buffer, bv.bv_page, bv.bv_offset,
> -					 size);
> +			memcpy_from_bvec(dma_buffer, &bv);
>  
>  		remaining -= size;
>  		dma_buffer += size;
> -- 
> 2.30.2
> 
> 

Re: [PATCH 03/10] zram: use memcpy_to_bvec in zram_bvec_read

[Christoph Hellwig]

On Mon, Feb 28, 2022 at 05:10:09PM -0800, Ira Weiny wrote:
> On Tue, Feb 22, 2022 at 04:51:49PM +0100, Christoph Hellwig wrote:
> > Use the proper helper instead of open coding the copy.
> > 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> Looks fine but I don't see a reason to keep the other operation atomic.  Could
> the src map also use kmap_local_page()?

That switch obviously makes sense, but in this series I've focussed on
the bio_vec maps so far.

Re: [PATCH 06/10] nvdimm-btt: use bvec_kmap_local in btt_rw_integrity

[Christoph Hellwig]

On Mon, Feb 28, 2022 at 07:59:06PM -0800, Ira Weiny wrote:
> On Tue, Feb 22, 2022 at 04:51:52PM +0100, Christoph Hellwig wrote:
> > Using local kmaps slightly reduces the chances to stray writes, and
> > the bvec interface cleans up the code a little bit.
> > 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > ---
> >  drivers/nvdimm/btt.c | 10 ++++------
> >  1 file changed, 4 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c
> > index cbd994f7f1fe6..9613e54c7a675 100644
> > --- a/drivers/nvdimm/btt.c
> > +++ b/drivers/nvdimm/btt.c
> > @@ -1163,17 +1163,15 @@ static int btt_rw_integrity(struct btt *btt, struct bio_integrity_payload *bip,
> >  		 */
> >  
> >  		cur_len = min(len, bv.bv_len);
> > -		mem = kmap_atomic(bv.bv_page);
> > +		mem = bvec_kmap_local(&bv);
> >  		if (rw)
> > -			ret = arena_write_bytes(arena, meta_nsoff,
> > -					mem + bv.bv_offset, cur_len,
> > +			ret = arena_write_bytes(arena, meta_nsoff, mem, cur_len,
> 
> Why drop bv.bv_offset here and below?

Because bvec_kmap_local includes bv_offset in the pointer that it
returns.

Re: [PATCH 06/10] nvdimm-btt: use bvec_kmap_local in btt_rw_integrity

[Ira Weiny]

On Tue, Mar 01, 2022 at 01:20:42PM +0100, Christoph Hellwig wrote:
> On Mon, Feb 28, 2022 at 07:59:06PM -0800, Ira Weiny wrote:
> > On Tue, Feb 22, 2022 at 04:51:52PM +0100, Christoph Hellwig wrote:
> > > Using local kmaps slightly reduces the chances to stray writes, and
> > > the bvec interface cleans up the code a little bit.
> > > 
> > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > > ---
> > >  drivers/nvdimm/btt.c | 10 ++++------
> > >  1 file changed, 4 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c
> > > index cbd994f7f1fe6..9613e54c7a675 100644
> > > --- a/drivers/nvdimm/btt.c
> > > +++ b/drivers/nvdimm/btt.c
> > > @@ -1163,17 +1163,15 @@ static int btt_rw_integrity(struct btt *btt, struct bio_integrity_payload *bip,
> > >  		 */
> > >  
> > >  		cur_len = min(len, bv.bv_len);
> > > -		mem = kmap_atomic(bv.bv_page);
> > > +		mem = bvec_kmap_local(&bv);
> > >  		if (rw)
> > > -			ret = arena_write_bytes(arena, meta_nsoff,
> > > -					mem + bv.bv_offset, cur_len,
> > > +			ret = arena_write_bytes(arena, meta_nsoff, mem, cur_len,
> > 
> > Why drop bv.bv_offset here and below?
> 
> Because bvec_kmap_local includes bv_offset in the pointer that it
> returns.

Ah I missed that.  Thanks,
Ira

[PATCH 02/10] aoe: use bvec_kmap_local in bvcpy

[Christoph Hellwig]

Using local kmaps slightly reduces the chances to stray writes, and
the bvec interface cleans up the code a little bit.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/block/aoe/aoecmd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/block/aoe/aoecmd.c b/drivers/block/aoe/aoecmd.c
index cc11f89a0928f..384073ef2323c 100644
--- a/drivers/block/aoe/aoecmd.c
+++ b/drivers/block/aoe/aoecmd.c
@@ -1018,9 +1018,9 @@ bvcpy(struct sk_buff *skb, struct bio *bio, struct bvec_iter iter, long cnt)
 	iter.bi_size = cnt;
 
 	__bio_for_each_segment(bv, bio, iter, iter) {
-		char *p = kmap_atomic(bv.bv_page) + bv.bv_offset;
+		char *p = bvec_kmap_local(&bv);
 		skb_copy_bits(skb, soff, p, bv.bv_len);
-		kunmap_atomic(p);
+		kunmap_local(p);
 		soff += bv.bv_len;
 	}
 }
-- 
2.30.2