[RFC PATCH] btrfs: Replace kmap_atomic() with kmap_local_page()

[RFC PATCH] btrfs: Replace kmap_atomic() with kmap_local_page()

[Fabio M. De Francesco]

kmap_atomic() is being deprecated in favor of kmap_local_page() where it
is feasible. With kmap_local_page() mappings are per thread, CPU local,
and not globally visible.

As far as I can see, the kmap_atomic() calls in compression.c and in
inode.c can be safely converted.

Above all else, David Sterba has confirmed that "The context in
check_compressed_csum is atomic [...]" and that "kmap_atomic() in inode.c
[...] also can be replaced by kmap_local_page().".[1]

Therefore, convert all kmap_atomic() calls currently still left in fs/btrfs
to kmap_local_page().

This is an RFC only because, testing with "./check -g quick" (xfstests) on
a QEMU + KVM 32-bits VM with 4GB RAM and booting a kernel with HIGHMEM64GB
enabled, outputs several errors. These errors seem to be exactly the same
which are being output without this patch. It apparently seems that these
changes don't introduce further errors, however I'd like to ask for
comments before sending a "real" patch.

With this patch, there are no more call sites for kmap() and kmap_atomic()
in fs/btrfs.

[1] https://lore.kernel.org/linux-btrfs/20220601132545.GM20633@twin.jikos.cz/

Suggested-by: Ira Weiny <ira.weiny@intel.com>
Suggested-by: David Sterba <dsterba@suse.cz>
Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
---
 fs/btrfs/compression.c |  4 ++--
 fs/btrfs/inode.c       | 12 ++++++------
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index f4564f32f6d9..b49719ae45b4 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -175,10 +175,10 @@ 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 = kmap_local_page(page);
 			crypto_shash_digest(shash, kaddr + pg_offset,
 					    sectorsize, csum);
-			kunmap_atomic(kaddr);
+			kunmap_local(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 7d84d57a0653..5fb4f6e929e5 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -332,9 +332,9 @@ 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 = kmap_local_page(cpage);
 			write_extent_buffer(leaf, kaddr, ptr, cur_size);
-			kunmap_atomic(kaddr);
+			kunmap_local(kaddr);
 
 			i++;
 			ptr += cur_size;
@@ -345,9 +345,9 @@ static int insert_inline_extent(struct btrfs_trans_handle *trans,
 	} else {
 		page = find_get_page(inode->vfs_inode.i_mapping, 0);
 		btrfs_set_file_extent_compression(leaf, ei, 0);
-		kaddr = kmap_atomic(page);
+		kaddr = kmap_local_page(page);
 		write_extent_buffer(leaf, kaddr, ptr, size);
-		kunmap_atomic(kaddr);
+		kunmap_local(kaddr);
 		put_page(page);
 	}
 	btrfs_mark_buffer_dirty(leaf);
@@ -3355,11 +3355,11 @@ static int check_data_csum(struct inode *inode, struct btrfs_bio *bbio,
 	offset_sectors = bio_offset >> fs_info->sectorsize_bits;
 	csum_expected = ((u8 *)bbio->csum) + offset_sectors * csum_size;
 
-	kaddr = kmap_atomic(page);
+	kaddr = kmap_local_page(page);
 	shash->tfm = fs_info->csum_shash;
 
 	crypto_shash_digest(shash, kaddr + pgoff, len, csum);
-	kunmap_atomic(kaddr);
+	kunmap_local(kaddr);
 
 	if (memcmp(csum, csum_expected, csum_size))
 		goto zeroit;
-- 
2.36.1

Re: [RFC PATCH] btrfs: Replace kmap_atomic() with kmap_local_page()

[Fabio M. De Francesco]

On venerdì 24 giugno 2022 10:42:15 CEST Fabio M. De Francesco wrote:
> kmap_atomic() is being deprecated in favor of kmap_local_page() where it
> is feasible. With kmap_local_page() mappings are per thread, CPU local,
> and not globally visible.
> 
> As far as I can see, the kmap_atomic() calls in compression.c and in
> inode.c can be safely converted.
> 
> Above all else, David Sterba has confirmed that "The context in
> check_compressed_csum is atomic [...]" and that "kmap_atomic() in inode.c
> [...] also can be replaced by kmap_local_page().".[1]
> 
> Therefore, convert all kmap_atomic() calls currently still left in fs/
btrfs
> to kmap_local_page().
> 
> This is an RFC only because, testing with "./check -g quick" (xfstests) 
on
> a QEMU + KVM 32-bits VM with 4GB RAM and booting a kernel with 
HIGHMEM64GB
> enabled, outputs several errors. These errors seem to be exactly the same
> which are being output without this patch. It apparently seems that these
> changes don't introduce further errors, however I'd like to ask for
> comments before sending a "real" patch.
> 
> With this patch, there are no more call sites for kmap() and 
kmap_atomic()
> in fs/btrfs.
> 
> [1] https://lore.kernel.org/linux-btrfs/
20220601132545.GM20633@twin.jikos.cz/
> 
> Suggested-by: Ira Weiny <ira.weiny@intel.com>
> Suggested-by: David Sterba <dsterba@suse.cz>
> Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> ---
>  fs/btrfs/compression.c |  4 ++--
>  fs/btrfs/inode.c       | 12 ++++++------
>  2 files changed, 8 insertions(+), 8 deletions(-)
> 

Please discard this RFC.

I've just submitted a real patch:

https://lore.kernel.org/lkml/20220627174849.29962-1-fmdefrancesco@gmail.com/

Thanks,

Fabio