Messages in this thread | | | Date | Thu, 7 Jul 2022 23:32:01 +0200 | From | David Sterba <> | Subject | Re: [PATCH] btrfs: Replace kmap_atomic() with kmap_local_page() |
| |
On Mon, Jun 27, 2022 at 07:48:49PM +0200, 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(). > > Tested with xfstests on a QEMU + KVM 32-bits VM with 4GB RAM and booting a > kernel with HIGHMEM64GB enabled. > > [1] https://lore.kernel.org/linux-btrfs/20220601132545.GM20 > 633@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>
Added to the kmap patches, thanks.
> --- > > Tests of groups "quick" and "compress" output several errors largely due > to memory leaks and shift-out-of-bounds. However, these errors are exactly > the same which are output without this and other conversions of mine to use > kmap_local_page(). Therefore, it looks like these changes don't introduce > regressions. > > The previous RFC PATCH can be ignored: > https://lore.kernel.org/lkml/20220624084215.7287-1-fmdefrancesco@gmail.com/ > > With this patch, in fs/btrfs there are no longer call sites of kmap() and > kmap_atomic(). > > --- 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);
After some cleanups and simplifications in checksumming functions (that mapped the buffers) only kmap_atomic in insert_inline_extent remains, so the final patch will be a bit shorter.
> 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);
| |