From: "Fabio M. De Francesco" <fmdefrancesco@gmail.com>
To: Chris Mason <clm@fb.com>, Josef Bacik <josef@toxicpanda.com>,
David Sterba <dsterba@suse.com>, Nick Terrell <terrelln@fb.com>,
Chris Down <chris@chrisdown.name>,
Filipe Manana <fdmanana@suse.com>, Qu Wenruo <wqu@suse.com>,
Nikolay Borisov <nborisov@suse.com>,
Gabriel Niebler <gniebler@suse.com>,
linux-btrfs@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: "Fabio M. De Francesco" <fmdefrancesco@gmail.com>,
Ira Weiny <ira.weiny@intel.com>
Subject: [RFC PATCH] btrfs: Replace kmap() with kmap_local_page() in zlib.c
Date: Tue, 14 Jun 2022 12:47:18 +0200 [thread overview]
Message-ID: <20220614104718.9193-1-fmdefrancesco@gmail.com> (raw)
The use of kmap() is being deprecated in favor of kmap_local_page(). With
kmap_local_page(), the mapping is per thread, CPU local and not globally
visible.
Therefore, use kmap_local_page() / kunmap_local() in zlib.c because in
this file the mappings are per thread and are not visible in other
contexts; meanwhile refactor zlib_compress_pages() to comply with nested
local mapping / unmapping ordering rules.
Tested with xfstests on QEMU + KVM 32 bits VM with 4GB of RAM and
HIGHMEM64G enabled.
This is an RFC PATCH because it passes all tests of the "compress"
group, with the only two exceptions of tests/btrfs/138 (it freezes the
VM) and tests/btrfs/251 (it runs forever but doesn't freeze the VM).
Can anyone please take a look and tell me what I'm still overlooking
after days of code inspections?
Suggested-by: Ira Weiny <ira.weiny@intel.com>
Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
---
fs/btrfs/zlib.c | 75 ++++++++++++++++++++++++++++++++-----------------
1 file changed, 50 insertions(+), 25 deletions(-)
diff --git a/fs/btrfs/zlib.c b/fs/btrfs/zlib.c
index 767a0c6c9694..18f111bb2deb 100644
--- a/fs/btrfs/zlib.c
+++ b/fs/btrfs/zlib.c
@@ -97,8 +97,8 @@ int zlib_compress_pages(struct list_head *ws, struct address_space *mapping,
{
struct workspace *workspace = list_entry(ws, struct workspace, list);
int ret;
- char *data_in;
- char *cpage_out;
+ char *data_in = NULL;
+ char *cpage_out = NULL;
int nr_pages = 0;
struct page *in_page = NULL;
struct page *out_page = NULL;
@@ -126,7 +126,7 @@ int zlib_compress_pages(struct list_head *ws, struct address_space *mapping,
ret = -ENOMEM;
goto out;
}
- cpage_out = kmap(out_page);
+ cpage_out = kmap_local_page(out_page);
pages[0] = out_page;
nr_pages = 1;
@@ -148,26 +148,26 @@ int zlib_compress_pages(struct list_head *ws, struct address_space *mapping,
int i;
for (i = 0; i < in_buf_pages; i++) {
- if (in_page) {
- kunmap(in_page);
+ if (data_in) {
+ kunmap_local(data_in);
put_page(in_page);
}
in_page = find_get_page(mapping,
start >> PAGE_SHIFT);
- data_in = kmap(in_page);
+ data_in = kmap_local_page(in_page);
memcpy(workspace->buf + i * PAGE_SIZE,
data_in, PAGE_SIZE);
start += PAGE_SIZE;
}
workspace->strm.next_in = workspace->buf;
} else {
- if (in_page) {
- kunmap(in_page);
+ if (data_in) {
+ kunmap_local(data_in);
put_page(in_page);
}
in_page = find_get_page(mapping,
start >> PAGE_SHIFT);
- data_in = kmap(in_page);
+ data_in = kmap_local_page(in_page);
start += PAGE_SIZE;
workspace->strm.next_in = data_in;
}
@@ -196,9 +196,14 @@ int zlib_compress_pages(struct list_head *ws, struct address_space *mapping,
* the stream end if required
*/
if (workspace->strm.avail_out == 0) {
- kunmap(out_page);
+ kunmap_local(data_in);
+ data_in = NULL;
+ put_page(in_page);
+
+ kunmap_local(cpage_out);
if (nr_pages == nr_dest_pages) {
- out_page = NULL;
+ cpage_out = NULL;
+ put_page(out_page);
ret = -E2BIG;
goto out;
}
@@ -207,7 +212,14 @@ int zlib_compress_pages(struct list_head *ws, struct address_space *mapping,
ret = -ENOMEM;
goto out;
}
- cpage_out = kmap(out_page);
+ cpage_out = kmap_local_page(out_page);
+
+ in_page = find_get_page(mapping, start >> PAGE_SHIFT);
+ data_in = kmap_local_page(in_page);
+ workspace->strm.next_in = data_in;
+ workspace->strm.avail_in = min(bytes_left,
+ (unsigned long)workspace->buf_size);
+
pages[nr_pages] = out_page;
nr_pages++;
workspace->strm.avail_out = PAGE_SIZE;
@@ -233,10 +245,15 @@ int zlib_compress_pages(struct list_head *ws, struct address_space *mapping,
ret = -EIO;
goto out;
} else if (workspace->strm.avail_out == 0) {
+ kunmap_local(data_in);
+ data_in = NULL;
+ put_page(in_page);
+
/* get another page for the stream end */
- kunmap(out_page);
+ kunmap_local(cpage_out);
if (nr_pages == nr_dest_pages) {
- out_page = NULL;
+ cpage_out = NULL;
+ put_page(out_page);
ret = -E2BIG;
goto out;
}
@@ -245,7 +262,14 @@ int zlib_compress_pages(struct list_head *ws, struct address_space *mapping,
ret = -ENOMEM;
goto out;
}
- cpage_out = kmap(out_page);
+ cpage_out = kmap_local_page(out_page);
+
+ in_page = find_get_page(mapping, start >> PAGE_SHIFT);
+ data_in = kmap_local_page(in_page);
+ workspace->strm.next_in = data_in;
+ workspace->strm.avail_in = min(bytes_left,
+ (unsigned long)workspace->buf_size);
+
pages[nr_pages] = out_page;
nr_pages++;
workspace->strm.avail_out = PAGE_SIZE;
@@ -264,13 +288,13 @@ int zlib_compress_pages(struct list_head *ws, struct address_space *mapping,
*total_in = workspace->strm.total_in;
out:
*out_pages = nr_pages;
- if (out_page)
- kunmap(out_page);
-
- if (in_page) {
- kunmap(in_page);
+ if (data_in) {
+ kunmap_local(data_in);
put_page(in_page);
}
+ if (cpage_out)
+ kunmap_local(cpage_out);
+
return ret;
}
@@ -287,7 +311,7 @@ int zlib_decompress_bio(struct list_head *ws, struct compressed_bio *cb)
unsigned long buf_start;
struct page **pages_in = cb->compressed_pages;
- data_in = kmap(pages_in[page_in_index]);
+ data_in = kmap_local_page(pages_in[page_in_index]);
workspace->strm.next_in = data_in;
workspace->strm.avail_in = min_t(size_t, srclen, PAGE_SIZE);
workspace->strm.total_in = 0;
@@ -309,7 +333,7 @@ int zlib_decompress_bio(struct list_head *ws, struct compressed_bio *cb)
if (Z_OK != zlib_inflateInit2(&workspace->strm, wbits)) {
pr_warn("BTRFS: inflateInit failed\n");
- kunmap(pages_in[page_in_index]);
+ kunmap_local(data_in);
return -EIO;
}
while (workspace->strm.total_in < srclen) {
@@ -336,13 +360,14 @@ int zlib_decompress_bio(struct list_head *ws, struct compressed_bio *cb)
if (workspace->strm.avail_in == 0) {
unsigned long tmp;
- kunmap(pages_in[page_in_index]);
+
+ kunmap_local(data_in);
page_in_index++;
if (page_in_index >= total_pages_in) {
data_in = NULL;
break;
}
- data_in = kmap(pages_in[page_in_index]);
+ data_in = kmap_local_page(pages_in[page_in_index]);
workspace->strm.next_in = data_in;
tmp = srclen - workspace->strm.total_in;
workspace->strm.avail_in = min(tmp, PAGE_SIZE);
@@ -355,7 +380,7 @@ int zlib_decompress_bio(struct list_head *ws, struct compressed_bio *cb)
done:
zlib_inflateEnd(&workspace->strm);
if (data_in)
- kunmap(pages_in[page_in_index]);
+ kunmap_local(data_in);
if (!ret)
zero_fill_bio(cb->orig_bio);
return ret;
--
2.36.1
next reply other threads:[~2022-06-14 10:47 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-06-14 10:47 Fabio M. De Francesco [this message]
2022-06-14 14:13 ` Ira Weiny
2022-06-17 12:18 ` Fabio M. De Francesco
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220614104718.9193-1-fmdefrancesco@gmail.com \
--to=fmdefrancesco@gmail.com \
--cc=chris@chrisdown.name \
--cc=clm@fb.com \
--cc=dsterba@suse.com \
--cc=fdmanana@suse.com \
--cc=gniebler@suse.com \
--cc=ira.weiny@intel.com \
--cc=josef@toxicpanda.com \
--cc=linux-btrfs@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=nborisov@suse.com \
--cc=terrelln@fb.com \
--cc=wqu@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®