| Message ID | cover.1659116355.git.dsterba@suse.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Selectable checksum implementation | expand |
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 >
On Fri, Jul 29, 2022 at 02:01:41PM -0700, Boris Burkov wrote: > 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). That DEFAULT was not set is the problem, in patch 2 there's removed line setting it, this is a rebase error on my side, I was splitting the patches from longer series. Thanks for catching it, I'll resend. > 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. I think you need to enable in .config the accelerated version, CONFIG_CRYPTO_SHA256_SSSE3=m, and run KVM guest that has the CPU features available. I'm running it with '-host', virtio only for random number generator and block devices.