Re: [PATCH v2 0/4] Selectable checksum implementation

[Boris Burkov <boris@bur.io>]

On Fri, Jul 29, 2022 at 07:42:42PM +0200, David Sterba wrote:
> Add a possibility to load accelerated checksum implementation after a
> filesystem has been mounted. Detailed description in patch 3.

I naively attempted to use this on my test VM and it blew up in mount.

What I ran:
mkfs.btrfs -f $dev --csum sha256
mount $dev $mnt

I got this oops:
https://pastebin.com/DAbSem7K

which indicates a null pointer access in this code:
(gdb) list *open_ctree+0x3c9
0xffffffff8210634e is in open_ctree (fs/btrfs/disk-io.c:2437).
2432
2433            /*
2434             * Find the fastest implementation available, but keep the slots
2435             * matching the type.
2436             */
2437            if (strstr(crypto_shash_driver_name(fs_info->csum_shash[CSUM_DEFAULT]),
2438                       "generic") != NULL) {
2439                    fs_info->csum_shash[CSUM_GENERIC] = csum_shash;
2440                    clear_bit(BTRFS_FS_CSUM_IMPL_FAST, &fs_info->flags);
2441            } else {

So I suspect the problem is in btrfs_init_csum_hash. Looking at it, two
things come to mind:
1. the old code used the name from the allocated hash, maybe we need to
check that against "generic" instead of the DEFAULT
2. we never set the DEFAULT entry to anything, so it isn't clear to me
how the use of the checksum would work after this (which it does seem to
do, to csum the super block).

Running misc-next, it mounted and reported it was using sha256-generic:
$ cat /sys/fs/btrfs/8ffa8559-f6c4-41d3-a806-c12738367d72/checksum
sha256 (sha256-generic)

I have not yet figured out how to get the virtio accel stuff to actually
work in the VM, so I haven't tested the happy case yet, but I figured
this report would be interesting on its own.

For further info, lsmod is empty on this VM. And here are the contents
of /proc/crypto:
https://pastebin.com/mZW6tq29

Hope that's helpful, and apologies if I didn't use the API correctly...

> 
> v2: rebased on misc-next
> 
> David Sterba (4):
>   btrfs: prepare more slots for checksum shash
>   btrfs: assign checksum shash slots on init
>   btrfs: add checksum implementation selection after mount
>   btrfs: sysfs: print all loaded csums implementations
> 
>  fs/btrfs/check-integrity.c |   4 +-
>  fs/btrfs/ctree.h           |  13 ++++-
>  fs/btrfs/disk-io.c         |  30 +++++++----
>  fs/btrfs/file-item.c       |   4 +-
>  fs/btrfs/inode.c           |   2 +-
>  fs/btrfs/scrub.c           |  12 ++---
>  fs/btrfs/super.c           |   2 -
>  fs/btrfs/sysfs.c           | 101 +++++++++++++++++++++++++++++++++++--
>  8 files changed, 141 insertions(+), 27 deletions(-)
> 
> -- 
> 2.36.1
>