On Wed, Dec 10, 2025 at 04:23:36PM +0100, Christoph Hellwig wrote:
> Calling mempool_alloc in a lot is not safe unless the maximum allocation
lot => loop
> + /*
> + * Try a bulk allocation first. This could leave random pages in the
> + * array unallocated, but we'll fix that up later in mempool_alloc_bulk.
> + *
> + * Note: alloc_pages_bulk needs the array to be zeroed, as it assumes
> + * any non-zero slot already contains a valid allocation.
> + */
> + memset(pages, 0, sizeof(struct page *) * nr_segs);
> + nr_allocated = alloc_pages_bulk(GFP_KERNEL, nr_segs, pages);
> + if (nr_allocated < nr_segs)
> + mempool_alloc_bulk(blk_crypto_bounce_page_pool, (void **)pages,
> + nr_segs, nr_allocated);
alloc_pages_bulk() is documented to fill in pages sequentially. So the
"random pages in the array unallocated" part seems misleading. This
also means that only the remaining portion needs to be passed to
mempool_alloc_bulk(), similar to blk_crypto_fallback_encrypt_endio().
> @@ -210,6 +249,7 @@ static void __blk_crypto_fallback_encrypt_bio(struct bio
> *src_bio,
> u64 curr_dun[BLK_CRYPTO_DUN_ARRAY_SIZE];
> struct scatterlist src, dst;
> union blk_crypto_iv iv;
> + struct page **enc_pages;
> unsigned int enc_idx;
> struct bio *enc_bio;
> unsigned int j;
> @@ -227,15 +267,13 @@ static void __blk_crypto_fallback_encrypt_bio(struct
> bio *src_bio,
>
> /* Encrypt each page in the source bio */
> new_bio:
> - enc_bio = blk_crypto_alloc_enc_bio(src_bio, nr_segs);
> + enc_bio = blk_crypto_alloc_enc_bio(src_bio, nr_segs, &enc_pages);
> enc_idx = 0;
> for (;;) {
> struct bio_vec src_bv =
> bio_iter_iovec(src_bio, src_bio->bi_iter);
> - struct page *enc_page;
> + struct page *enc_page = enc_pages[enc_idx];
>
> - enc_page = mempool_alloc(blk_crypto_bounce_page_pool,
> - GFP_NOIO);
> __bio_add_page(enc_bio, enc_page, src_bv.bv_len,
> src_bv.bv_offset);
[...]
> out_free_bounce_pages:
> enc_idx++;
> while (enc_idx > 0)
> mempool_free(enc_bio->bi_io_vec[--enc_idx].bv_page,
> blk_crypto_bounce_page_pool);
> bio_put(enc_bio);
This error handling path looks incorrect after this change. It needs to
free all the pages, not just the ones used so far.
Otherwise this looks good, thanks.
- Eric
