On Tue, 2 Jun 2026 at 01:09, Eric Biggers <[email protected]> wrote:
> On Wed, May 13, 2026 at 10:52:39AM +0200, Daniel Vacek wrote:
> > @@ -330,6 +330,17 @@ static void __blk_crypto_fallback_encrypt_bio(struct
> > bio *src_bio,
> > }
> > }
> >
> > + /* Process the encrypted bio before we submit it. */
> > + if (bc->bc_key->crypto_cfg.process_bio) {
> > + blk_status_t status;
> > +
> > + status = bc->bc_key->crypto_cfg.process_bio(src_bio, enc_bio);
> > + if (status != BLK_STS_OK) {
> > + enc_bio->bi_status = status;
> > + goto out_free_enc_bio;
> > + }
> > + }
>
> This looks very out of place in this function, because this function
> potentially submits multiple encrypted bios whereas this code assumes
> there's just one. I think we could at least use a comment here, as well
> as a WARN_ON_ONCE(!bc->bc_key->crypto_cfg.process_bio) in the case where
> an additional bio is being submitted.
While this funcion may submit multiple encrypted bios in general case,
the check below makes sure that in case of crypto_cfg.process_bio
callback splitting is not allowed and only single encrypted bio will
be submitted. The WARN_ON would be impossible to hit. That's why no
handling is implemented with the in-loop submit.
> > + /*
> > + * We cannot split bio's that have process_bio, as they require the
> > original bio.
> > + * The upper layer must make sure to limit the submitted bio's
> > appropriately.
> > + */
> > + if (bio_segments(src_bio) > BIO_MAX_VECS &&
> > bc->bc_key->crypto_cfg.process_bio) {
>
> Checks should be reversed so that the expensive bio_segments() check
> only gets done if process_bio is non-NULL.
>
> (Sashiko spotted this too, by the way)
Yup, true.
> > + * @process_bio: the call back if the upper layer needs to process the
> > encrypted
> > + * bio
>
> or NULL if no such callback is needed.
I'll add this.
> > + * @proces_bio: optional callback to process encrypted bios.
>
> Typo.
Already fixed. Sashiko also noticed.
> By the way, after writing kerneldoc you should run the kerneldoc
> checker, as it will find things like this:
>
> $ scripts/kernel-doc -v -none include/linux/blk-crypto.h
> Info: include/linux/blk-crypto.h:79 Scanning doc for struct blk_crypto_config
> Warning: include/linux/blk-crypto.h:95 struct member 'process_bio' not
> described in 'blk_crypto_config'
Hey, this is a good hint. Thanks, I'll keep that in mind.
--nX
