LinuxLists.cc - switch the block layer to use kmap_local

[permalink] [raw]

Subject: [PATCH 03/16] bvec: fix the include guards for bvec.h

Fix the include guards to match the file naming.

Signed-off-by: Christoph Hellwig <[email protected]>
---
include/linux/bvec.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/linux/bvec.h b/include/linux/bvec.h
index ff832e698efb..883faf5f1523 100644
--- a/include/linux/bvec.h
+++ b/include/linux/bvec.h
@@ -4,8 +4,8 @@
*
* Copyright (C) 2001 Ming Lei <[email protected]>
*/
-#ifndef __LINUX_BVEC_ITER_H
-#define __LINUX_BVEC_ITER_H
+#ifndef __LINUX_BVEC_H
+#define __LINUX_BVEC_H

#include <linux/bug.h>
#include <linux/errno.h>
@@ -183,4 +183,4 @@ static inline void bvec_advance(const struct bio_vec *bvec,
}
}

-#endif /* __LINUX_BVEC_ITER_H */
+#endif /* __LINUX_BVEC_H */
--
2.30.2

2021-06-08 16:09:06

[permalink] [raw]

Subject: [PATCH 06/16] block: use memzero_page in zero_fill_bio

Use memzero_bvec to zero each segment in the bio instead of manually
mapping and zeroing the data.

Signed-off-by: Christoph Hellwig <[email protected]>
---
block/bio.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/block/bio.c b/block/bio.c
index 44205dfb6b60..1d7abdb83a39 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -495,16 +495,11 @@ EXPORT_SYMBOL(bio_kmalloc);

void zero_fill_bio(struct bio *bio)
{
- unsigned long flags;
struct bio_vec bv;
struct bvec_iter iter;

- bio_for_each_segment(bv, bio, iter) {
- char *data = bvec_kmap_irq(&bv, &flags);
- memset(data, 0, bv.bv_len);
- flush_dcache_page(bv.bv_page);
- bvec_kunmap_irq(data, &flags);
- }
+ bio_for_each_segment(bv, bio, iter)
+ memzero_bvec(&bv);
}
EXPORT_SYMBOL(zero_fill_bio);

--
2.30.2

2021-06-08 16:09:10

[permalink] [raw]

Subject: [PATCH 07/16] rbd: use memzero_bvec

Use memzero_bvec instead of reimplementing it.

Signed-off-by: Christoph Hellwig <[email protected]>
---
drivers/block/rbd.c | 15 ++-------------
1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index bbb88eb009e0..eb243fc4d108 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -1219,24 +1219,13 @@ static void rbd_dev_mapping_clear(struct rbd_device *rbd_dev)
rbd_dev->mapping.size = 0;
}

-static void zero_bvec(struct bio_vec *bv)
-{
- void *buf;
- unsigned long flags;
-
- buf = bvec_kmap_irq(bv, &flags);
- memset(buf, 0, bv->bv_len);
- flush_dcache_page(bv->bv_page);
- bvec_kunmap_irq(buf, &flags);
-}
-
static void zero_bios(struct ceph_bio_iter *bio_pos, u32 off, u32 bytes)
{
struct ceph_bio_iter it = *bio_pos;

ceph_bio_iter_advance(&it, off);
ceph_bio_iter_advance_step(&it, bytes, ({
- zero_bvec(&bv);
+ memzero_bvec(&bv);
}));
}

@@ -1246,7 +1235,7 @@ static void zero_bvecs(struct ceph_bvec_iter *bvec_pos, u32 off, u32 bytes)

ceph_bvec_iter_advance(&it, off);
ceph_bvec_iter_advance_step(&it, bytes, ({
- zero_bvec(&bv);
+ memzero_bvec(&bv);
}));
}

--
2.30.2

2021-06-08 16:09:34

[permalink] [raw]

Subject: [PATCH 05/16] bvec: add memcpy_{from,to}_bvec and memzero_bvec helper

Add helpers to perform common memory operation on a bvec.

Signed-off-by: Christoph Hellwig <[email protected]>
---
include/linux/bvec.h | 15 +++++++++++++++
1 file changed, 15 insertions(+)

diff --git a/include/linux/bvec.h b/include/linux/bvec.h
index d64d6c0ceb77..ac835fa01ee3 100644
--- a/include/linux/bvec.h
+++ b/include/linux/bvec.h
@@ -189,4 +189,19 @@ static inline void *bvec_kmap_local(struct bio_vec *bvec)
return kmap_local_page(bvec->bv_page) + bvec->bv_offset;
}

+static inline void memcpy_from_bvec(char *to, struct bio_vec *bvec)
+{
+ memcpy_from_page(to, bvec->bv_page, bvec->bv_offset, bvec->bv_len);
+}
+
+static inline void memcpy_to_bvec(struct bio_vec *bvec, const char *from)
+{
+ memcpy_to_page(bvec->bv_page, bvec->bv_offset, from, bvec->bv_len);
+}
+
+static inline void memzero_bvec(struct bio_vec *bvec)
+{
+ memzero_page(bvec->bv_page, bvec->bv_offset, bvec->bv_len);
+}
+
#endif /* __LINUX_BVEC_H */
--
2.30.2

2021-06-08 16:10:05

[permalink] [raw]

Subject: [PATCH 09/16] ps3disk: use memcpy_{from,to}_bvec

Use the bvec helpers instead of open coding the copy.

Signed-off-by: Christoph Hellwig <[email protected]>
---
drivers/block/ps3disk.c | 19 +++----------------
1 file changed, 3 insertions(+), 16 deletions(-)

diff --git a/drivers/block/ps3disk.c b/drivers/block/ps3disk.c
index ba3ece56cbb3..f2eb0225814f 100644
--- a/drivers/block/ps3disk.c
+++ b/drivers/block/ps3disk.c
@@ -84,26 +84,13 @@ static void ps3disk_scatter_gather(struct ps3_storage_device *dev,
unsigned int offset = 0;
struct req_iterator iter;
struct bio_vec bvec;
- unsigned int i = 0;
- size_t size;
- void *buf;

rq_for_each_segment(bvec, req, iter) {
- unsigned long flags;
- dev_dbg(&dev->sbd.core, "%s:%u: bio %u: %u sectors from %llu\n",
- __func__, __LINE__, i, bio_sectors(iter.bio),
- iter.bio->bi_iter.bi_sector);
-
- size = bvec.bv_len;
- buf = bvec_kmap_irq(&bvec, &flags);
if (gather)
- memcpy(dev->bounce_buf+offset, buf, size);
+ memcpy_from_bvec(dev->bounce_buf + offset, &bvec);
else
- memcpy(buf, dev->bounce_buf+offset, size);
- offset += size;
- flush_kernel_dcache_page(bvec.bv_page);
- bvec_kunmap_irq(buf, &flags);
- i++;
+ memcpy_to_bvec(&bvec, dev->bounce_buf + offset);
+ offset += bvec.bv_len;
}
}

--
2.30.2

2021-06-08 16:10:22

[permalink] [raw]

Subject: [PATCH 10/16] block: remove bvec_kmap_irq and bvec_kunmap_irq

These two helpers are entirely unused now.

Signed-off-by: Christoph Hellwig <[email protected]>
---
include/linux/bio.h | 42 ------------------------------------------
1 file changed, 42 deletions(-)

diff --git a/include/linux/bio.h b/include/linux/bio.h
index a0b4cfdf62a4..169b14b10c16 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -5,7 +5,6 @@
#ifndef __LINUX_BIO_H
#define __LINUX_BIO_H

-#include <linux/highmem.h>
#include <linux/mempool.h>
#include <linux/ioprio.h>
/* struct bio, bio_vec and BIO_* flags are defined in blk_types.h */
@@ -523,47 +522,6 @@ static inline void bio_clone_blkg_association(struct bio *dst,
struct bio *src) { }
#endif /* CONFIG_BLK_CGROUP */

-#ifdef CONFIG_HIGHMEM
-/*
- * remember never ever reenable interrupts between a bvec_kmap_irq and
- * bvec_kunmap_irq!
- */
-static inline char *bvec_kmap_irq(struct bio_vec *bvec, unsigned long *flags)
-{
- unsigned long addr;
-
- /*
- * might not be a highmem page, but the preempt/irq count
- * balancing is a lot nicer this way
- */
- local_irq_save(*flags);
- addr = (unsigned long) kmap_atomic(bvec->bv_page);
-
- BUG_ON(addr & ~PAGE_MASK);
-
- return (char *) addr + bvec->bv_offset;
-}
-
-static inline void bvec_kunmap_irq(char *buffer, unsigned long *flags)
-{
- unsigned long ptr = (unsigned long) buffer & PAGE_MASK;
-
- kunmap_atomic((void *) ptr);
- local_irq_restore(*flags);
-}
-
-#else
-static inline char *bvec_kmap_irq(struct bio_vec *bvec, unsigned long *flags)
-{
- return page_address(bvec->bv_page) + bvec->bv_offset;
-}
-
-static inline void bvec_kunmap_irq(char *buffer, unsigned long *flags)
-{
- *flags = 0;
-}
-#endif
-
/*
* BIO list management for use by remapping drivers (e.g. DM or MD) and loop.
*
--
2.30.2

2021-06-08 16:11:55

[permalink] [raw]

Subject: [PATCH 12/16] block: use memcpy_to_bvec in copy_to_high_bio_irq

Use memcpy_to_bvec instead of opencoding the logic.

Signed-off-by: Christoph Hellwig <[email protected]>
---
block/bounce.c | 14 +-------------
1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/block/bounce.c b/block/bounce.c
index 94081e013c58..a2fc6326b6c9 100644
--- a/block/bounce.c
+++ b/block/bounce.c
@@ -67,18 +67,6 @@ static __init int init_emergency_pool(void)

__initcall(init_emergency_pool);

-/*
- * highmem version, map in to vec
- */
-static void bounce_copy_vec(struct bio_vec *to, unsigned char *vfrom)
-{
- unsigned char *vto;
-
- vto = kmap_atomic(to->bv_page);
- memcpy(vto + to->bv_offset, vfrom, to->bv_len);
- kunmap_atomic(vto);
-}
-
/*
* Simple bounce buffer support for highmem pages. Depending on the
* queue gfp mask set, *to may or may not be a highmem page. kmap it
@@ -107,7 +95,7 @@ static void copy_to_high_bio_irq(struct bio *to, struct bio *from)
vfrom = page_address(fromvec.bv_page) +
tovec.bv_offset;

- bounce_copy_vec(&tovec, vfrom);
+ memcpy_to_bvec(&tovec, vfrom);
flush_dcache_page(tovec.bv_page);
}
bio_advance_iter(from, &from_iter, tovec.bv_len);
--
2.30.2

2021-06-08 16:12:09

[permalink] [raw]

Subject: [PATCH 16/16] block: use bvec_kmap_local in bio_integrity_process

2021-06-08 16:12:19

[permalink] [raw]

Subject: [PATCH 15/16] block: use bvec_kmap_local in t10_pi_type1_{prepare,complete}

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 <[email protected]>
---
block/t10-pi.c | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/block/t10-pi.c b/block/t10-pi.c
index d910534b3a41..00c203b2a921 100644
--- a/block/t10-pi.c
+++ b/block/t10-pi.c
@@ -147,11 +147,10 @@ static void t10_pi_type1_prepare(struct request *rq)
break;

bip_for_each_vec(iv, bip, iter) {
- void *p, *pmap;
unsigned int j;
+ void *p;

- pmap = kmap_atomic(iv.bv_page);
- p = pmap + iv.bv_offset;
+ p = bvec_kmap_local(&iv);
for (j = 0; j < iv.bv_len; j += tuple_sz) {
struct t10_pi_tuple *pi = p;

@@ -161,8 +160,7 @@ static void t10_pi_type1_prepare(struct request *rq)
ref_tag++;
p += tuple_sz;
}
-
- kunmap_atomic(pmap);
+ kunmap_local(p);
}

bip->bip_flags |= BIP_MAPPED_INTEGRITY;
@@ -195,11 +193,10 @@ static void t10_pi_type1_complete(struct request *rq, unsigned int nr_bytes)
struct bvec_iter iter;

bip_for_each_vec(iv, bip, iter) {
- void *p, *pmap;
unsigned int j;
+ void *p;

- pmap = kmap_atomic(iv.bv_page);
- p = pmap + iv.bv_offset;
+ p = bvec_kmap_local(&iv);
for (j = 0; j < iv.bv_len && intervals; j += tuple_sz) {
struct t10_pi_tuple *pi = p;

@@ -210,8 +207,7 @@ static void t10_pi_type1_complete(struct request *rq, unsigned int nr_bytes)
intervals--;
p += tuple_sz;
}
-
- kunmap_atomic(pmap);
+ kunmap_local(p);
}
}
}
--
2.30.2

2021-06-08 16:13:12

[permalink] [raw]

Subject: [PATCH 14/16] block: use memcpy_from_bvec in __blk_queue_bounce

Rewrite the actual bounce buffering loop in __blk_queue_bounce to that
the memcpy_to_bvec helper can be used to perform the data copies.

Signed-off-by: Christoph Hellwig <[email protected]>
---
block/bounce.c | 21 +++++++--------------
1 file changed, 7 insertions(+), 14 deletions(-)

diff --git a/block/bounce.c b/block/bounce.c
index a2fc6326b6c9..b5ad09e07bcf 100644
--- a/block/bounce.c
+++ b/block/bounce.c
@@ -243,24 +243,17 @@ void __blk_queue_bounce(struct request_queue *q, struct bio **bio_orig)
* because the 'bio' is single-page bvec.
*/
for (i = 0, to = bio->bi_io_vec; i < bio->bi_vcnt; to++, i++) {
- struct page *page = to->bv_page;
+ struct page *bounce_page;

- if (!PageHighMem(page))
+ if (!PageHighMem(to->bv_page))
continue;

- to->bv_page = mempool_alloc(&page_pool, GFP_NOIO);
- inc_zone_page_state(to->bv_page, NR_BOUNCE);
+ bounce_page = mempool_alloc(&page_pool, GFP_NOIO);
+ inc_zone_page_state(bounce_page, NR_BOUNCE);

- if (rw == WRITE) {
- char *vto, *vfrom;
-
- flush_dcache_page(page);
-
- vto = page_address(to->bv_page) + to->bv_offset;
- vfrom = kmap_atomic(page) + to->bv_offset;
- memcpy(vto, vfrom, to->bv_len);
- kunmap_atomic(vfrom);
- }
+ if (rw == WRITE)
+ memcpy_from_bvec(page_address(bounce_page), to);
+ to->bv_page = bounce_page;
}

trace_block_bio_bounce(*bio_orig);
--
2.30.2

2021-06-08 18:23:39

[permalink] [raw]

Subject: Re: [PATCH 05/16] bvec: add memcpy_{from,to}_bvec and memzero_bvec helper

On 6/8/21 09:07, Christoph Hellwig wrote:
> Add helpers to perform common memory operation on a bvec.
>
> Signed-off-by: Christoph Hellwig <[email protected]>

Looks good.

Reviewed-by: Chaitanya Kulkarni <[email protected]>

2021-06-08 18:24:39

[permalink] [raw]

Subject: Re: [PATCH 06/16] block: use memzero_page in zero_fill_bio

On 6/8/21 09:07, Christoph Hellwig wrote:
> Use memzero_bvec to zero each segment in the bio instead of manually
> mapping and zeroing the data.
>
> Signed-off-by: Christoph Hellwig <[email protected]>

Looks good.

Reviewed-by: Chaitanya Kulkarni <[email protected]>

2021-06-08 18:28:28

[permalink] [raw]

Subject: Re: [PATCH 12/16] block: use memcpy_to_bvec in copy_to_high_bio_irq

On 6/8/21 09:08, Christoph Hellwig wrote:
> Use memcpy_to_bvec instead of opencoding the logic.
>
> Signed-off-by: Christoph Hellwig <[email protected]>

Looks good.

Reviewed-by: Chaitanya Kulkarni <[email protected]>

2021-06-09 05:22:03

[permalink] [raw]

Subject: [PATCH 04/16] bvec: add a bvec_kmap_local helper

Add a helper to call kmap_local_page on a bvec. There is no need for
an unmap helper given that kunmap_local accept any address in the mapped
page.

Signed-off-by: Christoph Hellwig <[email protected]>
---
include/linux/bvec.h | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/include/linux/bvec.h b/include/linux/bvec.h
index 883faf5f1523..d64d6c0ceb77 100644
--- a/include/linux/bvec.h
+++ b/include/linux/bvec.h
@@ -7,6 +7,7 @@
#ifndef __LINUX_BVEC_H
#define __LINUX_BVEC_H

+#include <linux/highmem.h>
#include <linux/bug.h>
#include <linux/errno.h>
#include <linux/limits.h>
@@ -183,4 +184,9 @@ static inline void bvec_advance(const struct bio_vec *bvec,
}
}

+static inline void *bvec_kmap_local(struct bio_vec *bvec)
+{
+ return kmap_local_page(bvec->bv_page) + bvec->bv_offset;
+}
+
#endif /* __LINUX_BVEC_H */
--
2.30.2

2021-06-09 05:36:25

by Bart Van Assche

[permalink] [raw]

Subject: Re: [PATCH 03/16] bvec: fix the include guards for bvec.h

On 6/8/21 9:05 AM, Christoph Hellwig wrote:
> Fix the include guards to match the file naming.

Reviewed-by: Bart Van Assche <[email protected]>

2021-06-09 05:53:29

[permalink] [raw]

Subject: [PATCH 08/16] dm-writecache: use bvec_kmap_local instead of bvec_kmap_irq

There is no need to disable interrupts in bio_copy_block, and the local
only mappings helps to avoid any sort of problems with stray writes
into the bio data.

Signed-off-by: Christoph Hellwig <[email protected]>
---
drivers/md/dm-writecache.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/md/dm-writecache.c b/drivers/md/dm-writecache.c
index aecc246ade26..93ca454eaca9 100644
--- a/drivers/md/dm-writecache.c
+++ b/drivers/md/dm-writecache.c
@@ -1205,14 +1205,13 @@ static void memcpy_flushcache_optimized(void *dest, void *source, size_t size)
static void bio_copy_block(struct dm_writecache *wc, struct bio *bio, void *data)
{
void *buf;
- unsigned long flags;
unsigned size;
int rw = bio_data_dir(bio);
unsigned remaining_size = wc->block_size;

do {
struct bio_vec bv = bio_iter_iovec(bio, bio->bi_iter);
- buf = bvec_kmap_irq(&bv, &flags);
+ buf = bvec_kmap_local(&bv);
size = bv.bv_len;
if (unlikely(size > remaining_size))
size = remaining_size;
@@ -1230,7 +1229,7 @@ static void bio_copy_block(struct dm_writecache *wc, struct bio *bio, void *data
memcpy_flushcache_optimized(data, buf, size);
}

- bvec_kunmap_irq(buf, &flags);
+ kunmap_local(buf);

data = (char *)data + size;
remaining_size -= size;
--
2.30.2

2021-06-09 05:53:59

[permalink] [raw]

Subject: [PATCH 11/16] block: rewrite bio_copy_data_iter to use bvec_kmap_local and memcpy_to_bvec

Use the proper helpers instead of open coding the copy.

Signed-off-by: Christoph Hellwig <[email protected]>
---
block/bio.c | 28 ++++++++--------------------
1 file changed, 8 insertions(+), 20 deletions(-)

diff --git a/block/bio.c b/block/bio.c
index 1d7abdb83a39..c14d2e66c084 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -1186,27 +1186,15 @@ EXPORT_SYMBOL(bio_advance);
void bio_copy_data_iter(struct bio *dst, struct bvec_iter *dst_iter,
struct bio *src, struct bvec_iter *src_iter)
{
- struct bio_vec src_bv, dst_bv;
- void *src_p, *dst_p;
- unsigned bytes;
-
while (src_iter->bi_size && dst_iter->bi_size) {
- src_bv = bio_iter_iovec(src, *src_iter);
- dst_bv = bio_iter_iovec(dst, *dst_iter);
-
- bytes = min(src_bv.bv_len, dst_bv.bv_len);
-
- src_p = kmap_atomic(src_bv.bv_page);
- dst_p = kmap_atomic(dst_bv.bv_page);
-
- memcpy(dst_p + dst_bv.bv_offset,
- src_p + src_bv.bv_offset,
- bytes);
-
- kunmap_atomic(dst_p);
- kunmap_atomic(src_p);
-
- flush_dcache_page(dst_bv.bv_page);
+ struct bio_vec src_bv = bio_iter_iovec(src, *src_iter);
+ struct bio_vec dst_bv = bio_iter_iovec(dst, *dst_iter);
+ unsigned int bytes = min(src_bv.bv_len, dst_bv.bv_len);
+ void *src_buf;
+
+ src_buf = bvec_kmap_local(&src_bv);
+ memcpy_to_bvec(&dst_bv, src_buf);
+ kunmap_local(src_buf);

bio_advance_iter_single(src, src_iter, bytes);
bio_advance_iter_single(dst, dst_iter, bytes);
--
2.30.2

2021-06-09 05:54:08

[permalink] [raw]

Subject: [PATCH 13/16] block: use memcpy_from_bvec in bio_copy_kern_endio_read

Use memcpy_from_bvec instead of open coding the logic.

Signed-off-by: Christoph Hellwig <[email protected]>
---
block/blk-map.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/blk-map.c b/block/blk-map.c
index 3743158ddaeb..d1448aaad980 100644
--- a/block/blk-map.c
+++ b/block/blk-map.c
@@ -400,7 +400,7 @@ static void bio_copy_kern_endio_read(struct bio *bio)
struct bvec_iter_all iter_all;

bio_for_each_segment_all(bvec, bio, iter_all) {
- memcpy(p, page_address(bvec->bv_page), bvec->bv_len);
+ memcpy_from_bvec(p, bvec);
p += bvec->bv_len;
}

--
2.30.2

2021-06-09 08:57:23

[permalink] [raw]

Subject: Re: [PATCH 03/16] bvec: fix the include guards for bvec.h

On 6/8/21 09:06, Christoph Hellwig wrote:
> Fix the include guards to match the file naming.
>
> Signed-off-by: Christoph Hellwig <[email protected]>

Looks good.

Reviewed-by: Chaitanya Kulkarni <[email protected]>

2021-06-09 12:22:55

[permalink] [raw]

Subject: Re: [PATCH 09/16] ps3disk: use memcpy_{from,to}_bvec

On Tue, Jun 08, 2021 at 06:05:56PM +0200, Christoph Hellwig wrote:
>
> rq_for_each_segment(bvec, req, iter) {
> - unsigned long flags;
> - dev_dbg(&dev->sbd.core, "%s:%u: bio %u: %u sectors from %llu\n",
> - __func__, __LINE__, i, bio_sectors(iter.bio),
> - iter.bio->bi_iter.bi_sector);
> -
> - size = bvec.bv_len;
> - buf = bvec_kmap_irq(&bvec, &flags);
> if (gather)
> - memcpy(dev->bounce_buf+offset, buf, size);
> + memcpy_from_bvec(dev->bounce_buf + offset, &bvec);
> else
> - memcpy(buf, dev->bounce_buf+offset, size);
> - offset += size;
> - flush_kernel_dcache_page(bvec.bv_page);

I'm still not 100% sure that these flushes are needed but the are not no-ops on
every arch. Would it be best to preserve them after the memcpy_to/from_bvec()?

Same thing in patch 11 and 14.

Ira

2021-06-09 12:24:29

[permalink] [raw]

Subject: Re: [PATCH 14/16] block: use memcpy_from_bvec in __blk_queue_bounce

On Tue, Jun 08, 2021 at 06:06:01PM +0200, Christoph Hellwig wrote:
> Rewrite the actual bounce buffering loop in __blk_queue_bounce to that
> the memcpy_to_bvec helper can be used to perform the data copies.
>
> Signed-off-by: Christoph Hellwig <[email protected]>
> ---
> block/bounce.c | 21 +++++++--------------
> 1 file changed, 7 insertions(+), 14 deletions(-)
>
> diff --git a/block/bounce.c b/block/bounce.c
> index a2fc6326b6c9..b5ad09e07bcf 100644
> --- a/block/bounce.c
> +++ b/block/bounce.c
> @@ -243,24 +243,17 @@ void __blk_queue_bounce(struct request_queue *q, struct bio **bio_orig)
> * because the 'bio' is single-page bvec.
> */
> for (i = 0, to = bio->bi_io_vec; i < bio->bi_vcnt; to++, i++) {
> - struct page *page = to->bv_page;
> + struct page *bounce_page;
>
> - if (!PageHighMem(page))
> + if (!PageHighMem(to->bv_page))
> continue;
>
> - to->bv_page = mempool_alloc(&page_pool, GFP_NOIO);
> - inc_zone_page_state(to->bv_page, NR_BOUNCE);
> + bounce_page = mempool_alloc(&page_pool, GFP_NOIO);
> + inc_zone_page_state(bounce_page, NR_BOUNCE);
>
> - if (rw == WRITE) {
> - char *vto, *vfrom;
> -
> - flush_dcache_page(page);
> -
> - vto = page_address(to->bv_page) + to->bv_offset;
> - vfrom = kmap_atomic(page) + to->bv_offset;
> - memcpy(vto, vfrom, to->bv_len);
> - kunmap_atomic(vfrom);
> - }
> + if (rw == WRITE)
> + memcpy_from_bvec(page_address(bounce_page), to);

NIT: the fact that the copy is from 'to' makes my head hurt... But I don't
see a good way to change that without declaring unnecessary variables... :-(

The logic seems right.

Ira

> + to->bv_page = bounce_page;
> }
>
> trace_block_bio_bounce(*bio_orig);
> --
> 2.30.2
>

2021-06-09 17:05:18

[permalink] [raw]

Subject: Re: switch the block layer to use kmap_local_page

On Tue, Jun 08, 2021 at 06:05:47PM +0200, Christoph Hellwig wrote:
> Hi all,
>
> this series switches the core block layer code and all users of the
> existing bvec kmap helpers to use kmap_local_page. Drivers that
> currently use open coded kmap_atomic calls will converted in a follow
> on series.

Other than the missing flush_dcache's.

For the series.

Reviewed-by: Ira Weiny <[email protected]>

>
> Diffstat:
> arch/mips/include/asm/mach-rc32434/rb.h | 2 -
> block/bio-integrity.c | 14 ++++------
> block/bio.c | 37 +++++++---------------------
> block/blk-map.c | 2 -
> block/bounce.c | 35 ++++++--------------------
> block/t10-pi.c | 16 ++++--------
> drivers/block/ps3disk.c | 19 ++------------
> drivers/block/rbd.c | 15 +----------
> drivers/md/dm-writecache.c | 5 +--
> include/linux/bio.h | 42 --------------------------------
> include/linux/bvec.h | 27 ++++++++++++++++++--
> include/linux/highmem.h | 4 +--
> 12 files changed, 64 insertions(+), 154 deletions(-)

2021-06-09 17:15:24

by Ilya Dryomov

[permalink] [raw]

Subject: Re: [PATCH 04/16] bvec: add a bvec_kmap_local helper

On Tue, Jun 8, 2021 at 6:06 PM Christoph Hellwig <[email protected]> wrote:
>
> Add a helper to call kmap_local_page on a bvec. There is no need for
> an unmap helper given that kunmap_local accept any address in the mapped
> page.
>
> Signed-off-by: Christoph Hellwig <[email protected]>
> ---
> include/linux/bvec.h | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/include/linux/bvec.h b/include/linux/bvec.h
> index 883faf5f1523..d64d6c0ceb77 100644
> --- a/include/linux/bvec.h
> +++ b/include/linux/bvec.h
> @@ -7,6 +7,7 @@
> #ifndef __LINUX_BVEC_H
> #define __LINUX_BVEC_H
>
> +#include <linux/highmem.h>
> #include <linux/bug.h>
> #include <linux/errno.h>
> #include <linux/limits.h>
> @@ -183,4 +184,9 @@ static inline void bvec_advance(const struct bio_vec *bvec,
> }
> }
>
> +static inline void *bvec_kmap_local(struct bio_vec *bvec)
> +{
> + return kmap_local_page(bvec->bv_page) + bvec->bv_offset;
> +}
> +
> #endif /* __LINUX_BVEC_H */

Might be useful to add the second sentence of the commit message as
a comment for bvec_kmap_local(). It could be expanded to mention the
single-page bvec caveat too.

Thanks,

Ilya

2021-06-09 17:15:40

by Ilya Dryomov

[permalink] [raw]

Subject: Re: [PATCH 07/16] rbd: use memzero_bvec

On Tue, Jun 8, 2021 at 6:06 PM Christoph Hellwig <[email protected]> wrote:
>
> Use memzero_bvec instead of reimplementing it.
>
> Signed-off-by: Christoph Hellwig <[email protected]>
> ---
> drivers/block/rbd.c | 15 ++-------------
> 1 file changed, 2 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index bbb88eb009e0..eb243fc4d108 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -1219,24 +1219,13 @@ static void rbd_dev_mapping_clear(struct rbd_device *rbd_dev)
> rbd_dev->mapping.size = 0;
> }
>
> -static void zero_bvec(struct bio_vec *bv)
> -{
> - void *buf;
> - unsigned long flags;
> -
> - buf = bvec_kmap_irq(bv, &flags);
> - memset(buf, 0, bv->bv_len);
> - flush_dcache_page(bv->bv_page);
> - bvec_kunmap_irq(buf, &flags);
> -}
> -
> static void zero_bios(struct ceph_bio_iter *bio_pos, u32 off, u32 bytes)
> {
> struct ceph_bio_iter it = *bio_pos;
>
> ceph_bio_iter_advance(&it, off);
> ceph_bio_iter_advance_step(&it, bytes, ({
> - zero_bvec(&bv);
> + memzero_bvec(&bv);
> }));
> }
>
> @@ -1246,7 +1235,7 @@ static void zero_bvecs(struct ceph_bvec_iter *bvec_pos, u32 off, u32 bytes)
>
> ceph_bvec_iter_advance(&it, off);
> ceph_bvec_iter_advance_step(&it, bytes, ({
> - zero_bvec(&bv);
> + memzero_bvec(&bv);
> }));
> }
>

Ira already brought up the fact that this conversion drops
flush_dcache_page() calls throughout. Other than that:

Acked-by: Ilya Dryomov <[email protected]>

Thanks,

Ilya

2021-06-11 06:56:06

[permalink] [raw]

Subject: Re: [PATCH 09/16] ps3disk: use memcpy_{from,to}_bvec

On Tue, Jun 08, 2021 at 06:48:22PM -0700, Ira Weiny wrote:
> I'm still not 100% sure that these flushes are needed but the are not no-ops on
> every arch. Would it be best to preserve them after the memcpy_to/from_bvec()?
>
> Same thing in patch 11 and 14.

To me it seems kunmap_local should basically always call the equivalent
of flush_kernel_dcache_page. parisc does this through
kunmap_flush_on_unmap, but none of the other architectures with VIVT
caches or other coherency issues does.

Does anyone have a history or other insights here?

2021-06-12 04:09:55