linux-kernel - [RFC PATCH] btrfs: Replace kmap_atomic() with kmap_local

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20220624084215.7287-1-fmdefrancesco@gmail.com>
Date:   Fri, 24 Jun 2022 10:42:15 +0200
From:   "Fabio M. De Francesco" <fmdefrancesco@...il.com>
To:     Chris Mason <clm@...com>, Josef Bacik <josef@...icpanda.com>,
        David Sterba <dsterba@...e.com>,
        Nick Terrell <terrelln@...com>,
        Chris Down <chris@...isdown.name>,
        Filipe Manana <fdmanana@...e.com>, Qu Wenruo <wqu@...e.com>,
        Nikolay Borisov <nborisov@...e.com>,
        Gabriel Niebler <gniebler@...e.com>,
        Ira Weiny <ira.weiny@...el.com>, linux-btrfs@...r.kernel.org,
        linux-kernel@...r.kernel.org
Cc:     "Fabio M. De Francesco" <fmdefrancesco@...il.com>,
        David Sterba <dsterba@...e.cz>
Subject: [RFC PATCH] btrfs: Replace kmap_atomic() with kmap_local_page()

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@...el.com>
Suggested-by: David Sterba <dsterba@...e.cz>
Signed-off-by: Fabio M. De Francesco <fmdefrancesco@...il.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