From: Ira Weiny <[email protected]>
Changes from V1:
Rework commit messages because they were very weak
Change 'fs/btrfs: X' to 'btrfs: x'
https://lore.kernel.org/lkml/[email protected]/
Per Andrew
Split out changes to highmem.h
The addition memcpy, memmove, and memset page functions
The change from kmap_atomic to kmap_local_page
The addition of BUG_ON
The conversion of memzero_page to zero_user in iov_iter
Change BUG_ON to VM_BUG_ON
While we are refactoring adjust the line length down per Chaitany
There are many places where kmap/<operation>/kunmap patterns occur. We lift a
couple of these patterns to the core common functions and use them as well as
existing core functions in the btrfs file system. At the same time we convert
those core functions to use kmap_local_page() which is more efficient in those
calls.
Per the conversation on V1 it looks like Andrew would like this to go through
the btrfs tree. I think that is fine. The other users of
memcpy_[to|from]_page are probably not ready and I believe could be taken in an
early rc after David submits.
Is that ok with you David?
Thanks,
Ira
Ira Weiny (8):
mm/highmem: Lift memcpy_[to|from]_page to core
mm/highmem: Convert memcpy_[to|from]_page() to kmap_local_page()
mm/highmem: Introduce memcpy_page(), memmove_page(), and memset_page()
mm/highmem: Add VM_BUG_ON() to mem*_page() calls
iov_iter: Remove memzero_page() in favor of zero_user()
btrfs: use memcpy_[to|from]_page() and kmap_local_page()
btrfs: use copy_highpage() instead of 2 kmaps()
btrfs: convert to zero_user()
fs/btrfs/compression.c | 11 +++-----
fs/btrfs/extent_io.c | 22 +++-------------
fs/btrfs/inode.c | 33 ++++++++----------------
fs/btrfs/lzo.c | 4 +--
fs/btrfs/raid56.c | 10 +-------
fs/btrfs/reflink.c | 12 ++-------
fs/btrfs/send.c | 7 ++----
fs/btrfs/zlib.c | 10 +++-----
fs/btrfs/zstd.c | 11 +++-----
include/linux/highmem.h | 56 +++++++++++++++++++++++++++++++++++++++++
lib/iov_iter.c | 26 +++----------------
11 files changed, 89 insertions(+), 113 deletions(-)
--
2.28.0.rc0.12.gb6a658bd00c9
From: Ira Weiny <[email protected]>
There are many places where the pattern kmap/memcpy/kunmap occurs.
This pattern was lifted to the core common functions
memcpy_[to|from]_page().
Use these new functions to reduce the code, eliminate direct uses of
kmap, and leverage the new core functions use of kmap_local_page().
Also, there is 1 place where a kmap/memcpy is followed by an
optional memset. Here we leave the kmap open coded to avoid remapping
the page but use kmap_local_page() directly.
Development of this patch was aided by the coccinelle script:
// <smpl>
// SPDX-License-Identifier: GPL-2.0-only
// Find kmap/memcpy/kunmap pattern and replace with memcpy*page calls
//
// NOTE: Offsets and other expressions may be more complex than what the script
// will automatically generate. Therefore a catchall rule is provided to find
// the pattern which then must be evaluated by hand.
//
// Confidence: Low
// Copyright: (C) 2021 Intel Corporation
// URL: http://coccinelle.lip6.fr/
// Comments:
// Options:
//
// simple memcpy version
//
@ memcpy_rule1 @
expression page, T, F, B, Off;
identifier ptr;
type VP;
@@
(
-VP ptr = kmap(page);
|
-ptr = kmap(page);
|
-VP ptr = kmap_atomic(page);
|
-ptr = kmap_atomic(page);
)
<+...
(
-memcpy(ptr + Off, F, B);
+memcpy_to_page(page, Off, F, B);
|
-memcpy(ptr, F, B);
+memcpy_to_page(page, 0, F, B);
|
-memcpy(T, ptr + Off, B);
+memcpy_from_page(T, page, Off, B);
|
-memcpy(T, ptr, B);
+memcpy_from_page(T, page, 0, B);
)
...+>
(
-kunmap(page);
|
-kunmap_atomic(ptr);
)
// Remove any pointers left unused
@
depends on memcpy_rule1
@
identifier memcpy_rule1.ptr;
type VP, VP1;
@@
-VP ptr;
... when != ptr;
? VP1 ptr;
//
// Some callers kmap without a temp pointer
//
@ memcpy_rule2 @
expression page, T, Off, F, B;
@@
<+...
(
-memcpy(kmap(page) + Off, F, B);
+memcpy_to_page(page, Off, F, B);
|
-memcpy(kmap(page), F, B);
+memcpy_to_page(page, 0, F, B);
|
-memcpy(T, kmap(page) + Off, B);
+memcpy_from_page(T, page, Off, B);
|
-memcpy(T, kmap(page), B);
+memcpy_from_page(T, page, 0, B);
)
...+>
-kunmap(page);
// No need for the ptr variable removal
//
// Catch all
//
@ memcpy_rule3 @
expression page;
expression GenTo, GenFrom, GenSize;
identifier ptr;
type VP;
@@
(
-VP ptr = kmap(page);
|
-ptr = kmap(page);
|
-VP ptr = kmap_atomic(page);
|
-ptr = kmap_atomic(page);
)
<+...
(
//
// Some call sites have complex expressions within the memcpy
// match a catch all to be evaluated by hand.
//
-memcpy(GenTo, GenFrom, GenSize);
+memcpy_to_pageExtra(page, GenTo, GenFrom, GenSize);
+memcpy_from_pageExtra(GenTo, page, GenFrom, GenSize);
)
...+>
(
-kunmap(page);
|
-kunmap_atomic(ptr);
)
// Remove any pointers left unused
@
depends on memcpy_rule3
@
identifier memcpy_rule3.ptr;
type VP, VP1;
@@
-VP ptr;
... when != ptr;
? VP1 ptr;
// <smpl>
Signed-off-by: Ira Weiny <[email protected]>
---
Changes from v1:
Update commit message per David
https://lore.kernel.org/lkml/[email protected]/
---
fs/btrfs/compression.c | 6 ++----
fs/btrfs/lzo.c | 4 ++--
fs/btrfs/reflink.c | 6 +-----
fs/btrfs/send.c | 7 ++-----
fs/btrfs/zlib.c | 5 ++---
fs/btrfs/zstd.c | 6 ++----
6 files changed, 11 insertions(+), 23 deletions(-)
diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index 5ae3fa0386b7..047b632b4139 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -1231,7 +1231,6 @@ int btrfs_decompress_buf2page(const char *buf, unsigned long buf_start,
unsigned long prev_start_byte;
unsigned long working_bytes = total_out - buf_start;
unsigned long bytes;
- char *kaddr;
struct bio_vec bvec = bio_iter_iovec(bio, bio->bi_iter);
/*
@@ -1262,9 +1261,8 @@ int btrfs_decompress_buf2page(const char *buf, unsigned long buf_start,
PAGE_SIZE - (buf_offset % PAGE_SIZE));
bytes = min(bytes, working_bytes);
- kaddr = kmap_atomic(bvec.bv_page);
- memcpy(kaddr + bvec.bv_offset, buf + buf_offset, bytes);
- kunmap_atomic(kaddr);
+ memcpy_to_page(bvec.bv_page, bvec.bv_offset, buf + buf_offset,
+ bytes);
flush_dcache_page(bvec.bv_page);
buf_offset += bytes;
diff --git a/fs/btrfs/lzo.c b/fs/btrfs/lzo.c
index aa9cd11f4b78..9084a950dc09 100644
--- a/fs/btrfs/lzo.c
+++ b/fs/btrfs/lzo.c
@@ -467,7 +467,7 @@ int lzo_decompress(struct list_head *ws, unsigned char *data_in,
destlen = min_t(unsigned long, destlen, PAGE_SIZE);
bytes = min_t(unsigned long, destlen, out_len - start_byte);
- kaddr = kmap_atomic(dest_page);
+ kaddr = kmap_local_page(dest_page);
memcpy(kaddr, workspace->buf + start_byte, bytes);
/*
@@ -477,7 +477,7 @@ int lzo_decompress(struct list_head *ws, unsigned char *data_in,
*/
if (bytes < destlen)
memset(kaddr+bytes, 0, destlen-bytes);
- kunmap_atomic(kaddr);
+ kunmap_local(kaddr);
out:
return ret;
}
diff --git a/fs/btrfs/reflink.c b/fs/btrfs/reflink.c
index b03e7891394e..74c62e49c0c9 100644
--- a/fs/btrfs/reflink.c
+++ b/fs/btrfs/reflink.c
@@ -103,12 +103,8 @@ static int copy_inline_to_page(struct btrfs_inode *inode,
set_bit(BTRFS_INODE_NO_DELALLOC_FLUSH, &inode->runtime_flags);
if (comp_type == BTRFS_COMPRESS_NONE) {
- char *map;
-
- map = kmap(page);
- memcpy(map, data_start, datal);
+ memcpy_to_page(page, 0, data_start, datal);
flush_dcache_page(page);
- kunmap(page);
} else {
ret = btrfs_decompress(comp_type, data_start, page, 0,
inline_size, datal);
diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index 78a35374d492..83982b3b7057 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -4948,7 +4948,6 @@ static int put_file_data(struct send_ctx *sctx, u64 offset, u32 len)
struct btrfs_fs_info *fs_info = root->fs_info;
struct inode *inode;
struct page *page;
- char *addr;
pgoff_t index = offset >> PAGE_SHIFT;
pgoff_t last_index;
unsigned pg_offset = offset_in_page(offset);
@@ -5001,10 +5000,8 @@ static int put_file_data(struct send_ctx *sctx, u64 offset, u32 len)
}
}
- addr = kmap(page);
- memcpy(sctx->send_buf + sctx->send_size, addr + pg_offset,
- cur_len);
- kunmap(page);
+ memcpy_from_page(sctx->send_buf + sctx->send_size, page,
+ pg_offset, cur_len);
unlock_page(page);
put_page(page);
index++;
diff --git a/fs/btrfs/zlib.c b/fs/btrfs/zlib.c
index 05615a1099db..d524acf7b3e5 100644
--- a/fs/btrfs/zlib.c
+++ b/fs/btrfs/zlib.c
@@ -432,9 +432,8 @@ int zlib_decompress(struct list_head *ws, unsigned char *data_in,
PAGE_SIZE - (buf_offset % PAGE_SIZE));
bytes = min(bytes, bytes_left);
- kaddr = kmap_atomic(dest_page);
- memcpy(kaddr + pg_offset, workspace->buf + buf_offset, bytes);
- kunmap_atomic(kaddr);
+ memcpy_to_page(dest_page, pg_offset,
+ workspace->buf + buf_offset, bytes);
pg_offset += bytes;
bytes_left -= bytes;
diff --git a/fs/btrfs/zstd.c b/fs/btrfs/zstd.c
index 9a4871636c6c..8e9626d63976 100644
--- a/fs/btrfs/zstd.c
+++ b/fs/btrfs/zstd.c
@@ -688,10 +688,8 @@ int zstd_decompress(struct list_head *ws, unsigned char *data_in,
bytes = min_t(unsigned long, destlen - pg_offset,
workspace->out_buf.size - buf_offset);
- kaddr = kmap_atomic(dest_page);
- memcpy(kaddr + pg_offset, workspace->out_buf.dst + buf_offset,
- bytes);
- kunmap_atomic(kaddr);
+ memcpy_to_page(dest_page, pg_offset,
+ workspace->out_buf.dst + buf_offset, bytes);
pg_offset += bytes;
}
--
2.28.0.rc0.12.gb6a658bd00c9
Looks good,
Reviewed-by: Christoph Hellwig <[email protected]>
On Tue, Feb 09, 2021 at 10:22:13PM -0800, [email protected] wrote:
> From: Ira Weiny <[email protected]>
>
> Changes from V1:
> Rework commit messages because they were very weak
> Change 'fs/btrfs: X' to 'btrfs: x'
> https://lore.kernel.org/lkml/[email protected]/
> Per Andrew
> Split out changes to highmem.h
> The addition memcpy, memmove, and memset page functions
> The change from kmap_atomic to kmap_local_page
> The addition of BUG_ON
> The conversion of memzero_page to zero_user in iov_iter
> Change BUG_ON to VM_BUG_ON
> While we are refactoring adjust the line length down per Chaitany
>
>
> There are many places where kmap/<operation>/kunmap patterns occur. We lift a
> couple of these patterns to the core common functions and use them as well as
> existing core functions in the btrfs file system. At the same time we convert
> those core functions to use kmap_local_page() which is more efficient in those
> calls.
>
> Per the conversation on V1 it looks like Andrew would like this to go through
> the btrfs tree. I think that is fine. The other users of
> memcpy_[to|from]_page are probably not ready and I believe could be taken in an
> early rc after David submits.
>
> Is that ok with you David?
Yes.
The branch is now in
https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git/log/?h=kmap-conversion
let me know if I've missed acked-by or reviewed-by, I added those sent
to the mailinglist and added mine to the btrfs ones and to the iov_iter
patch.
I'll add the patchset to my for-next so it gets picked by linux-next and
will keep testing it for at least a week.
Though this is less than the expected time before merge window, the
reasoning is that it's exporting helpers that are going to be used in
various subsystems. The changes in btrfs are simple and would allow to
focus on the other less trivial conversions. ETA for the pull request is
mid of the 2nd week of the merge window or after rc1.
On Thu, Feb 11, 2021 at 08:38:03PM +0100, David Sterba wrote:
> On Tue, Feb 09, 2021 at 10:22:13PM -0800, [email protected] wrote:
> > From: Ira Weiny <[email protected]>
> >
> > Per the conversation on V1 it looks like Andrew would like this to go through
> > the btrfs tree. I think that is fine. The other users of
> > memcpy_[to|from]_page are probably not ready and I believe could be taken in an
> > early rc after David submits.
> >
> > Is that ok with you David?
>
> Yes.
>
> The branch is now in
> https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git/log/?h=kmap-conversion
> let me know if I've missed acked-by or reviewed-by, I added those sent
> to the mailinglist and added mine to the btrfs ones and to the iov_iter
> patch.
Looks good. Thank you!
>
> I'll add the patchset to my for-next so it gets picked by linux-next and
> will keep testing it for at least a week.
>
> Though this is less than the expected time before merge window, the
> reasoning is that it's exporting helpers that are going to be used in
> various subsystems. The changes in btrfs are simple and would allow to
> focus on the other less trivial conversions. ETA for the pull request is
> mid of the 2nd week of the merge window or after rc1.
Thanks for working with me on this. Yes these were the more straight forward
conversions. The next set will require more review and I should have them
posted soon at least for RFC. Unfortunately, there are 2 places which are
proving difficult to follow the mapping orders required of kmap_local_page().
I'll open that discussion with the next round of conversions.
For now, thank you again,
Ira