On 10 May 2019, at 9:45, Chris Mason wrote:
> On 10 May 2019, at 7:15, Johannes Thumshirn wrote: > >> Currently btrfs_csum_data() relied on the crc32c() wrapper around the >> crypto >> framework for calculating the CRCs. >> >> As we have our own crypto_shash structure in the fs_info now, we can >> directly call into the crypto framework without going trough the >> wrapper. > > It has been a while since I dug through the cryptoapi internals. I > have one vague and somewhat unfounded concern, where we're defining > the disk format to be whatever is returned from looking up sha256 or > crc32c in the crypto tables. I'm not really sure how to resolve this > since we obviously don't want our own sha256 implementation. > > I'm a little concerned about about btrfs_csum_data() and > btrfs_csum_final() below. We're using two different > SHASH_DESC_ON_STACK() and then overwriting internals > (shash_desc_ctx()) with the assumption that whatever we're doing is > going to be the same as using the same shash_desc struct for both the > update and final calls. I think we should be either using or adding a > helper to the crypto api for this. We're digging too deep into > cryptoapi private structs with the current usage. Looking at the callers of btrfs_csum_final() and btrfs_csum_data(), lets just pass the shash_desc from the callers. That way you don't have to poke at memcpy and shash_desc_ctx(). I'm a little worried about stack usage (at least 360 bytes), but worst case we can do some percpu gymnastics. -chris
