linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* switch the block layer to use kmap_local_page
@ 2021-06-08 16:05 Christoph Hellwig
  2021-06-08 16:05 ` [PATCH 01/16] mm: use kmap_local_page in memzero_page Christoph Hellwig
                   ` (16 more replies)
  0 siblings, 17 replies; 36+ messages in thread
From: Christoph Hellwig @ 2021-06-08 16:05 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Thomas Bogendoerfer, Geoff Levand, Ilya Dryomov, Dongsheng Yang,
	Mike Snitzer, Ira Weiny, dm-devel, linux-mips, linux-kernel,
	linux-block, linuxppc-dev, ceph-devel

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.

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(-)

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

* [PATCH 01/16] mm: use kmap_local_page in memzero_page
  2021-06-08 16:05 switch the block layer to use kmap_local_page Christoph Hellwig
@ 2021-06-08 16:05 ` Christoph Hellwig
  2021-06-08 18:17   ` Chaitanya Kulkarni
  2021-06-08 16:05 ` [PATCH 02/16] MIPS: don't include <linux/genhd.h> in <asm/mach-rc32434/rb.h> Christoph Hellwig
                   ` (15 subsequent siblings)
  16 siblings, 1 reply; 36+ messages in thread
From: Christoph Hellwig @ 2021-06-08 16:05 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Thomas Bogendoerfer, Geoff Levand, Ilya Dryomov, Dongsheng Yang,
	Mike Snitzer, Ira Weiny, dm-devel, linux-mips, linux-kernel,
	linux-block, linuxppc-dev, ceph-devel

No need for kmap_atomic here.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 include/linux/highmem.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/highmem.h b/include/linux/highmem.h
index 832b49b50c7b..0dc0451cf1d1 100644
--- a/include/linux/highmem.h
+++ b/include/linux/highmem.h
@@ -334,9 +334,9 @@ static inline void memcpy_to_page(struct page *page, size_t offset,
 
 static inline void memzero_page(struct page *page, size_t offset, size_t len)
 {
-	char *addr = kmap_atomic(page);
+	char *addr = kmap_local_page(page);
 	memset(addr + offset, 0, len);
-	kunmap_atomic(addr);
+	kunmap_local(addr);
 }
 
 #endif /* _LINUX_HIGHMEM_H */
-- 
2.30.2


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

* [PATCH 02/16] MIPS: don't include <linux/genhd.h> in <asm/mach-rc32434/rb.h>
  2021-06-08 16:05 switch the block layer to use kmap_local_page Christoph Hellwig
  2021-06-08 16:05 ` [PATCH 01/16] mm: use kmap_local_page in memzero_page Christoph Hellwig
@ 2021-06-08 16:05 ` Christoph Hellwig
  2021-06-08 16:23   ` Bart Van Assche
  2021-06-08 16:05 ` [PATCH 03/16] bvec: fix the include guards for bvec.h Christoph Hellwig
                   ` (14 subsequent siblings)
  16 siblings, 1 reply; 36+ messages in thread
From: Christoph Hellwig @ 2021-06-08 16:05 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Thomas Bogendoerfer, Geoff Levand, Ilya Dryomov, Dongsheng Yang,
	Mike Snitzer, Ira Weiny, dm-devel, linux-mips, linux-kernel,
	linux-block, linuxppc-dev, ceph-devel

There is no need to include genhd.h from a random arch header, and not
doing so prevents the possibility for nasty include loops.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/mips/include/asm/mach-rc32434/rb.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/mips/include/asm/mach-rc32434/rb.h b/arch/mips/include/asm/mach-rc32434/rb.h
index d502673a4f6c..34d179ca020b 100644
--- a/arch/mips/include/asm/mach-rc32434/rb.h
+++ b/arch/mips/include/asm/mach-rc32434/rb.h
@@ -7,8 +7,6 @@
 #ifndef __ASM_RC32434_RB_H
 #define __ASM_RC32434_RB_H
 
-#include <linux/genhd.h>
-
 #define REGBASE		0x18000000
 #define IDT434_REG_BASE ((volatile void *) KSEG1ADDR(REGBASE))
 #define UART0BASE	0x58000
-- 
2.30.2


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

* [PATCH 03/16] bvec: fix the include guards for bvec.h
  2021-06-08 16:05 switch the block layer to use kmap_local_page Christoph Hellwig
  2021-06-08 16:05 ` [PATCH 01/16] mm: use kmap_local_page in memzero_page Christoph Hellwig
  2021-06-08 16:05 ` [PATCH 02/16] MIPS: don't include <linux/genhd.h> in <asm/mach-rc32434/rb.h> Christoph Hellwig
@ 2021-06-08 16:05 ` Christoph Hellwig
  2021-06-08 16:23   ` Bart Van Assche
  2021-06-08 18:18   ` Chaitanya Kulkarni
  2021-06-08 16:05 ` [PATCH 04/16] bvec: add a bvec_kmap_local helper Christoph Hellwig
                   ` (13 subsequent siblings)
  16 siblings, 2 replies; 36+ messages in thread
From: Christoph Hellwig @ 2021-06-08 16:05 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Thomas Bogendoerfer, Geoff Levand, Ilya Dryomov, Dongsheng Yang,
	Mike Snitzer, Ira Weiny, dm-devel, linux-mips, linux-kernel,
	linux-block, linuxppc-dev, ceph-devel

Fix the include guards to match the file naming.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 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 <ming.lei@canonical.com>
  */
-#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


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

* [PATCH 04/16] bvec: add a bvec_kmap_local helper
  2021-06-08 16:05 switch the block layer to use kmap_local_page Christoph Hellwig
                   ` (2 preceding siblings ...)
  2021-06-08 16:05 ` [PATCH 03/16] bvec: fix the include guards for bvec.h Christoph Hellwig
@ 2021-06-08 16:05 ` Christoph Hellwig
  2021-06-08 18:18   ` Chaitanya Kulkarni
  2021-06-09  9:33   ` Ilya Dryomov
  2021-06-08 16:05 ` [PATCH 05/16] bvec: add memcpy_{from,to}_bvec and memzero_bvec helper Christoph Hellwig
                   ` (12 subsequent siblings)
  16 siblings, 2 replies; 36+ messages in thread
From: Christoph Hellwig @ 2021-06-08 16:05 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Thomas Bogendoerfer, Geoff Levand, Ilya Dryomov, Dongsheng Yang,
	Mike Snitzer, Ira Weiny, dm-devel, linux-mips, linux-kernel,
	linux-block, linuxppc-dev, ceph-devel

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 <hch@lst.de>
---
 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


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

* [PATCH 05/16] bvec: add memcpy_{from,to}_bvec and memzero_bvec helper
  2021-06-08 16:05 switch the block layer to use kmap_local_page Christoph Hellwig
                   ` (3 preceding siblings ...)
  2021-06-08 16:05 ` [PATCH 04/16] bvec: add a bvec_kmap_local helper Christoph Hellwig
@ 2021-06-08 16:05 ` Christoph Hellwig
  2021-06-08 18:21   ` Chaitanya Kulkarni
  2021-06-08 16:05 ` [PATCH 06/16] block: use memzero_page in zero_fill_bio Christoph Hellwig
                   ` (11 subsequent siblings)
  16 siblings, 1 reply; 36+ messages in thread
From: Christoph Hellwig @ 2021-06-08 16:05 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Thomas Bogendoerfer, Geoff Levand, Ilya Dryomov, Dongsheng Yang,
	Mike Snitzer, Ira Weiny, dm-devel, linux-mips, linux-kernel,
	linux-block, linuxppc-dev, ceph-devel

Add helpers to perform common memory operation on a bvec.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 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


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

* [PATCH 06/16] block: use memzero_page in zero_fill_bio
  2021-06-08 16:05 switch the block layer to use kmap_local_page Christoph Hellwig
                   ` (4 preceding siblings ...)
  2021-06-08 16:05 ` [PATCH 05/16] bvec: add memcpy_{from,to}_bvec and memzero_bvec helper Christoph Hellwig
@ 2021-06-08 16:05 ` Christoph Hellwig
  2021-06-08 18:19   ` Chaitanya Kulkarni
  2021-06-08 16:05 ` [PATCH 07/16] rbd: use memzero_bvec Christoph Hellwig
                   ` (10 subsequent siblings)
  16 siblings, 1 reply; 36+ messages in thread
From: Christoph Hellwig @ 2021-06-08 16:05 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Thomas Bogendoerfer, Geoff Levand, Ilya Dryomov, Dongsheng Yang,
	Mike Snitzer, Ira Weiny, dm-devel, linux-mips, linux-kernel,
	linux-block, linuxppc-dev, ceph-devel

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

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 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


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

* [PATCH 07/16] rbd: use memzero_bvec
  2021-06-08 16:05 switch the block layer to use kmap_local_page Christoph Hellwig
                   ` (5 preceding siblings ...)
  2021-06-08 16:05 ` [PATCH 06/16] block: use memzero_page in zero_fill_bio Christoph Hellwig
@ 2021-06-08 16:05 ` Christoph Hellwig
  2021-06-09  9:37   ` Ilya Dryomov
  2021-06-08 16:05 ` [PATCH 08/16] dm-writecache: use bvec_kmap_local instead of bvec_kmap_irq Christoph Hellwig
                   ` (9 subsequent siblings)
  16 siblings, 1 reply; 36+ messages in thread
From: Christoph Hellwig @ 2021-06-08 16:05 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Thomas Bogendoerfer, Geoff Levand, Ilya Dryomov, Dongsheng Yang,
	Mike Snitzer, Ira Weiny, dm-devel, linux-mips, linux-kernel,
	linux-block, linuxppc-dev, ceph-devel

Use memzero_bvec instead of reimplementing it.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 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


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

* [PATCH 08/16] dm-writecache: use bvec_kmap_local instead of bvec_kmap_irq
  2021-06-08 16:05 switch the block layer to use kmap_local_page Christoph Hellwig
                   ` (6 preceding siblings ...)
  2021-06-08 16:05 ` [PATCH 07/16] rbd: use memzero_bvec Christoph Hellwig
@ 2021-06-08 16:05 ` Christoph Hellwig
  2021-06-08 16:30   ` Bart Van Assche
  2021-06-08 16:05 ` [PATCH 09/16] ps3disk: use memcpy_{from,to}_bvec Christoph Hellwig
                   ` (8 subsequent siblings)
  16 siblings, 1 reply; 36+ messages in thread
From: Christoph Hellwig @ 2021-06-08 16:05 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Thomas Bogendoerfer, Geoff Levand, Ilya Dryomov, Dongsheng Yang,
	Mike Snitzer, Ira Weiny, dm-devel, linux-mips, linux-kernel,
	linux-block, linuxppc-dev, ceph-devel

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 <hch@lst.de>
---
 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


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

* [PATCH 09/16] ps3disk: use memcpy_{from,to}_bvec
  2021-06-08 16:05 switch the block layer to use kmap_local_page Christoph Hellwig
                   ` (7 preceding siblings ...)
  2021-06-08 16:05 ` [PATCH 08/16] dm-writecache: use bvec_kmap_local instead of bvec_kmap_irq Christoph Hellwig
@ 2021-06-08 16:05 ` Christoph Hellwig
  2021-06-09  1:48   ` Ira Weiny
  2021-06-08 16:05 ` [PATCH 10/16] block: remove bvec_kmap_irq and bvec_kunmap_irq Christoph Hellwig
                   ` (7 subsequent siblings)
  16 siblings, 1 reply; 36+ messages in thread
From: Christoph Hellwig @ 2021-06-08 16:05 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Thomas Bogendoerfer, Geoff Levand, Ilya Dryomov, Dongsheng Yang,
	Mike Snitzer, Ira Weiny, dm-devel, linux-mips, linux-kernel,
	linux-block, linuxppc-dev, ceph-devel

Use the bvec helpers instead of open coding the copy.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 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


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

* [PATCH 10/16] block: remove bvec_kmap_irq and bvec_kunmap_irq
  2021-06-08 16:05 switch the block layer to use kmap_local_page Christoph Hellwig
                   ` (8 preceding siblings ...)
  2021-06-08 16:05 ` [PATCH 09/16] ps3disk: use memcpy_{from,to}_bvec Christoph Hellwig
@ 2021-06-08 16:05 ` Christoph Hellwig
  2021-06-08 16:05 ` [PATCH 11/16] block: rewrite bio_copy_data_iter to use bvec_kmap_local and memcpy_to_bvec Christoph Hellwig
                   ` (6 subsequent siblings)
  16 siblings, 0 replies; 36+ messages in thread
From: Christoph Hellwig @ 2021-06-08 16:05 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Thomas Bogendoerfer, Geoff Levand, Ilya Dryomov, Dongsheng Yang,
	Mike Snitzer, Ira Weiny, dm-devel, linux-mips, linux-kernel,
	linux-block, linuxppc-dev, ceph-devel

These two helpers are entirely unused now.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 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


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

* [PATCH 11/16] block: rewrite bio_copy_data_iter to use bvec_kmap_local and memcpy_to_bvec
  2021-06-08 16:05 switch the block layer to use kmap_local_page Christoph Hellwig
                   ` (9 preceding siblings ...)
  2021-06-08 16:05 ` [PATCH 10/16] block: remove bvec_kmap_irq and bvec_kunmap_irq Christoph Hellwig
@ 2021-06-08 16:05 ` Christoph Hellwig
  2021-06-08 16:05 ` [PATCH 12/16] block: use memcpy_to_bvec in copy_to_high_bio_irq Christoph Hellwig
                   ` (5 subsequent siblings)
  16 siblings, 0 replies; 36+ messages in thread
From: Christoph Hellwig @ 2021-06-08 16:05 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Thomas Bogendoerfer, Geoff Levand, Ilya Dryomov, Dongsheng Yang,
	Mike Snitzer, Ira Weiny, dm-devel, linux-mips, linux-kernel,
	linux-block, linuxppc-dev, ceph-devel

Use the proper helpers instead of open coding the copy.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 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


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

* [PATCH 12/16] block: use memcpy_to_bvec in copy_to_high_bio_irq
  2021-06-08 16:05 switch the block layer to use kmap_local_page Christoph Hellwig
                   ` (10 preceding siblings ...)
  2021-06-08 16:05 ` [PATCH 11/16] block: rewrite bio_copy_data_iter to use bvec_kmap_local and memcpy_to_bvec Christoph Hellwig
@ 2021-06-08 16:05 ` Christoph Hellwig
  2021-06-08 18:24   ` Chaitanya Kulkarni
  2021-06-08 16:06 ` [PATCH 13/16] block: use memcpy_from_bvec in bio_copy_kern_endio_read Christoph Hellwig
                   ` (4 subsequent siblings)
  16 siblings, 1 reply; 36+ messages in thread
From: Christoph Hellwig @ 2021-06-08 16:05 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Thomas Bogendoerfer, Geoff Levand, Ilya Dryomov, Dongsheng Yang,
	Mike Snitzer, Ira Weiny, dm-devel, linux-mips, linux-kernel,
	linux-block, linuxppc-dev, ceph-devel

Use memcpy_to_bvec instead of opencoding the logic.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 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


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

* [PATCH 13/16] block: use memcpy_from_bvec in bio_copy_kern_endio_read
  2021-06-08 16:05 switch the block layer to use kmap_local_page Christoph Hellwig
                   ` (11 preceding siblings ...)
  2021-06-08 16:05 ` [PATCH 12/16] block: use memcpy_to_bvec in copy_to_high_bio_irq Christoph Hellwig
@ 2021-06-08 16:06 ` Christoph Hellwig
  2021-06-08 18:26   ` Chaitanya Kulkarni
  2021-06-08 16:06 ` [PATCH 14/16] block: use memcpy_from_bvec in __blk_queue_bounce Christoph Hellwig
                   ` (3 subsequent siblings)
  16 siblings, 1 reply; 36+ messages in thread
From: Christoph Hellwig @ 2021-06-08 16:06 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Thomas Bogendoerfer, Geoff Levand, Ilya Dryomov, Dongsheng Yang,
	Mike Snitzer, Ira Weiny, dm-devel, linux-mips, linux-kernel,
	linux-block, linuxppc-dev, ceph-devel

Use memcpy_from_bvec instead of open coding the logic.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 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


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

* [PATCH 14/16] block: use memcpy_from_bvec in __blk_queue_bounce
  2021-06-08 16:05 switch the block layer to use kmap_local_page Christoph Hellwig
                   ` (12 preceding siblings ...)
  2021-06-08 16:06 ` [PATCH 13/16] block: use memcpy_from_bvec in bio_copy_kern_endio_read Christoph Hellwig
@ 2021-06-08 16:06 ` Christoph Hellwig
  2021-06-09  1:58   ` Ira Weiny
  2021-06-08 16:06 ` [PATCH 15/16] block: use bvec_kmap_local in t10_pi_type1_{prepare,complete} Christoph Hellwig
                   ` (2 subsequent siblings)
  16 siblings, 1 reply; 36+ messages in thread
From: Christoph Hellwig @ 2021-06-08 16:06 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Thomas Bogendoerfer, Geoff Levand, Ilya Dryomov, Dongsheng Yang,
	Mike Snitzer, Ira Weiny, dm-devel, linux-mips, linux-kernel,
	linux-block, linuxppc-dev, ceph-devel

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 <hch@lst.de>
---
 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


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

* [PATCH 15/16] block: use bvec_kmap_local in t10_pi_type1_{prepare,complete}
  2021-06-08 16:05 switch the block layer to use kmap_local_page Christoph Hellwig
                   ` (13 preceding siblings ...)
  2021-06-08 16:06 ` [PATCH 14/16] block: use memcpy_from_bvec in __blk_queue_bounce Christoph Hellwig
@ 2021-06-08 16:06 ` Christoph Hellwig
  2021-06-08 16:06 ` [PATCH 16/16] block: use bvec_kmap_local in bio_integrity_process Christoph Hellwig
  2021-06-09  1:59 ` switch the block layer to use kmap_local_page Ira Weiny
  16 siblings, 0 replies; 36+ messages in thread
From: Christoph Hellwig @ 2021-06-08 16:06 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Thomas Bogendoerfer, Geoff Levand, Ilya Dryomov, Dongsheng Yang,
	Mike Snitzer, Ira Weiny, dm-devel, linux-mips, linux-kernel,
	linux-block, linuxppc-dev, ceph-devel

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>
---
 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


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

* [PATCH 16/16] block: use bvec_kmap_local in bio_integrity_process
  2021-06-08 16:05 switch the block layer to use kmap_local_page Christoph Hellwig
                   ` (14 preceding siblings ...)
  2021-06-08 16:06 ` [PATCH 15/16] block: use bvec_kmap_local in t10_pi_type1_{prepare,complete} Christoph Hellwig
@ 2021-06-08 16:06 ` Christoph Hellwig
  2021-06-09  1:59 ` switch the block layer to use kmap_local_page Ira Weiny
  16 siblings, 0 replies; 36+ messages in thread
From: Christoph Hellwig @ 2021-06-08 16:06 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Thomas Bogendoerfer, Geoff Levand, Ilya Dryomov, Dongsheng Yang,
	Mike Snitzer, Ira Weiny, dm-devel, linux-mips, linux-kernel,
	linux-block, linuxppc-dev, ceph-devel

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>
---
 block/bio-integrity.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index 4b4eb8964a6f..8f54d49dc500 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -172,18 +172,16 @@ static blk_status_t bio_integrity_process(struct bio *bio,
 	iter.prot_buf = prot_buf;
 
 	__bio_for_each_segment(bv, bio, bviter, *proc_iter) {
-		void *kaddr = kmap_atomic(bv.bv_page);
+		void *kaddr = bvec_kmap_local(&bv);
 
-		iter.data_buf = kaddr + bv.bv_offset;
+		iter.data_buf = kaddr;
 		iter.data_size = bv.bv_len;
-
 		ret = proc_fn(&iter);
-		if (ret) {
-			kunmap_atomic(kaddr);
-			return ret;
-		}
+		kunmap_local(kaddr);
+
+		if (ret)
+			break;
 
-		kunmap_atomic(kaddr);
 	}
 	return ret;
 }
-- 
2.30.2


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

* Re: [PATCH 02/16] MIPS: don't include <linux/genhd.h> in <asm/mach-rc32434/rb.h>
  2021-06-08 16:05 ` [PATCH 02/16] MIPS: don't include <linux/genhd.h> in <asm/mach-rc32434/rb.h> Christoph Hellwig
@ 2021-06-08 16:23   ` Bart Van Assche
  0 siblings, 0 replies; 36+ messages in thread
From: Bart Van Assche @ 2021-06-08 16:23 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe
  Cc: Thomas Bogendoerfer, Geoff Levand, Ilya Dryomov, Dongsheng Yang,
	Mike Snitzer, Ira Weiny, dm-devel, linux-mips, linux-kernel,
	linux-block, linuxppc-dev, ceph-devel

On 6/8/21 9:05 AM, Christoph Hellwig wrote:
> There is no need to include genhd.h from a random arch header, and not
> doing so prevents the possibility for nasty include loops.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  arch/mips/include/asm/mach-rc32434/rb.h | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/arch/mips/include/asm/mach-rc32434/rb.h b/arch/mips/include/asm/mach-rc32434/rb.h
> index d502673a4f6c..34d179ca020b 100644
> --- a/arch/mips/include/asm/mach-rc32434/rb.h
> +++ b/arch/mips/include/asm/mach-rc32434/rb.h
> @@ -7,8 +7,6 @@
>  #ifndef __ASM_RC32434_RB_H
>  #define __ASM_RC32434_RB_H
>  
> -#include <linux/genhd.h>
> -
>  #define REGBASE		0x18000000
>  #define IDT434_REG_BASE ((volatile void *) KSEG1ADDR(REGBASE))
>  #define UART0BASE	0x58000

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

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

* Re: [PATCH 03/16] bvec: fix the include guards for bvec.h
  2021-06-08 16:05 ` [PATCH 03/16] bvec: fix the include guards for bvec.h Christoph Hellwig
@ 2021-06-08 16:23   ` Bart Van Assche
  2021-06-08 18:18   ` Chaitanya Kulkarni
  1 sibling, 0 replies; 36+ messages in thread
From: Bart Van Assche @ 2021-06-08 16:23 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe
  Cc: Thomas Bogendoerfer, Geoff Levand, Ilya Dryomov, Dongsheng Yang,
	Mike Snitzer, Ira Weiny, dm-devel, linux-mips, linux-kernel,
	linux-block, linuxppc-dev, ceph-devel

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

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

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

* Re: [PATCH 08/16] dm-writecache: use bvec_kmap_local instead of bvec_kmap_irq
  2021-06-08 16:05 ` [PATCH 08/16] dm-writecache: use bvec_kmap_local instead of bvec_kmap_irq Christoph Hellwig
@ 2021-06-08 16:30   ` Bart Van Assche
  2021-06-08 16:38     ` Christoph Hellwig
  0 siblings, 1 reply; 36+ messages in thread
From: Bart Van Assche @ 2021-06-08 16:30 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe
  Cc: Thomas Bogendoerfer, Geoff Levand, Ilya Dryomov, Dongsheng Yang,
	Mike Snitzer, Ira Weiny, dm-devel, linux-mips, linux-kernel,
	linux-block, linuxppc-dev, ceph-devel

On 6/8/21 9:05 AM, Christoph Hellwig wrote:
> 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;

From one of the functions called by kunmap_local():

unsigned long addr = (unsigned long) vaddr & PAGE_MASK;

This won't work well if bvec->bv_offset >= PAGE_SIZE I assume?

Thanks,

Bart.

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

* Re: [PATCH 08/16] dm-writecache: use bvec_kmap_local instead of bvec_kmap_irq
  2021-06-08 16:30   ` Bart Van Assche
@ 2021-06-08 16:38     ` Christoph Hellwig
  0 siblings, 0 replies; 36+ messages in thread
From: Christoph Hellwig @ 2021-06-08 16:38 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Christoph Hellwig, Jens Axboe, Thomas Bogendoerfer, Geoff Levand,
	Ilya Dryomov, Dongsheng Yang, Mike Snitzer, Ira Weiny, dm-devel,
	linux-mips, linux-kernel, linux-block, linuxppc-dev, ceph-devel

On Tue, Jun 08, 2021 at 09:30:56AM -0700, Bart Van Assche wrote:
> >From one of the functions called by kunmap_local():
> 
> unsigned long addr = (unsigned long) vaddr & PAGE_MASK;
> 
> This won't work well if bvec->bv_offset >= PAGE_SIZE I assume?

It won't indeed.  Both the existing and new helpers operate on single
page bvecs only, and all callers only use those.  I should have
probably mentioned that in the cover letter and documented the
assumptions in the code, though.

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

* Re: [PATCH 01/16] mm: use kmap_local_page in memzero_page
  2021-06-08 16:05 ` [PATCH 01/16] mm: use kmap_local_page in memzero_page Christoph Hellwig
@ 2021-06-08 18:17   ` Chaitanya Kulkarni
  0 siblings, 0 replies; 36+ messages in thread
From: Chaitanya Kulkarni @ 2021-06-08 18:17 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe
  Cc: Thomas Bogendoerfer, Geoff Levand, Ilya Dryomov, Dongsheng Yang,
	Mike Snitzer, Ira Weiny, dm-devel, linux-mips, linux-kernel,
	linux-block, linuxppc-dev, ceph-devel

On 6/8/21 09:06, Christoph Hellwig wrote:
> No need for kmap_atomic here.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good.

Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>




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

* Re: [PATCH 03/16] bvec: fix the include guards for bvec.h
  2021-06-08 16:05 ` [PATCH 03/16] bvec: fix the include guards for bvec.h Christoph Hellwig
  2021-06-08 16:23   ` Bart Van Assche
@ 2021-06-08 18:18   ` Chaitanya Kulkarni
  1 sibling, 0 replies; 36+ messages in thread
From: Chaitanya Kulkarni @ 2021-06-08 18:18 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Jens Axboe, Thomas Bogendoerfer, Geoff Levand, Ilya Dryomov,
	Dongsheng Yang, Mike Snitzer, Ira Weiny, dm-devel, linux-mips,
	linux-kernel, linux-block, linuxppc-dev, ceph-devel

On 6/8/21 09:06, Christoph Hellwig wrote:
> Fix the include guards to match the file naming.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good.

Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>


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

* Re: [PATCH 04/16] bvec: add a bvec_kmap_local helper
  2021-06-08 16:05 ` [PATCH 04/16] bvec: add a bvec_kmap_local helper Christoph Hellwig
@ 2021-06-08 18:18   ` Chaitanya Kulkarni
  2021-06-09  9:33   ` Ilya Dryomov
  1 sibling, 0 replies; 36+ messages in thread
From: Chaitanya Kulkarni @ 2021-06-08 18:18 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe
  Cc: Thomas Bogendoerfer, Geoff Levand, Ilya Dryomov, Dongsheng Yang,
	Mike Snitzer, Ira Weiny, dm-devel, linux-mips, linux-kernel,
	linux-block, linuxppc-dev, ceph-devel

On 6/8/21 09:06, Christoph Hellwig 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 <hch@lst.de>

Looks good.

Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>



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

* Re: [PATCH 06/16] block: use memzero_page in zero_fill_bio
  2021-06-08 16:05 ` [PATCH 06/16] block: use memzero_page in zero_fill_bio Christoph Hellwig
@ 2021-06-08 18:19   ` Chaitanya Kulkarni
  0 siblings, 0 replies; 36+ messages in thread
From: Chaitanya Kulkarni @ 2021-06-08 18:19 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe
  Cc: Thomas Bogendoerfer, Geoff Levand, Ilya Dryomov, Dongsheng Yang,
	Mike Snitzer, Ira Weiny, dm-devel, linux-mips, linux-kernel,
	linux-block, linuxppc-dev, ceph-devel

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 <hch@lst.de>

Looks good.

Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>



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

* Re: [PATCH 05/16] bvec: add memcpy_{from,to}_bvec and memzero_bvec helper
  2021-06-08 16:05 ` [PATCH 05/16] bvec: add memcpy_{from,to}_bvec and memzero_bvec helper Christoph Hellwig
@ 2021-06-08 18:21   ` Chaitanya Kulkarni
  0 siblings, 0 replies; 36+ messages in thread
From: Chaitanya Kulkarni @ 2021-06-08 18:21 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe
  Cc: Thomas Bogendoerfer, Geoff Levand, Ilya Dryomov, Dongsheng Yang,
	Mike Snitzer, Ira Weiny, dm-devel, linux-mips, linux-kernel,
	linux-block, linuxppc-dev, ceph-devel

On 6/8/21 09:07, Christoph Hellwig wrote:
> Add helpers to perform common memory operation on a bvec.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good.

Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>



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

* Re: [PATCH 12/16] block: use memcpy_to_bvec in copy_to_high_bio_irq
  2021-06-08 16:05 ` [PATCH 12/16] block: use memcpy_to_bvec in copy_to_high_bio_irq Christoph Hellwig
@ 2021-06-08 18:24   ` Chaitanya Kulkarni
  0 siblings, 0 replies; 36+ messages in thread
From: Chaitanya Kulkarni @ 2021-06-08 18:24 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Jens Axboe, Thomas Bogendoerfer, Geoff Levand, Ilya Dryomov,
	Dongsheng Yang, Mike Snitzer, Ira Weiny, dm-devel, linux-mips,
	linux-kernel, linux-block, linuxppc-dev, ceph-devel

On 6/8/21 09:08, Christoph Hellwig wrote:
> Use memcpy_to_bvec instead of opencoding the logic.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good.

Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>


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

* Re: [PATCH 13/16] block: use memcpy_from_bvec in bio_copy_kern_endio_read
  2021-06-08 16:06 ` [PATCH 13/16] block: use memcpy_from_bvec in bio_copy_kern_endio_read Christoph Hellwig
@ 2021-06-08 18:26   ` Chaitanya Kulkarni
  0 siblings, 0 replies; 36+ messages in thread
From: Chaitanya Kulkarni @ 2021-06-08 18:26 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe
  Cc: Thomas Bogendoerfer, Geoff Levand, Ilya Dryomov, Dongsheng Yang,
	Mike Snitzer, Ira Weiny, dm-devel, linux-mips, linux-kernel,
	linux-block, linuxppc-dev, ceph-devel

On 6/8/21 09:09, Christoph Hellwig wrote:
> Use memcpy_from_bvec instead of open coding the logic.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good.

Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>


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

* Re: [PATCH 09/16] ps3disk: use memcpy_{from,to}_bvec
  2021-06-08 16:05 ` [PATCH 09/16] ps3disk: use memcpy_{from,to}_bvec Christoph Hellwig
@ 2021-06-09  1:48   ` Ira Weiny
  2021-06-11  6:53     ` Christoph Hellwig
  0 siblings, 1 reply; 36+ messages in thread
From: Ira Weiny @ 2021-06-09  1:48 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Jens Axboe, Thomas Bogendoerfer, Geoff Levand, Ilya Dryomov,
	Dongsheng Yang, Mike Snitzer, dm-devel, linux-mips, linux-kernel,
	linux-block, linuxppc-dev, ceph-devel

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

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

* Re: [PATCH 14/16] block: use memcpy_from_bvec in __blk_queue_bounce
  2021-06-08 16:06 ` [PATCH 14/16] block: use memcpy_from_bvec in __blk_queue_bounce Christoph Hellwig
@ 2021-06-09  1:58   ` Ira Weiny
  0 siblings, 0 replies; 36+ messages in thread
From: Ira Weiny @ 2021-06-09  1:58 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Jens Axboe, Thomas Bogendoerfer, Geoff Levand, Ilya Dryomov,
	Dongsheng Yang, Mike Snitzer, dm-devel, linux-mips, linux-kernel,
	linux-block, linuxppc-dev, ceph-devel

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 <hch@lst.de>
> ---
>  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
> 

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

* Re: switch the block layer to use kmap_local_page
  2021-06-08 16:05 switch the block layer to use kmap_local_page Christoph Hellwig
                   ` (15 preceding siblings ...)
  2021-06-08 16:06 ` [PATCH 16/16] block: use bvec_kmap_local in bio_integrity_process Christoph Hellwig
@ 2021-06-09  1:59 ` Ira Weiny
  16 siblings, 0 replies; 36+ messages in thread
From: Ira Weiny @ 2021-06-09  1:59 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Jens Axboe, Thomas Bogendoerfer, Geoff Levand, Ilya Dryomov,
	Dongsheng Yang, Mike Snitzer, dm-devel, linux-mips, linux-kernel,
	linux-block, linuxppc-dev, ceph-devel

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 <ira.weiny@intel.com>

> 
> 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(-)

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

* Re: [PATCH 04/16] bvec: add a bvec_kmap_local helper
  2021-06-08 16:05 ` [PATCH 04/16] bvec: add a bvec_kmap_local helper Christoph Hellwig
  2021-06-08 18:18   ` Chaitanya Kulkarni
@ 2021-06-09  9:33   ` Ilya Dryomov
  1 sibling, 0 replies; 36+ messages in thread
From: Ilya Dryomov @ 2021-06-09  9:33 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Jens Axboe, Thomas Bogendoerfer, Geoff Levand, Dongsheng Yang,
	Mike Snitzer, Ira Weiny, dm-devel, linux-mips, LKML, linux-block,
	linuxppc-dev, Ceph Development

On Tue, Jun 8, 2021 at 6:06 PM Christoph Hellwig <hch@lst.de> 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 <hch@lst.de>
> ---
>  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

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

* Re: [PATCH 07/16] rbd: use memzero_bvec
  2021-06-08 16:05 ` [PATCH 07/16] rbd: use memzero_bvec Christoph Hellwig
@ 2021-06-09  9:37   ` Ilya Dryomov
  0 siblings, 0 replies; 36+ messages in thread
From: Ilya Dryomov @ 2021-06-09  9:37 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Jens Axboe, Thomas Bogendoerfer, Geoff Levand, Dongsheng Yang,
	Mike Snitzer, Ira Weiny, dm-devel, linux-mips, LKML, linux-block,
	linuxppc-dev, Ceph Development

On Tue, Jun 8, 2021 at 6:06 PM Christoph Hellwig <hch@lst.de> wrote:
>
> Use memzero_bvec instead of reimplementing it.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  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 <idryomov@gmail.com>

Thanks,

                Ilya

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

* Re: [PATCH 09/16] ps3disk: use memcpy_{from,to}_bvec
  2021-06-09  1:48   ` Ira Weiny
@ 2021-06-11  6:53     ` Christoph Hellwig
  2021-06-12  4:07       ` Ira Weiny
  0 siblings, 1 reply; 36+ messages in thread
From: Christoph Hellwig @ 2021-06-11  6:53 UTC (permalink / raw)
  To: Ira Weiny
  Cc: Christoph Hellwig, Jens Axboe, Thomas Bogendoerfer, Geoff Levand,
	Ilya Dryomov, Dongsheng Yang, Mike Snitzer, dm-devel, linux-mips,
	linux-kernel, linux-block, linuxppc-dev, ceph-devel,
	Thomas Gleixner, linux-arch

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?

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

* Re: [PATCH 09/16] ps3disk: use memcpy_{from,to}_bvec
  2021-06-11  6:53     ` Christoph Hellwig
@ 2021-06-12  4:07       ` Ira Weiny
  2021-06-15  5:02         ` Herbert Xu
  0 siblings, 1 reply; 36+ messages in thread
From: Ira Weiny @ 2021-06-12  4:07 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Jens Axboe, Thomas Bogendoerfer, Geoff Levand, Ilya Dryomov,
	Dongsheng Yang, Mike Snitzer, dm-devel, linux-mips, linux-kernel,
	linux-block, linuxppc-dev, ceph-devel, Thomas Gleixner,
	linux-arch, Tero Kristo, Herbert Xu, linux-arm-kernel,
	linux-csky, linux-sh, linux-mmc, linux-scsi

On Fri, Jun 11, 2021 at 08:53:38AM +0200, Christoph Hellwig wrote:
> 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?

I went digging into the current callers of flush_kernel_dcache_page() other
than this one.  To see if adding kunmap_flush_on_unmap() to the other arch's
would cause any problems.

In particular this call site stood out because it is not always called?!?!?!?

void sg_miter_stop(struct sg_mapping_iter *miter)
{
...
                if ((miter->__flags & SG_MITER_TO_SG) &&
                    !PageSlab(miter->page))
                        flush_kernel_dcache_page(miter->page);
...
}

Looking at 

3d77b50c5874 lib/scatterlist.c: don't flush_kernel_dcache_page on slab page[1]

It seems the restrictions they are quoting for the page are completely out of
date.  I don't see any current way for a VM_BUG_ON() to be triggered.  So is
this code really necessary?

More recently this was added:

7e34e0bbc644 crypto: omap-crypto - fix userspace copied buffer access

I'm CC'ing Tero and Herbert to see why they added the SLAB check.


Then we have interesting comments like this...

...
                /* This can go away once MIPS implements
		 * flush_kernel_dcache_page */
		flush_dcache_page(miter->page);
...


And some users optimizing.

...
		/* discard mappings */
		if (direction == DMA_FROM_DEVICE)
			flush_kernel_dcache_page(sg_page(sg));  
...

The uses in fs/exec.c are the most straight forward and can simply rely on the
kunmap() code to replace the call.

In conclusion I don't see a lot of reason to not define kunmap_flush_on_unmap()
on arm, csky, mips, nds32, and sh...  Then remove all the
flush_kernel_dcache_page() call sites and the documentation...

Something like [2] below...  Completely untested of course...

Ira


[1] commit 3d77b50c5874b7e923be946ba793644f82336b75
Author: Ming Lei <ming.lei@canonical.com>
Date:   Thu Oct 31 16:34:17 2013 -0700

    lib/scatterlist.c: don't flush_kernel_dcache_page on slab page
    
    Commit b1adaf65ba03 ("[SCSI] block: add sg buffer copy helper
    functions") introduces two sg buffer copy helpers, and calls
    flush_kernel_dcache_page() on pages in SG list after these pages are
    written to.
    
    Unfortunately, the commit may introduce a potential bug:
    
     - Before sending some SCSI commands, kmalloc() buffer may be passed to
       block layper, so flush_kernel_dcache_page() can see a slab page
       finally
    
     - According to cachetlb.txt, flush_kernel_dcache_page() is only called
       on "a user page", which surely can't be a slab page.
    
     - ARCH's implementation of flush_kernel_dcache_page() may use page
       mapping information to do optimization so page_mapping() will see the
       slab page, then VM_BUG_ON() is triggered.
    
    Aaro Koskinen reported the bug on ARM/kirkwood when DEBUG_VM is enabled,
    and this patch fixes the bug by adding test of '!PageSlab(miter->page)'
    before calling flush_kernel_dcache_page().


[2]


From 70b537c31d16c2a5e4e92c35895e8c59303bcbef Mon Sep 17 00:00:00 2001
From: Ira Weiny <ira.weiny@intel.com>
Date: Fri, 11 Jun 2021 18:24:27 -0700
Subject: [PATCH] COMPLETELY UNTESTED: highmem: Remove direct calls to flush_kernel_dcache_page

When to call flush_kernel_dcache_page() is confusing and inconsistent.  For
architectures which may need to do something the core kmap code should be
leveraged to handle this when direct kernel access is needed.

Like parisc define kunmap_flush_on_unmap() to be called when pages are
unmapped on arm, csky, mpis, nds32, and sh.

Remove all direct calls to flush_kernel_dcache_page() and let the
kunmap() code do this for the users.


Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-csky@vger.kernel.org
Cc: linux-mips@vger.kernel.org
Cc: linux-sh@vger.kernel.org
Cc: linux-crypto@vger.kernel.org
Cc: linux-mmc@vger.kernel.org
Cc: linux-scsi@vger.kernel.org
Cc: linux-fsdevel@vger.kernel.org
Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
 Documentation/core-api/cachetlb.rst  | 13 -------------
 arch/arm/include/asm/cacheflush.h    |  6 ++++++
 arch/csky/abiv1/inc/abi/cacheflush.h |  6 ++++++
 arch/mips/include/asm/cacheflush.h   |  6 ++++++
 arch/nds32/include/asm/cacheflush.h  |  6 ++++++
 arch/sh/include/asm/cacheflush.h     |  6 ++++++
 drivers/crypto/omap-crypto.c         |  3 ---
 drivers/mmc/host/mmc_spi.c           |  3 ---
 drivers/scsi/aacraid/aachba.c        |  1 -
 fs/exec.c                            |  3 ---
 include/linux/highmem.h              |  3 ---
 lib/scatterlist.c                    |  4 ----
 12 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/Documentation/core-api/cachetlb.rst b/Documentation/core-api/cachetlb.rst
index fe4290e26729..5c39de30e91f 100644
--- a/Documentation/core-api/cachetlb.rst
+++ b/Documentation/core-api/cachetlb.rst
@@ -351,19 +351,6 @@ maps this page at its virtual address.
 	architectures).  For incoherent architectures, it should flush
 	the cache of the page at vmaddr.
 
-  ``void flush_kernel_dcache_page(struct page *page)``
-
-	When the kernel needs to modify a user page is has obtained
-	with kmap, it calls this function after all modifications are
-	complete (but before kunmapping it) to bring the underlying
-	page up to date.  It is assumed here that the user has no
-	incoherent cached copies (i.e. the original page was obtained
-	from a mechanism like get_user_pages()).  The default
-	implementation is a nop and should remain so on all coherent
-	architectures.  On incoherent architectures, this should flush
-	the kernel cache for page (using page_address(page)).
-
-
   ``void flush_icache_range(unsigned long start, unsigned long end)``
 
   	When the kernel stores into addresses that it will execute
diff --git a/arch/arm/include/asm/cacheflush.h b/arch/arm/include/asm/cacheflush.h
index 2e24e765e6d3..1b7cb0af707f 100644
--- a/arch/arm/include/asm/cacheflush.h
+++ b/arch/arm/include/asm/cacheflush.h
@@ -315,6 +315,12 @@ static inline void flush_anon_page(struct vm_area_struct *vma,
 #define ARCH_HAS_FLUSH_KERNEL_DCACHE_PAGE
 extern void flush_kernel_dcache_page(struct page *);
 
+#define ARCH_HAS_FLUSH_ON_KUNMAP
+static inline void kunmap_flush_on_unmap(void *addr)
+{
+	flush_kernel_dcache_page_addr(addr);
+}
+
 #define flush_dcache_mmap_lock(mapping)		xa_lock_irq(&mapping->i_pages)
 #define flush_dcache_mmap_unlock(mapping)	xa_unlock_irq(&mapping->i_pages)
 
diff --git a/arch/csky/abiv1/inc/abi/cacheflush.h b/arch/csky/abiv1/inc/abi/cacheflush.h
index 6cab7afae962..e1ff554850f8 100644
--- a/arch/csky/abiv1/inc/abi/cacheflush.h
+++ b/arch/csky/abiv1/inc/abi/cacheflush.h
@@ -17,6 +17,12 @@ extern void flush_dcache_page(struct page *);
 #define ARCH_HAS_FLUSH_KERNEL_DCACHE_PAGE
 extern void flush_kernel_dcache_page(struct page *);
 
+#define ARCH_HAS_FLUSH_ON_KUNMAP
+static inline void kunmap_flush_on_unmap(void *addr)
+{
+	flush_kernel_dcache_page_addr(addr);
+}
+
 #define flush_dcache_mmap_lock(mapping)		xa_lock_irq(&mapping->i_pages)
 #define flush_dcache_mmap_unlock(mapping)	xa_unlock_irq(&mapping->i_pages)
 
diff --git a/arch/mips/include/asm/cacheflush.h b/arch/mips/include/asm/cacheflush.h
index d687b40b9fbb..c3043b600008 100644
--- a/arch/mips/include/asm/cacheflush.h
+++ b/arch/mips/include/asm/cacheflush.h
@@ -132,6 +132,12 @@ static inline void flush_kernel_dcache_page(struct page *page)
 	flush_dcache_page(page);
 }
 
+#define ARCH_HAS_FLUSH_ON_KUNMAP
+static inline void kunmap_flush_on_unmap(void *addr)
+{
+	flush_kernel_dcache_page_addr(addr);
+}
+
 /*
  * For now flush_kernel_vmap_range and invalidate_kernel_vmap_range both do a
  * cache writeback and invalidate operation.
diff --git a/arch/nds32/include/asm/cacheflush.h b/arch/nds32/include/asm/cacheflush.h
index 7d6824f7c0e8..bae980846e2a 100644
--- a/arch/nds32/include/asm/cacheflush.h
+++ b/arch/nds32/include/asm/cacheflush.h
@@ -43,6 +43,12 @@ void invalidate_kernel_vmap_range(void *addr, int size);
 #define flush_dcache_mmap_lock(mapping)   xa_lock_irq(&(mapping)->i_pages)
 #define flush_dcache_mmap_unlock(mapping) xa_unlock_irq(&(mapping)->i_pages)
 
+#define ARCH_HAS_FLUSH_ON_KUNMAP
+static inline void kunmap_flush_on_unmap(void *addr)
+{
+	flush_kernel_dcache_page_addr(addr);
+}
+
 #else
 void flush_icache_user_page(struct vm_area_struct *vma, struct page *page,
 	                     unsigned long addr, int len);
diff --git a/arch/sh/include/asm/cacheflush.h b/arch/sh/include/asm/cacheflush.h
index 4486a865ff62..2e23a8d71aa7 100644
--- a/arch/sh/include/asm/cacheflush.h
+++ b/arch/sh/include/asm/cacheflush.h
@@ -78,6 +78,12 @@ static inline void flush_kernel_dcache_page(struct page *page)
 	flush_dcache_page(page);
 }
 
+#define ARCH_HAS_FLUSH_ON_KUNMAP
+static inline void kunmap_flush_on_unmap(void *addr)
+{
+	flush_kernel_dcache_page_addr(addr);
+}
+
 extern void copy_to_user_page(struct vm_area_struct *vma,
 	struct page *page, unsigned long vaddr, void *dst, const void *src,
 	unsigned long len);
diff --git a/drivers/crypto/omap-crypto.c b/drivers/crypto/omap-crypto.c
index 94b2dba90f0d..cbc5a4151c3c 100644
--- a/drivers/crypto/omap-crypto.c
+++ b/drivers/crypto/omap-crypto.c
@@ -183,9 +183,6 @@ static void omap_crypto_copy_data(struct scatterlist *src,
 
 		memcpy(dstb, srcb, amt);
 
-		if (!PageSlab(sg_page(dst)))
-			flush_kernel_dcache_page(sg_page(dst));
-
 		kunmap_atomic(srcb);
 		kunmap_atomic(dstb);
 
diff --git a/drivers/mmc/host/mmc_spi.c b/drivers/mmc/host/mmc_spi.c
index 9776a03a10f5..e1aafbc6a0a1 100644
--- a/drivers/mmc/host/mmc_spi.c
+++ b/drivers/mmc/host/mmc_spi.c
@@ -947,9 +947,6 @@ mmc_spi_data_do(struct mmc_spi_host *host, struct mmc_command *cmd,
 				break;
 		}
 
-		/* discard mappings */
-		if (direction == DMA_FROM_DEVICE)
-			flush_kernel_dcache_page(sg_page(sg));
 		kunmap(sg_page(sg));
 		if (dma_dev)
 			dma_unmap_page(dma_dev, dma_addr, PAGE_SIZE, dir);
diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
index f1f62b5da8b7..8897d4ad78c6 100644
--- a/drivers/scsi/aacraid/aachba.c
+++ b/drivers/scsi/aacraid/aachba.c
@@ -25,7 +25,6 @@
 #include <linux/completion.h>
 #include <linux/blkdev.h>
 #include <linux/uaccess.h>
-#include <linux/highmem.h> /* For flush_kernel_dcache_page */
 #include <linux/module.h>
 
 #include <asm/unaligned.h>
diff --git a/fs/exec.c b/fs/exec.c
index 18594f11c31f..da9faa2da36b 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -577,7 +577,6 @@ static int copy_strings(int argc, struct user_arg_ptr argv,
 				}
 
 				if (kmapped_page) {
-					flush_kernel_dcache_page(kmapped_page);
 					kunmap(kmapped_page);
 					put_arg_page(kmapped_page);
 				}
@@ -595,7 +594,6 @@ static int copy_strings(int argc, struct user_arg_ptr argv,
 	ret = 0;
 out:
 	if (kmapped_page) {
-		flush_kernel_dcache_page(kmapped_page);
 		kunmap(kmapped_page);
 		put_arg_page(kmapped_page);
 	}
@@ -637,7 +635,6 @@ int copy_string_kernel(const char *arg, struct linux_binprm *bprm)
 		kaddr = kmap_atomic(page);
 		flush_arg_page(bprm, pos & PAGE_MASK, page);
 		memcpy(kaddr + offset_in_page(pos), arg, bytes_to_copy);
-		flush_kernel_dcache_page(page);
 		kunmap_atomic(kaddr);
 		put_arg_page(page);
 	}
diff --git a/include/linux/highmem.h b/include/linux/highmem.h
index 832b49b50c7b..7ef83bf52a6c 100644
--- a/include/linux/highmem.h
+++ b/include/linux/highmem.h
@@ -131,9 +131,6 @@ static inline void flush_anon_page(struct vm_area_struct *vma, struct page *page
 #endif
 
 #ifndef ARCH_HAS_FLUSH_KERNEL_DCACHE_PAGE
-static inline void flush_kernel_dcache_page(struct page *page)
-{
-}
 static inline void flush_kernel_vmap_range(void *vaddr, int size)
 {
 }
diff --git a/lib/scatterlist.c b/lib/scatterlist.c
index a59778946404..579b323a8042 100644
--- a/lib/scatterlist.c
+++ b/lib/scatterlist.c
@@ -887,10 +887,6 @@ void sg_miter_stop(struct sg_mapping_iter *miter)
 		miter->__offset += miter->consumed;
 		miter->__remaining -= miter->consumed;
 
-		if ((miter->__flags & SG_MITER_TO_SG) &&
-		    !PageSlab(miter->page))
-			flush_kernel_dcache_page(miter->page);
-
 		if (miter->__flags & SG_MITER_ATOMIC) {
 			WARN_ON_ONCE(preemptible());
 			kunmap_atomic(miter->addr);
-- 
2.28.0.rc0.12.gb6a658bd00c9


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

* Re: [PATCH 09/16] ps3disk: use memcpy_{from,to}_bvec
  2021-06-12  4:07       ` Ira Weiny
@ 2021-06-15  5:02         ` Herbert Xu
  0 siblings, 0 replies; 36+ messages in thread
From: Herbert Xu @ 2021-06-15  5:02 UTC (permalink / raw)
  To: Ira Weiny, David S. Miller
  Cc: Christoph Hellwig, Jens Axboe, Thomas Bogendoerfer, Geoff Levand,
	Ilya Dryomov, Dongsheng Yang, Mike Snitzer, dm-devel, linux-mips,
	linux-kernel, linux-block, linuxppc-dev, ceph-devel,
	Thomas Gleixner, linux-arch, Tero Kristo, linux-arm-kernel,
	linux-csky, linux-sh, linux-mmc, linux-scsi

On Fri, Jun 11, 2021 at 09:07:43PM -0700, Ira Weiny wrote:
>
> More recently this was added:
> 
> 7e34e0bbc644 crypto: omap-crypto - fix userspace copied buffer access
> 
> I'm CC'ing Tero and Herbert to see why they added the SLAB check.

Probably because the generic Crypto API has the same check.  This
all goes back to

commit 4f3e797ad07d52d34983354a77b365dfcd48c1b4
Author: Herbert Xu <herbert@gondor.apana.org.au>
Date:   Mon Feb 9 14:22:14 2009 +1100

    crypto: scatterwalk - Avoid flush_dcache_page on slab pages

    It's illegal to call flush_dcache_page on slab pages on a number
    of architectures.  So this patch avoids doing so if PageSlab is
    true.

    In future we can move the flush_dcache_page call to those page
    cache users that actually need it.

    Reported-by: David S. Miller <davem@davemloft.net>
    Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

But I can't find any emails discussing this so let me ask Dave
directly and see if he can tell us what the issue was or might
have been.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

end of thread, other threads:[~2021-06-15  5:04 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-08 16:05 switch the block layer to use kmap_local_page Christoph Hellwig
2021-06-08 16:05 ` [PATCH 01/16] mm: use kmap_local_page in memzero_page Christoph Hellwig
2021-06-08 18:17   ` Chaitanya Kulkarni
2021-06-08 16:05 ` [PATCH 02/16] MIPS: don't include <linux/genhd.h> in <asm/mach-rc32434/rb.h> Christoph Hellwig
2021-06-08 16:23   ` Bart Van Assche
2021-06-08 16:05 ` [PATCH 03/16] bvec: fix the include guards for bvec.h Christoph Hellwig
2021-06-08 16:23   ` Bart Van Assche
2021-06-08 18:18   ` Chaitanya Kulkarni
2021-06-08 16:05 ` [PATCH 04/16] bvec: add a bvec_kmap_local helper Christoph Hellwig
2021-06-08 18:18   ` Chaitanya Kulkarni
2021-06-09  9:33   ` Ilya Dryomov
2021-06-08 16:05 ` [PATCH 05/16] bvec: add memcpy_{from,to}_bvec and memzero_bvec helper Christoph Hellwig
2021-06-08 18:21   ` Chaitanya Kulkarni
2021-06-08 16:05 ` [PATCH 06/16] block: use memzero_page in zero_fill_bio Christoph Hellwig
2021-06-08 18:19   ` Chaitanya Kulkarni
2021-06-08 16:05 ` [PATCH 07/16] rbd: use memzero_bvec Christoph Hellwig
2021-06-09  9:37   ` Ilya Dryomov
2021-06-08 16:05 ` [PATCH 08/16] dm-writecache: use bvec_kmap_local instead of bvec_kmap_irq Christoph Hellwig
2021-06-08 16:30   ` Bart Van Assche
2021-06-08 16:38     ` Christoph Hellwig
2021-06-08 16:05 ` [PATCH 09/16] ps3disk: use memcpy_{from,to}_bvec Christoph Hellwig
2021-06-09  1:48   ` Ira Weiny
2021-06-11  6:53     ` Christoph Hellwig
2021-06-12  4:07       ` Ira Weiny
2021-06-15  5:02         ` Herbert Xu
2021-06-08 16:05 ` [PATCH 10/16] block: remove bvec_kmap_irq and bvec_kunmap_irq Christoph Hellwig
2021-06-08 16:05 ` [PATCH 11/16] block: rewrite bio_copy_data_iter to use bvec_kmap_local and memcpy_to_bvec Christoph Hellwig
2021-06-08 16:05 ` [PATCH 12/16] block: use memcpy_to_bvec in copy_to_high_bio_irq Christoph Hellwig
2021-06-08 18:24   ` Chaitanya Kulkarni
2021-06-08 16:06 ` [PATCH 13/16] block: use memcpy_from_bvec in bio_copy_kern_endio_read Christoph Hellwig
2021-06-08 18:26   ` Chaitanya Kulkarni
2021-06-08 16:06 ` [PATCH 14/16] block: use memcpy_from_bvec in __blk_queue_bounce Christoph Hellwig
2021-06-09  1:58   ` Ira Weiny
2021-06-08 16:06 ` [PATCH 15/16] block: use bvec_kmap_local in t10_pi_type1_{prepare,complete} Christoph Hellwig
2021-06-08 16:06 ` [PATCH 16/16] block: use bvec_kmap_local in bio_integrity_process Christoph Hellwig
2021-06-09  1:59 ` switch the block layer to use kmap_local_page Ira Weiny

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).