linux-kernel - Re: [PATCH 0/3] btrfs: Replace kmap() 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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20220601132545.GM20633@twin.jikos.cz>
Date:   Wed, 1 Jun 2022 15:25:45 +0200
From:   David Sterba <dsterba@...e.cz>
To:     "Fabio M. De Francesco" <fmdefrancesco@...il.com>
Cc:     Chris Mason <clm@...com>, Josef Bacik <josef@...icpanda.com>,
        David Sterba <dsterba@...e.com>,
        Ira Weiny <ira.weiny@...el.com>, linux-btrfs@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 0/3] btrfs: Replace kmap() with kmap_local_page()

On Tue, May 31, 2022 at 04:53:32PM +0200, Fabio M. De Francesco wrote:
> This is the first series of patches aimed towards the conversion of Btrfs
> filesystem from the use of kmap() to kmap_local_page().

We've already had patches converting kmaps and you're changing the last
ones, so this is could be the last series, with two exceptions.

1) You've changed lzo.c and zlib. but the same kmap/kunmap pattern is
   used in zstd.c.

2) kmap_atomic in inode.c, so that's technically not kmap but it's said
   to be deprecated and also can be replaced by kmap_local_page. The
   context in check_compressed_csum is atomic (in end io) so the kmap
   hasn't been used there.

> tweed32:~ # btrfs check -p ~zoek/dev/btrfs.file

That won't verify if the kmap conversion is OK, it's a runtime thing
while 'check' verifies the data on device. Have you run any kind of
stress test with enabled compression before running the check?

> Opening filesystem to check...
> Checking filesystem on /home/zoek/dev/btrfs.file
> UUID: 897d65c5-1167-45b4-b811-2bfe74a320ca
> [1/7] checking root items                      (0:00:00 elapsed, 1774 items checked)
> [2/7] checking extents                         (0:00:00 elapsed, 135 items checked)
> [3/7] checking free space tree                 (0:00:00 elapsed, 4 items checked)
> [4/7] checking fs roots                        (0:00:00 elapsed, 104 items checked)
> [5/7] checking csums (without verifying data)  (0:00:00 elapsed, 205 items checked)
> [6/7] checking root refs                       (0:00:00 elapsed, 3 items checked)
> [7/7] checking quota groups skipped (not enabled on this FS)
> found 47394816 bytes used, no error found
> total csum bytes: 44268
> total tree bytes: 2064384
> total fs tree bytes: 1720320
> total extent tree bytes: 180224
> btree space waste bytes: 465350
> file data blocks allocated: 45330432
>  referenced 45330432
> 
> Fabio M. De Francesco (3):
>   btrfs: Replace kmap() with kmap_local_page() in inode.c
>   btrfs: Replace kmap() with kmap_local_page() in lzo.c
>   btrfs: Replace kmap() with kmap_local_page() in zlib.c

Please send patches converting zstd.c and the remaining kmap_atomic
usage in inode.c, otherwise the 3 patches are now in misc-next, thanks.