[PATCH 0/6] Remove highmem allocations, kmap/kunmap

[PATCH 0/6] Remove highmem allocations, kmap/kunmap

[David Sterba]

The highmem was maybe was a good idea long time ago but with 64bit
architectures everywhere I don't think we need to take it into account.
This does not mean this 32bit won't work, just that it won't try to use
temporary pages in highmem for compression and raid56. The key word is
temporary. Combining a very fast device (like hundreds of megabytes
throughput) and 32bit machine with reasonable memory (for 32bit, like
8G), it could become a problem once low memory is scarce.

David Sterba (6):
  btrfs: drop from __GFP_HIGHMEM all allocations
  btrfs: compression: drop kmap/kunmap from lzo
  btrfs: compression: drop kmap/kunmap from zlib
  btrfs: compression: drop kmap/kunmap from zstd
  btrfs: compression: drop kmap/kunmap from generic helpers
  btrfs: check-integrity: drop kmap/kunmap for block pages

 fs/btrfs/check-integrity.c | 11 +++-------
 fs/btrfs/compression.c     |  6 ++----
 fs/btrfs/inode.c           |  3 +--
 fs/btrfs/lzo.c             | 42 +++++++++++---------------------------
 fs/btrfs/raid56.c          | 10 ++++-----
 fs/btrfs/zlib.c            | 42 +++++++++++++-------------------------
 fs/btrfs/zstd.c            | 33 +++++++++++-------------------
 7 files changed, 49 insertions(+), 98 deletions(-)

-- 
2.31.1

[PATCH 1/6] btrfs: drop from __GFP_HIGHMEM all allocations

[David Sterba]

The highmem flag is used for allocating pages for compression and for
raid56 pages. The high memory makes sense on 32bit systems but is not
without problems. On 64bit system's it's just another layer of wrappers.

The time the pages are allocated for compression or raid56 is relatively
short (about a transaction commit), so the pages are not blocked
indefinitely. As the number of pages depends on the amount of data being
written/read, there's a theoretical problem. A fast device on a 32bit
system could use most of the low memory pool, while with the highmem
allocation that would not happen. This was possibly the original idea
long time ago, but nowadays we optimize for 64bit systems.

This patch removes all usage of the __GFP_HIGHMEM flag for page
allocation, the kmap/kunmap are still in place and will be removed in
followup patches. Remaining is masking out the bit in
alloc_extent_state and __lookup_free_space_inode, that can safely stay.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/compression.c |  3 +--
 fs/btrfs/lzo.c         |  4 ++--
 fs/btrfs/raid56.c      | 10 +++++-----
 fs/btrfs/zlib.c        |  6 +++---
 fs/btrfs/zstd.c        |  6 +++---
 5 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index 9a023ae0f98b..8cf5903a5be2 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -721,8 +721,7 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
 		goto fail1;
 
 	for (pg_index = 0; pg_index < nr_pages; pg_index++) {
-		cb->compressed_pages[pg_index] = alloc_page(GFP_NOFS |
-							      __GFP_HIGHMEM);
+		cb->compressed_pages[pg_index] = alloc_page(GFP_NOFS);
 		if (!cb->compressed_pages[pg_index]) {
 			faili = pg_index - 1;
 			ret = BLK_STS_RESOURCE;
diff --git a/fs/btrfs/lzo.c b/fs/btrfs/lzo.c
index cd042c7567a4..2bebb60c5830 100644
--- a/fs/btrfs/lzo.c
+++ b/fs/btrfs/lzo.c
@@ -146,7 +146,7 @@ int lzo_compress_pages(struct list_head *ws, struct address_space *mapping,
 	 * store the size of all chunks of compressed data in
 	 * the first 4 bytes
 	 */
-	out_page = alloc_page(GFP_NOFS | __GFP_HIGHMEM);
+	out_page = alloc_page(GFP_NOFS);
 	if (out_page == NULL) {
 		ret = -ENOMEM;
 		goto out;
@@ -216,7 +216,7 @@ int lzo_compress_pages(struct list_head *ws, struct address_space *mapping,
 					goto out;
 				}
 
-				out_page = alloc_page(GFP_NOFS | __GFP_HIGHMEM);
+				out_page = alloc_page(GFP_NOFS);
 				if (out_page == NULL) {
 					ret = -ENOMEM;
 					goto out;
diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
index 244d499ebc72..a40a45a007d4 100644
--- a/fs/btrfs/raid56.c
+++ b/fs/btrfs/raid56.c
@@ -1035,7 +1035,7 @@ static int alloc_rbio_pages(struct btrfs_raid_bio *rbio)
 	for (i = 0; i < rbio->nr_pages; i++) {
 		if (rbio->stripe_pages[i])
 			continue;
-		page = alloc_page(GFP_NOFS | __GFP_HIGHMEM);
+		page = alloc_page(GFP_NOFS);
 		if (!page)
 			return -ENOMEM;
 		rbio->stripe_pages[i] = page;
@@ -1054,7 +1054,7 @@ static int alloc_rbio_parity_pages(struct btrfs_raid_bio *rbio)
 	for (; i < rbio->nr_pages; i++) {
 		if (rbio->stripe_pages[i])
 			continue;
-		page = alloc_page(GFP_NOFS | __GFP_HIGHMEM);
+		page = alloc_page(GFP_NOFS);
 		if (!page)
 			return -ENOMEM;
 		rbio->stripe_pages[i] = page;
@@ -2300,7 +2300,7 @@ static int alloc_rbio_essential_pages(struct btrfs_raid_bio *rbio)
 			if (rbio->stripe_pages[index])
 				continue;
 
-			page = alloc_page(GFP_NOFS | __GFP_HIGHMEM);
+			page = alloc_page(GFP_NOFS);
 			if (!page)
 				return -ENOMEM;
 			rbio->stripe_pages[index] = page;
@@ -2350,14 +2350,14 @@ static noinline void finish_parity_scrub(struct btrfs_raid_bio *rbio,
 	if (!need_check)
 		goto writeback;
 
-	p_page = alloc_page(GFP_NOFS | __GFP_HIGHMEM);
+	p_page = alloc_page(GFP_NOFS);
 	if (!p_page)
 		goto cleanup;
 	SetPageUptodate(p_page);
 
 	if (has_qstripe) {
 		/* RAID6, allocate and map temp space for the Q stripe */
-		q_page = alloc_page(GFP_NOFS | __GFP_HIGHMEM);
+		q_page = alloc_page(GFP_NOFS);
 		if (!q_page) {
 			__free_page(p_page);
 			goto cleanup;
diff --git a/fs/btrfs/zlib.c b/fs/btrfs/zlib.c
index c3fa7d3fa770..2c792bc5a987 100644
--- a/fs/btrfs/zlib.c
+++ b/fs/btrfs/zlib.c
@@ -121,7 +121,7 @@ int zlib_compress_pages(struct list_head *ws, struct address_space *mapping,
 	workspace->strm.total_in = 0;
 	workspace->strm.total_out = 0;
 
-	out_page = alloc_page(GFP_NOFS | __GFP_HIGHMEM);
+	out_page = alloc_page(GFP_NOFS);
 	if (out_page == NULL) {
 		ret = -ENOMEM;
 		goto out;
@@ -202,7 +202,7 @@ int zlib_compress_pages(struct list_head *ws, struct address_space *mapping,
 				ret = -E2BIG;
 				goto out;
 			}
-			out_page = alloc_page(GFP_NOFS | __GFP_HIGHMEM);
+			out_page = alloc_page(GFP_NOFS);
 			if (out_page == NULL) {
 				ret = -ENOMEM;
 				goto out;
@@ -240,7 +240,7 @@ int zlib_compress_pages(struct list_head *ws, struct address_space *mapping,
 				ret = -E2BIG;
 				goto out;
 			}
-			out_page = alloc_page(GFP_NOFS | __GFP_HIGHMEM);
+			out_page = alloc_page(GFP_NOFS);
 			if (out_page == NULL) {
 				ret = -ENOMEM;
 				goto out;
diff --git a/fs/btrfs/zstd.c b/fs/btrfs/zstd.c
index 3e26b466476a..9451d2bb984e 100644
--- a/fs/btrfs/zstd.c
+++ b/fs/btrfs/zstd.c
@@ -405,7 +405,7 @@ int zstd_compress_pages(struct list_head *ws, struct address_space *mapping,
 
 
 	/* Allocate and map in the output buffer */
-	out_page = alloc_page(GFP_NOFS | __GFP_HIGHMEM);
+	out_page = alloc_page(GFP_NOFS);
 	if (out_page == NULL) {
 		ret = -ENOMEM;
 		goto out;
@@ -452,7 +452,7 @@ int zstd_compress_pages(struct list_head *ws, struct address_space *mapping,
 				ret = -E2BIG;
 				goto out;
 			}
-			out_page = alloc_page(GFP_NOFS | __GFP_HIGHMEM);
+			out_page = alloc_page(GFP_NOFS);
 			if (out_page == NULL) {
 				ret = -ENOMEM;
 				goto out;
@@ -512,7 +512,7 @@ int zstd_compress_pages(struct list_head *ws, struct address_space *mapping,
 			ret = -E2BIG;
 			goto out;
 		}
-		out_page = alloc_page(GFP_NOFS | __GFP_HIGHMEM);
+		out_page = alloc_page(GFP_NOFS);
 		if (out_page == NULL) {
 			ret = -ENOMEM;
 			goto out;
-- 
2.31.1

[PATCH 2/6] btrfs: compression: drop kmap/kunmap from lzo

[David Sterba]

As we don't use highmem pages anymore, drop the kmap/kunmap. The kmap is
simply page_address and kunmap is a no-op.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/lzo.c | 38 ++++++++++----------------------------
 1 file changed, 10 insertions(+), 28 deletions(-)

diff --git a/fs/btrfs/lzo.c b/fs/btrfs/lzo.c
index 2bebb60c5830..576a0e6142ad 100644
--- a/fs/btrfs/lzo.c
+++ b/fs/btrfs/lzo.c
@@ -140,7 +140,7 @@ int lzo_compress_pages(struct list_head *ws, struct address_space *mapping,
 	*total_in = 0;
 
 	in_page = find_get_page(mapping, start >> PAGE_SHIFT);
-	data_in = kmap(in_page);
+	data_in = page_address(in_page);
 
 	/*
 	 * store the size of all chunks of compressed data in
@@ -151,7 +151,7 @@ int lzo_compress_pages(struct list_head *ws, struct address_space *mapping,
 		ret = -ENOMEM;
 		goto out;
 	}
-	cpage_out = kmap(out_page);
+	cpage_out = page_address(out_page);
 	out_offset = LZO_LEN;
 	tot_out = LZO_LEN;
 	pages[0] = out_page;
@@ -209,7 +209,6 @@ int lzo_compress_pages(struct list_head *ws, struct address_space *mapping,
 				if (out_len == 0 && tot_in >= len)
 					break;
 
-				kunmap(out_page);
 				if (nr_pages == nr_dest_pages) {
 					out_page = NULL;
 					ret = -E2BIG;
@@ -221,7 +220,7 @@ int lzo_compress_pages(struct list_head *ws, struct address_space *mapping,
 					ret = -ENOMEM;
 					goto out;
 				}
-				cpage_out = kmap(out_page);
+				cpage_out = page_address(out_page);
 				pages[nr_pages++] = out_page;
 
 				pg_bytes_left = PAGE_SIZE;
@@ -243,12 +242,11 @@ int lzo_compress_pages(struct list_head *ws, struct address_space *mapping,
 			break;
 
 		bytes_left = len - tot_in;
-		kunmap(in_page);
 		put_page(in_page);
 
 		start += PAGE_SIZE;
 		in_page = find_get_page(mapping, start >> PAGE_SHIFT);
-		data_in = kmap(in_page);
+		data_in = page_address(in_page);
 		in_len = min(bytes_left, PAGE_SIZE);
 	}
 
@@ -258,22 +256,17 @@ int lzo_compress_pages(struct list_head *ws, struct address_space *mapping,
 	}
 
 	/* store the size of all chunks of compressed data */
-	sizes_ptr = kmap_local_page(pages[0]);
+	sizes_ptr = page_address(pages[0]);
 	write_compress_length(sizes_ptr, tot_out);
-	kunmap_local(sizes_ptr);
 
 	ret = 0;
 	*total_out = tot_out;
 	*total_in = tot_in;
 out:
 	*out_pages = nr_pages;
-	if (out_page)
-		kunmap(out_page);
 
-	if (in_page) {
-		kunmap(in_page);
+	if (in_page)
 		put_page(in_page);
-	}
 
 	return ret;
 }
@@ -299,12 +292,11 @@ int lzo_decompress_bio(struct list_head *ws, struct compressed_bio *cb)
 	unsigned long tot_out;
 	unsigned long tot_len;
 	char *buf;
-	bool may_late_unmap, need_unmap;
 	struct page **pages_in = cb->compressed_pages;
 	u64 disk_start = cb->start;
 	struct bio *orig_bio = cb->orig_bio;
 
-	data_in = kmap(pages_in[0]);
+	data_in = page_address(pages_in[0]);
 	tot_len = read_compress_length(data_in);
 	/*
 	 * Compressed data header check.
@@ -345,13 +337,11 @@ int lzo_decompress_bio(struct list_head *ws, struct compressed_bio *cb)
 
 		tot_in += in_len;
 		working_bytes = in_len;
-		may_late_unmap = need_unmap = false;
 
 		/* fast path: avoid using the working buffer */
 		if (in_page_bytes_left >= in_len) {
 			buf = data_in + in_offset;
 			bytes = in_len;
-			may_late_unmap = true;
 			goto cont;
 		}
 
@@ -381,12 +371,8 @@ int lzo_decompress_bio(struct list_head *ws, struct compressed_bio *cb)
 					goto done;
 				}
 
-				if (may_late_unmap)
-					need_unmap = true;
-				else
-					kunmap(pages_in[page_in_index]);
-
-				data_in = kmap(pages_in[++page_in_index]);
+				page_in_index++;
+				data_in = page_address(pages_in[page_in_index]);
 
 				in_page_bytes_left = PAGE_SIZE;
 				in_offset = 0;
@@ -396,8 +382,6 @@ int lzo_decompress_bio(struct list_head *ws, struct compressed_bio *cb)
 		out_len = max_segment_len;
 		ret = lzo1x_decompress_safe(buf, in_len, workspace->buf,
 					    &out_len);
-		if (need_unmap)
-			kunmap(pages_in[page_in_index - 1]);
 		if (ret != LZO_E_OK) {
 			pr_warn("BTRFS: decompress failed\n");
 			ret = -EIO;
@@ -413,7 +397,6 @@ int lzo_decompress_bio(struct list_head *ws, struct compressed_bio *cb)
 			break;
 	}
 done:
-	kunmap(pages_in[page_in_index]);
 	if (!ret)
 		zero_fill_bio(orig_bio);
 	return ret;
@@ -466,7 +449,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_local_page(dest_page);
+	kaddr = page_address(dest_page);
 	memcpy(kaddr, workspace->buf + start_byte, bytes);
 
 	/*
@@ -476,7 +459,6 @@ int lzo_decompress(struct list_head *ws, unsigned char *data_in,
 	 */
 	if (bytes < destlen)
 		memset(kaddr+bytes, 0, destlen-bytes);
-	kunmap_local(kaddr);
 out:
 	return ret;
 }
-- 
2.31.1

[PATCH 3/6] btrfs: compression: drop kmap/kunmap from zlib

[David Sterba]

As we don't use highmem pages anymore, drop the kmap/kunmap. The kmap is
simply page_address and kunmap is a no-op.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/zlib.c | 36 +++++++++++-------------------------
 1 file changed, 11 insertions(+), 25 deletions(-)

diff --git a/fs/btrfs/zlib.c b/fs/btrfs/zlib.c
index 2c792bc5a987..5e18d7ad75a4 100644
--- a/fs/btrfs/zlib.c
+++ b/fs/btrfs/zlib.c
@@ -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 = page_address(out_page);
 	pages[0] = out_page;
 	nr_pages = 1;
 
@@ -148,26 +148,22 @@ 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 (in_page)
 						put_page(in_page);
-					}
 					in_page = find_get_page(mapping,
 								start >> PAGE_SHIFT);
-					data_in = kmap(in_page);
+					data_in = page_address(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 (in_page)
 					put_page(in_page);
-				}
 				in_page = find_get_page(mapping,
 							start >> PAGE_SHIFT);
-				data_in = kmap(in_page);
+				data_in = page_address(in_page);
 				start += PAGE_SIZE;
 				workspace->strm.next_in = data_in;
 			}
@@ -196,7 +192,6 @@ 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);
 			if (nr_pages == nr_dest_pages) {
 				out_page = NULL;
 				ret = -E2BIG;
@@ -207,7 +202,7 @@ int zlib_compress_pages(struct list_head *ws, struct address_space *mapping,
 				ret = -ENOMEM;
 				goto out;
 			}
-			cpage_out = kmap(out_page);
+			cpage_out = page_address(out_page);
 			pages[nr_pages] = out_page;
 			nr_pages++;
 			workspace->strm.avail_out = PAGE_SIZE;
@@ -234,7 +229,6 @@ int zlib_compress_pages(struct list_head *ws, struct address_space *mapping,
 			goto out;
 		} else if (workspace->strm.avail_out == 0) {
 			/* get another page for the stream end */
-			kunmap(out_page);
 			if (nr_pages == nr_dest_pages) {
 				out_page = NULL;
 				ret = -E2BIG;
@@ -245,7 +239,7 @@ int zlib_compress_pages(struct list_head *ws, struct address_space *mapping,
 				ret = -ENOMEM;
 				goto out;
 			}
-			cpage_out = kmap(out_page);
+			cpage_out = page_address(out_page);
 			pages[nr_pages] = out_page;
 			nr_pages++;
 			workspace->strm.avail_out = PAGE_SIZE;
@@ -264,13 +258,8 @@ 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 (in_page)
 		put_page(in_page);
-	}
 	return ret;
 }
 
@@ -289,7 +278,7 @@ int zlib_decompress_bio(struct list_head *ws, struct compressed_bio *cb)
 	u64 disk_start = cb->start;
 	struct bio *orig_bio = cb->orig_bio;
 
-	data_in = kmap(pages_in[page_in_index]);
+	data_in = page_address(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;
@@ -311,7 +300,6 @@ 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]);
 		return -EIO;
 	}
 	while (workspace->strm.total_in < srclen) {
@@ -339,13 +327,13 @@ 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]);
+
 			page_in_index++;
 			if (page_in_index >= total_pages_in) {
 				data_in = NULL;
 				break;
 			}
-			data_in = kmap(pages_in[page_in_index]);
+			data_in = page_address(pages_in[page_in_index]);
 			workspace->strm.next_in = data_in;
 			tmp = srclen - workspace->strm.total_in;
 			workspace->strm.avail_in = min(tmp,
@@ -358,8 +346,6 @@ int zlib_decompress_bio(struct list_head *ws, struct compressed_bio *cb)
 		ret = 0;
 done:
 	zlib_inflateEnd(&workspace->strm);
-	if (data_in)
-		kunmap(pages_in[page_in_index]);
 	if (!ret)
 		zero_fill_bio(orig_bio);
 	return ret;
-- 
2.31.1

[PATCH 4/6] btrfs: compression: drop kmap/kunmap from zstd

[David Sterba]

As we don't use highmem pages anymore, drop the kmap/kunmap. The kmap is
simply page_address and kunmap is a no-op.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/zstd.c | 27 +++++++++------------------
 1 file changed, 9 insertions(+), 18 deletions(-)

diff --git a/fs/btrfs/zstd.c b/fs/btrfs/zstd.c
index 9451d2bb984e..200ba08bfae6 100644
--- a/fs/btrfs/zstd.c
+++ b/fs/btrfs/zstd.c
@@ -399,7 +399,7 @@ int zstd_compress_pages(struct list_head *ws, struct address_space *mapping,
 
 	/* map in the first page of input data */
 	in_page = find_get_page(mapping, start >> PAGE_SHIFT);
-	workspace->in_buf.src = kmap(in_page);
+	workspace->in_buf.src = page_address(in_page);
 	workspace->in_buf.pos = 0;
 	workspace->in_buf.size = min_t(size_t, len, PAGE_SIZE);
 
@@ -411,7 +411,7 @@ int zstd_compress_pages(struct list_head *ws, struct address_space *mapping,
 		goto out;
 	}
 	pages[nr_pages++] = out_page;
-	workspace->out_buf.dst = kmap(out_page);
+	workspace->out_buf.dst = page_address(out_page);
 	workspace->out_buf.pos = 0;
 	workspace->out_buf.size = min_t(size_t, max_out, PAGE_SIZE);
 
@@ -446,7 +446,6 @@ int zstd_compress_pages(struct list_head *ws, struct address_space *mapping,
 		if (workspace->out_buf.pos == workspace->out_buf.size) {
 			tot_out += PAGE_SIZE;
 			max_out -= PAGE_SIZE;
-			kunmap(out_page);
 			if (nr_pages == nr_dest_pages) {
 				out_page = NULL;
 				ret = -E2BIG;
@@ -458,7 +457,7 @@ int zstd_compress_pages(struct list_head *ws, struct address_space *mapping,
 				goto out;
 			}
 			pages[nr_pages++] = out_page;
-			workspace->out_buf.dst = kmap(out_page);
+			workspace->out_buf.dst = page_address(out_page);
 			workspace->out_buf.pos = 0;
 			workspace->out_buf.size = min_t(size_t, max_out,
 							PAGE_SIZE);
@@ -473,13 +472,12 @@ int zstd_compress_pages(struct list_head *ws, struct address_space *mapping,
 		/* Check if we need more input */
 		if (workspace->in_buf.pos == workspace->in_buf.size) {
 			tot_in += PAGE_SIZE;
-			kunmap(in_page);
 			put_page(in_page);
 
 			start += PAGE_SIZE;
 			len -= PAGE_SIZE;
 			in_page = find_get_page(mapping, start >> PAGE_SHIFT);
-			workspace->in_buf.src = kmap(in_page);
+			workspace->in_buf.src = page_address(in_page);
 			workspace->in_buf.pos = 0;
 			workspace->in_buf.size = min_t(size_t, len, PAGE_SIZE);
 		}
@@ -506,7 +504,6 @@ int zstd_compress_pages(struct list_head *ws, struct address_space *mapping,
 
 		tot_out += PAGE_SIZE;
 		max_out -= PAGE_SIZE;
-		kunmap(out_page);
 		if (nr_pages == nr_dest_pages) {
 			out_page = NULL;
 			ret = -E2BIG;
@@ -518,7 +515,7 @@ int zstd_compress_pages(struct list_head *ws, struct address_space *mapping,
 			goto out;
 		}
 		pages[nr_pages++] = out_page;
-		workspace->out_buf.dst = kmap(out_page);
+		workspace->out_buf.dst = page_address(out_page);
 		workspace->out_buf.pos = 0;
 		workspace->out_buf.size = min_t(size_t, max_out, PAGE_SIZE);
 	}
@@ -534,12 +531,8 @@ int zstd_compress_pages(struct list_head *ws, struct address_space *mapping,
 out:
 	*out_pages = nr_pages;
 	/* Cleanup */
-	if (in_page) {
-		kunmap(in_page);
+	if (in_page)
 		put_page(in_page);
-	}
-	if (out_page)
-		kunmap(out_page);
 	return ret;
 }
 
@@ -565,7 +558,7 @@ int zstd_decompress_bio(struct list_head *ws, struct compressed_bio *cb)
 		goto done;
 	}
 
-	workspace->in_buf.src = kmap(pages_in[page_in_index]);
+	workspace->in_buf.src = page_address(pages_in[page_in_index]);
 	workspace->in_buf.pos = 0;
 	workspace->in_buf.size = min_t(size_t, srclen, PAGE_SIZE);
 
@@ -601,14 +594,14 @@ int zstd_decompress_bio(struct list_head *ws, struct compressed_bio *cb)
 			break;
 
 		if (workspace->in_buf.pos == workspace->in_buf.size) {
-			kunmap(pages_in[page_in_index++]);
+			page_in_index++;
 			if (page_in_index >= total_pages_in) {
 				workspace->in_buf.src = NULL;
 				ret = -EIO;
 				goto done;
 			}
 			srclen -= PAGE_SIZE;
-			workspace->in_buf.src = kmap(pages_in[page_in_index]);
+			workspace->in_buf.src = page_address(pages_in[page_in_index]);
 			workspace->in_buf.pos = 0;
 			workspace->in_buf.size = min_t(size_t, srclen, PAGE_SIZE);
 		}
@@ -616,8 +609,6 @@ int zstd_decompress_bio(struct list_head *ws, struct compressed_bio *cb)
 	ret = 0;
 	zero_fill_bio(orig_bio);
 done:
-	if (workspace->in_buf.src)
-		kunmap(pages_in[page_in_index]);
 	return ret;
 }
 
-- 
2.31.1

[PATCH 5/6] btrfs: compression: drop kmap/kunmap from generic helpers

[David Sterba]

The pages in compressed_pages are not from highmem anymore so we can
drop the mapping for checksum calculation and inline extent.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/compression.c | 3 +--
 fs/btrfs/inode.c       | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index 8cf5903a5be2..d40c2c878c83 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -172,10 +172,9 @@ static int check_compressed_csum(struct btrfs_inode *inode, struct bio *bio,
 		/* Hash through the page sector by sector */
 		for (pg_offset = 0; pg_offset < bytes_left;
 		     pg_offset += sectorsize) {
-			kaddr = kmap_atomic(page);
+			kaddr = page_address(page);
 			crypto_shash_digest(shash, kaddr + pg_offset,
 					    sectorsize, csum);
-			kunmap_atomic(kaddr);
 
 			if (memcmp(&csum, cb_sum, csum_size) != 0) {
 				btrfs_print_data_csum_error(inode, disk_start,
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index e6eb20987351..6993f0ffa9f2 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -286,9 +286,8 @@ static int insert_inline_extent(struct btrfs_trans_handle *trans,
 			cur_size = min_t(unsigned long, compressed_size,
 				       PAGE_SIZE);
 
-			kaddr = kmap_atomic(cpage);
+			kaddr = page_address(cpage);
 			write_extent_buffer(leaf, kaddr, ptr, cur_size);
-			kunmap_atomic(kaddr);
 
 			i++;
 			ptr += cur_size;
-- 
2.31.1

[PATCH 6/6] btrfs: check-integrity: drop kmap/kunmap for block pages

[David Sterba]

The pages in block_ctx have never been allocated from highmem (in
btrfsic_read_block) so the mapping is pointless and can be removed.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/check-integrity.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/check-integrity.c b/fs/btrfs/check-integrity.c
index 169508609324..232dd3f85ac0 100644
--- a/fs/btrfs/check-integrity.c
+++ b/fs/btrfs/check-integrity.c
@@ -1558,10 +1558,8 @@ static void btrfsic_release_block_ctx(struct btrfsic_block_data_ctx *block_ctx)
 		/* Pages must be unmapped in reverse order */
 		while (num_pages > 0) {
 			num_pages--;
-			if (block_ctx->datav[num_pages]) {
-				kunmap_local(block_ctx->datav[num_pages]);
+			if (block_ctx->datav[num_pages])
 				block_ctx->datav[num_pages] = NULL;
-			}
 			if (block_ctx->pagev[num_pages]) {
 				__free_page(block_ctx->pagev[num_pages]);
 				block_ctx->pagev[num_pages] = NULL;
@@ -1638,7 +1636,7 @@ static int btrfsic_read_block(struct btrfsic_state *state,
 		i = j;
 	}
 	for (i = 0; i < num_pages; i++)
-		block_ctx->datav[i] = kmap_local_page(block_ctx->pagev[i]);
+		block_ctx->datav[i] = page_address(block_ctx->pagev[i]);
 
 	return block_ctx->len;
 }
@@ -2703,7 +2701,7 @@ static void __btrfsic_submit_bio(struct bio *bio)
 
 		bio_for_each_segment(bvec, bio, iter) {
 			BUG_ON(bvec.bv_len != PAGE_SIZE);
-			mapped_datav[i] = kmap_local_page(bvec.bv_page);
+			mapped_datav[i] = page_address(bvec.bv_page);
 			i++;
 
 			if (dev_state->state->print_mask &
@@ -2716,9 +2714,6 @@ static void __btrfsic_submit_bio(struct bio *bio)
 					      mapped_datav, segs,
 					      bio, &bio_is_patched,
 					      bio->bi_opf);
-		/* Unmap in reverse order */
-		for (--i; i >= 0; i--)
-			kunmap_local(mapped_datav[i]);
 		kfree(mapped_datav);
 	} else if (NULL != dev_state && (bio->bi_opf & REQ_PREFLUSH)) {
 		if (dev_state->state->print_mask &
-- 
2.31.1

Re: [PATCH 0/6] Remove highmem allocations, kmap/kunmap

[Neal Gompa]

On Thu, Jul 8, 2021 at 7:48 AM David Sterba <dsterba@suse.com> wrote:
>
> The highmem was maybe was a good idea long time ago but with 64bit
> architectures everywhere I don't think we need to take it into account.
> This does not mean this 32bit won't work, just that it won't try to use
> temporary pages in highmem for compression and raid56. The key word is
> temporary. Combining a very fast device (like hundreds of megabytes
> throughput) and 32bit machine with reasonable memory (for 32bit, like
> 8G), it could become a problem once low memory is scarce.
>
> David Sterba (6):
>   btrfs: drop from __GFP_HIGHMEM all allocations
>   btrfs: compression: drop kmap/kunmap from lzo
>   btrfs: compression: drop kmap/kunmap from zlib
>   btrfs: compression: drop kmap/kunmap from zstd
>   btrfs: compression: drop kmap/kunmap from generic helpers
>   btrfs: check-integrity: drop kmap/kunmap for block pages
>
>  fs/btrfs/check-integrity.c | 11 +++-------
>  fs/btrfs/compression.c     |  6 ++----
>  fs/btrfs/inode.c           |  3 +--
>  fs/btrfs/lzo.c             | 42 +++++++++++---------------------------
>  fs/btrfs/raid56.c          | 10 ++++-----
>  fs/btrfs/zlib.c            | 42 +++++++++++++-------------------------
>  fs/btrfs/zstd.c            | 33 +++++++++++-------------------
>  7 files changed, 49 insertions(+), 98 deletions(-)
>

I'd be concerned about the impact of this on SBC devices. All Fedora
ARM images have zstd compression applied to them, and it would suck if
we had a performance regression here because of this.


-- 
真実はいつも一つ！/ Always, there's only one truth!

Re: [PATCH 0/6] Remove highmem allocations, kmap/kunmap

[David Sterba]

On Thu, Jul 08, 2021 at 08:45:12AM -0400, Neal Gompa wrote:
> On Thu, Jul 8, 2021 at 7:48 AM David Sterba <dsterba@suse.com> wrote:
> >
> > The highmem was maybe was a good idea long time ago but with 64bit
> > architectures everywhere I don't think we need to take it into account.
> > This does not mean this 32bit won't work, just that it won't try to use
> > temporary pages in highmem for compression and raid56. The key word is
> > temporary. Combining a very fast device (like hundreds of megabytes
> > throughput) and 32bit machine with reasonable memory (for 32bit, like
> > 8G), it could become a problem once low memory is scarce.
> >
> > David Sterba (6):
> >   btrfs: drop from __GFP_HIGHMEM all allocations
> >   btrfs: compression: drop kmap/kunmap from lzo
> >   btrfs: compression: drop kmap/kunmap from zlib
> >   btrfs: compression: drop kmap/kunmap from zstd
> >   btrfs: compression: drop kmap/kunmap from generic helpers
> >   btrfs: check-integrity: drop kmap/kunmap for block pages
> >
> >  fs/btrfs/check-integrity.c | 11 +++-------
> >  fs/btrfs/compression.c     |  6 ++----
> >  fs/btrfs/inode.c           |  3 +--
> >  fs/btrfs/lzo.c             | 42 +++++++++++---------------------------
> >  fs/btrfs/raid56.c          | 10 ++++-----
> >  fs/btrfs/zlib.c            | 42 +++++++++++++-------------------------
> >  fs/btrfs/zstd.c            | 33 +++++++++++-------------------
> >  7 files changed, 49 insertions(+), 98 deletions(-)
> >
> 
> I'd be concerned about the impact of this on SBC devices. All Fedora
> ARM images have zstd compression applied to them, and it would suck if
> we had a performance regression here because of this.

How much memory do the SBC devices have?

Re: [PATCH 0/6] Remove highmem allocations, kmap/kunmap

[Neal Gompa]

On Thu, Jul 8, 2021 at 8:52 AM David Sterba <dsterba@suse.cz> wrote:
>
> On Thu, Jul 08, 2021 at 08:45:12AM -0400, Neal Gompa wrote:
> > On Thu, Jul 8, 2021 at 7:48 AM David Sterba <dsterba@suse.com> wrote:
> > >
> > > The highmem was maybe was a good idea long time ago but with 64bit
> > > architectures everywhere I don't think we need to take it into account.
> > > This does not mean this 32bit won't work, just that it won't try to use
> > > temporary pages in highmem for compression and raid56. The key word is
> > > temporary. Combining a very fast device (like hundreds of megabytes
> > > throughput) and 32bit machine with reasonable memory (for 32bit, like
> > > 8G), it could become a problem once low memory is scarce.
> > >
> > > David Sterba (6):
> > >   btrfs: drop from __GFP_HIGHMEM all allocations
> > >   btrfs: compression: drop kmap/kunmap from lzo
> > >   btrfs: compression: drop kmap/kunmap from zlib
> > >   btrfs: compression: drop kmap/kunmap from zstd
> > >   btrfs: compression: drop kmap/kunmap from generic helpers
> > >   btrfs: check-integrity: drop kmap/kunmap for block pages
> > >
> > >  fs/btrfs/check-integrity.c | 11 +++-------
> > >  fs/btrfs/compression.c     |  6 ++----
> > >  fs/btrfs/inode.c           |  3 +--
> > >  fs/btrfs/lzo.c             | 42 +++++++++++---------------------------
> > >  fs/btrfs/raid56.c          | 10 ++++-----
> > >  fs/btrfs/zlib.c            | 42 +++++++++++++-------------------------
> > >  fs/btrfs/zstd.c            | 33 +++++++++++-------------------
> > >  7 files changed, 49 insertions(+), 98 deletions(-)
> > >
> >
> > I'd be concerned about the impact of this on SBC devices. All Fedora
> > ARM images have zstd compression applied to them, and it would suck if
> > we had a performance regression here because of this.
>
> How much memory do the SBC devices have?

On average? Probably between 1 to 4 GB of RAM. Usually 2GB of RAM is common.


-- 
真実はいつも一つ！/ Always, there's only one truth!

Re: [PATCH 0/6] Remove highmem allocations, kmap/kunmap

[Qu Wenruo]

On 2021/7/8 下午8:45, Neal Gompa wrote:
> On Thu, Jul 8, 2021 at 7:48 AM David Sterba <dsterba@suse.com> wrote:
>>
>> The highmem was maybe was a good idea long time ago but with 64bit
>> architectures everywhere I don't think we need to take it into account.
>> This does not mean this 32bit won't work, just that it won't try to use
>> temporary pages in highmem for compression and raid56. The key word is
>> temporary. Combining a very fast device (like hundreds of megabytes
>> throughput) and 32bit machine with reasonable memory (for 32bit, like
>> 8G), it could become a problem once low memory is scarce.
>>
>> David Sterba (6):
>>    btrfs: drop from __GFP_HIGHMEM all allocations
>>    btrfs: compression: drop kmap/kunmap from lzo
>>    btrfs: compression: drop kmap/kunmap from zlib
>>    btrfs: compression: drop kmap/kunmap from zstd
>>    btrfs: compression: drop kmap/kunmap from generic helpers
>>    btrfs: check-integrity: drop kmap/kunmap for block pages
>>
>>   fs/btrfs/check-integrity.c | 11 +++-------
>>   fs/btrfs/compression.c     |  6 ++----
>>   fs/btrfs/inode.c           |  3 +--
>>   fs/btrfs/lzo.c             | 42 +++++++++++---------------------------
>>   fs/btrfs/raid56.c          | 10 ++++-----
>>   fs/btrfs/zlib.c            | 42 +++++++++++++-------------------------
>>   fs/btrfs/zstd.c            | 33 +++++++++++-------------------
>>   7 files changed, 49 insertions(+), 98 deletions(-)
>>
>
> I'd be concerned about the impact of this on SBC devices. All Fedora
> ARM images have zstd compression applied to them, and it would suck if
> we had a performance regression here because of this.

Sorry, I can't see the reason why it would cause performance drop or
higher memory usage.

The point of HIGHMEM is to work on archs where system can only access
memory below 4G reliably, any memory above 4G must be manually mapped
into the 4G range before access.

AFAIK it's only x86 using PAE needs this, and none of the ARM SoC uses
such feature.

So it shouldn't affect those ARM boards at all.

Thanks,
Qu

Re: [PATCH 0/6] Remove highmem allocations, kmap/kunmap

[Christoph Hellwig]

On Fri, Jul 09, 2021 at 07:53:39AM +0800, Qu Wenruo wrote:
> Sorry, I can't see the reason why it would cause performance drop or
> higher memory usage.
> 
> The point of HIGHMEM is to work on archs where system can only access
> memory below 4G reliably, any memory above 4G must be manually mapped
> into the 4G range before access.
> 
> AFAIK it's only x86 using PAE needs this, and none of the ARM SoC uses
> such feature.

Arm calls it LPAE, but otherwise it is the same.

Re: [PATCH 0/6] Remove highmem allocations, kmap/kunmap

[Qu Wenruo]

On 2021/7/9 下午2:46, Christoph Hellwig wrote:
> On Fri, Jul 09, 2021 at 07:53:39AM +0800, Qu Wenruo wrote:
>> Sorry, I can't see the reason why it would cause performance drop or
>> higher memory usage.
>>
>> The point of HIGHMEM is to work on archs where system can only access
>> memory below 4G reliably, any memory above 4G must be manually mapped
>> into the 4G range before access.
>>
>> AFAIK it's only x86 using PAE needs this, and none of the ARM SoC uses
>> such feature.
>
> Arm calls it LPAE, but otherwise it is the same.
>
Yeah, and I have never seen any toy ARM boards using LPAE neither.

Either those boards have too small memory to bother (<= 4G), or they go
directly aarch64.
(And most boards are even aarch64 with <= 4G ram, like RK3399 or Amlogic
SoCs, they are aarch64 but only support up to 4G physical RAM).

Way less historical burden here.

Thanks,
Qu

Re: [PATCH 0/6] Remove highmem allocations, kmap/kunmap

[Neal Gompa]

On Fri, Jul 9, 2021 at 3:12 AM Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:
>
>
>
> On 2021/7/9 下午2:46, Christoph Hellwig wrote:
> > On Fri, Jul 09, 2021 at 07:53:39AM +0800, Qu Wenruo wrote:
> >> Sorry, I can't see the reason why it would cause performance drop or
> >> higher memory usage.
> >>
> >> The point of HIGHMEM is to work on archs where system can only access
> >> memory below 4G reliably, any memory above 4G must be manually mapped
> >> into the 4G range before access.
> >>
> >> AFAIK it's only x86 using PAE needs this, and none of the ARM SoC uses
> >> such feature.
> >
> > Arm calls it LPAE, but otherwise it is the same.
> >
> Yeah, and I have never seen any toy ARM boards using LPAE neither.
>
> Either those boards have too small memory to bother (<= 4G), or they go
> directly aarch64.
> (And most boards are even aarch64 with <= 4G ram, like RK3399 or Amlogic
> SoCs, they are aarch64 but only support up to 4G physical RAM).
>

Raspberry Pi uses LPAE, I believe (RPi OS is 32-bit for support for
all RPi models).



-- 
真実はいつも一つ！/ Always, there's only one truth!

Re: [PATCH 0/6] Remove highmem allocations, kmap/kunmap

[Qu Wenruo]

On 2021/7/9 下午8:15, Neal Gompa wrote:
> On Fri, Jul 9, 2021 at 3:12 AM Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:
>>
>>
>>
>> On 2021/7/9 下午2:46, Christoph Hellwig wrote:
>>> On Fri, Jul 09, 2021 at 07:53:39AM +0800, Qu Wenruo wrote:
>>>> Sorry, I can't see the reason why it would cause performance drop or
>>>> higher memory usage.
>>>>
>>>> The point of HIGHMEM is to work on archs where system can only access
>>>> memory below 4G reliably, any memory above 4G must be manually mapped
>>>> into the 4G range before access.
>>>>
>>>> AFAIK it's only x86 using PAE needs this, and none of the ARM SoC uses
>>>> such feature.
>>>
>>> Arm calls it LPAE, but otherwise it is the same.
>>>
>> Yeah, and I have never seen any toy ARM boards using LPAE neither.
>>
>> Either those boards have too small memory to bother (<= 4G), or they go
>> directly aarch64.
>> (And most boards are even aarch64 with <= 4G ram, like RK3399 or Amlogic
>> SoCs, they are aarch64 but only support up to 4G physical RAM).
>>
>
> Raspberry Pi uses LPAE, I believe (RPi OS is 32-bit for support for
> all RPi models).
>
>
>
OK, you got me.

I thought everyone would just go upstream kernel for RPI4, but obviously
there are tons of guys still relying on the vendor kernel to provide
things like its VC4 user space tools.

But in that case, it's still not a big deal.

For RPI4 with <=4G ram, it's no difference.

For RPI4 with 8G ram, it will bring some impact, but I don't believe it
will be that big. We at most allocate 128K (which is super rare) for a
compressed extent.

And if the first 2 sectors can't compress well, we won't try again on
compressing the file, thus under most case it should be way smaller than
64K.
Not to mention such memory usage is transient, only when we read/write
compressed extent such memory is allocated.
And when the read/write is finished, the memory also get freed.


Don't forget that, our biggest memory usage, inode pages are all
allocated without HIGHMEM, just like XFS.
If it's causing problems, then it would have already caused more series
performance impact.

Thanks,
Qu

Re: [PATCH 0/6] Remove highmem allocations, kmap/kunmap

[Christoph Hellwig]

On Sat, Jul 10, 2021 at 05:50:47PM +0800, Qu Wenruo wrote:
> Don't forget that, our biggest memory usage, inode pages are all
> allocated without HIGHMEM, just like XFS.
> If it's causing problems, then it would have already caused more series
> performance impact.

What do you mean with "inode pages"?  The actual page cache data,
which should be the biggest users of memory in most file system
workloads is allocated using highmem for basicaly every file system.

Re: [PATCH 0/6] Remove highmem allocations, kmap/kunmap

[Qu Wenruo]

On 2021/7/10 下午7:15, Christoph Hellwig wrote:
> On Sat, Jul 10, 2021 at 05:50:47PM +0800, Qu Wenruo wrote:
>> Don't forget that, our biggest memory usage, inode pages are all
>> allocated without HIGHMEM, just like XFS.
>> If it's causing problems, then it would have already caused more series
>> performance impact.
>
> What do you mean with "inode pages"?  The actual page cache data,
> which should be the biggest users of memory in most file system
> workloads is allocated using highmem for basicaly every file system.
>
My bad...

I just did a quick grep for HIGHMEM, forgot that we get the gfp mask
from the address_space, which could have.

In fact I just checked the default address_space::gfp_mask, even on
x86_64 it indeed has HIGHMEM.

Thanks for pointing out this,
Qu