Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

blk-crypto: optimize bio splitting in blk_crypto_fallback_encrypt_bio

The current code in blk_crypto_fallback_encrypt_bio is inefficient and
prone to deadlocks under memory pressure: It first walks the passed in
plaintext bio to see how much of it can fit into a single encrypted
bio using up to BIO_MAX_VEC PAGE_SIZE segments, and then allocates a
plaintext clone that fits the size, only to allocate another bio for
the ciphertext later. While the plaintext clone uses a bioset to avoid
deadlocks when allocations could fail, the ciphertex one uses bio_kmalloc
which is a no-go in the file system I/O path.

Switch blk_crypto_fallback_encrypt_bio to walk the source plaintext bio
while consuming bi_iter without cloning it, and instead allocate a
ciphertext bio at the beginning and whenever we fille up the previous
one. The existing bio_set for the plaintext clones is reused for the
ciphertext bios to remove the deadlock risk.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Eric Biggers <ebiggers@kernel.org>
Signed-off-by: Jens Axboe <axboe@kernel.dk>

authored by

Christoph Hellwig and committed by
Jens Axboe
b37fbce4 aefc2a1f

+76 -105
+76 -105
block/blk-crypto-fallback.c
··· 81 81 static struct blk_crypto_profile *blk_crypto_fallback_profile; 82 82 static struct workqueue_struct *blk_crypto_wq; 83 83 static mempool_t *blk_crypto_bounce_page_pool; 84 - static struct bio_set crypto_bio_split; 84 + static struct bio_set enc_bio_set; 85 85 86 86 /* 87 87 * This is the key we set when evicting a keyslot. This *should* be the all 0's ··· 150 150 mempool_free(enc_bio->bi_io_vec[i].bv_page, 151 151 blk_crypto_bounce_page_pool); 152 152 153 - src_bio->bi_status = enc_bio->bi_status; 153 + if (enc_bio->bi_status) 154 + cmpxchg(&src_bio->bi_status, 0, enc_bio->bi_status); 154 155 155 - bio_uninit(enc_bio); 156 - kfree(enc_bio); 156 + bio_put(enc_bio); 157 157 bio_endio(src_bio); 158 158 } 159 159 160 - static struct bio *blk_crypto_fallback_clone_bio(struct bio *bio_src) 160 + static struct bio *blk_crypto_alloc_enc_bio(struct bio *bio_src, 161 + unsigned int nr_segs) 161 162 { 162 - unsigned int nr_segs = bio_segments(bio_src); 163 - struct bvec_iter iter; 164 - struct bio_vec bv; 165 163 struct bio *bio; 166 164 167 - bio = bio_kmalloc(nr_segs, GFP_NOIO); 168 - if (!bio) 169 - return NULL; 170 - bio_init_inline(bio, bio_src->bi_bdev, nr_segs, bio_src->bi_opf); 165 + bio = bio_alloc_bioset(bio_src->bi_bdev, nr_segs, bio_src->bi_opf, 166 + GFP_NOIO, &enc_bio_set); 171 167 if (bio_flagged(bio_src, BIO_REMAPPED)) 172 168 bio_set_flag(bio, BIO_REMAPPED); 169 + bio->bi_private = bio_src; 170 + bio->bi_end_io = blk_crypto_fallback_encrypt_endio; 173 171 bio->bi_ioprio = bio_src->bi_ioprio; 174 172 bio->bi_write_hint = bio_src->bi_write_hint; 175 173 bio->bi_write_stream = bio_src->bi_write_stream; 176 174 bio->bi_iter.bi_sector = bio_src->bi_iter.bi_sector; 177 - bio->bi_iter.bi_size = bio_src->bi_iter.bi_size; 178 - 179 - bio_for_each_segment(bv, bio_src, iter) 180 - bio->bi_io_vec[bio->bi_vcnt++] = bv; 181 - 182 175 bio_clone_blkg_association(bio, bio_src); 183 - 184 176 return bio; 185 177 } 186 178 ··· 200 208 return true; 201 209 } 202 210 203 - static bool blk_crypto_fallback_split_bio_if_needed(struct bio **bio_ptr) 204 - { 205 - struct bio *bio = *bio_ptr; 206 - unsigned int i = 0; 207 - unsigned int num_sectors = 0; 208 - struct bio_vec bv; 209 - struct bvec_iter iter; 210 - 211 - bio_for_each_segment(bv, bio, iter) { 212 - num_sectors += bv.bv_len >> SECTOR_SHIFT; 213 - if (++i == BIO_MAX_VECS) 214 - break; 215 - } 216 - 217 - if (num_sectors < bio_sectors(bio)) { 218 - bio = bio_submit_split_bioset(bio, num_sectors, 219 - &crypto_bio_split); 220 - if (!bio) 221 - return false; 222 - 223 - *bio_ptr = bio; 224 - } 225 - 226 - return true; 227 - } 228 - 229 211 union blk_crypto_iv { 230 212 __le64 dun[BLK_CRYPTO_DUN_ARRAY_SIZE]; 231 213 u8 bytes[BLK_CRYPTO_MAX_IV_SIZE]; ··· 223 257 */ 224 258 static void blk_crypto_fallback_encrypt_bio(struct bio *src_bio) 225 259 { 226 - struct bio *enc_bio; 227 - struct bio_crypt_ctx *bc; 228 - struct blk_crypto_keyslot *slot; 229 - int data_unit_size; 260 + struct bio_crypt_ctx *bc = src_bio->bi_crypt_context; 261 + int data_unit_size = bc->bc_key->crypto_cfg.data_unit_size; 230 262 struct skcipher_request *ciph_req = NULL; 263 + struct blk_crypto_keyslot *slot; 231 264 DECLARE_CRYPTO_WAIT(wait); 232 265 u64 curr_dun[BLK_CRYPTO_DUN_ARRAY_SIZE]; 233 266 struct scatterlist src, dst; 234 267 union blk_crypto_iv iv; 235 - unsigned int i, j; 236 - blk_status_t blk_st; 237 - 238 - /* Split the bio if it's too big for single page bvec */ 239 - if (!blk_crypto_fallback_split_bio_if_needed(&src_bio)) 240 - goto out_endio; 241 - 242 - bc = src_bio->bi_crypt_context; 243 - data_unit_size = bc->bc_key->crypto_cfg.data_unit_size; 244 - 245 - /* Allocate bounce bio for encryption */ 246 - enc_bio = blk_crypto_fallback_clone_bio(src_bio); 247 - if (!enc_bio) { 248 - src_bio->bi_status = BLK_STS_RESOURCE; 249 - goto out_endio; 250 - } 268 + unsigned int nr_enc_pages, enc_idx; 269 + struct bio *enc_bio; 270 + blk_status_t status; 271 + unsigned int i; 251 272 252 273 /* 253 274 * Get a blk-crypto-fallback keyslot that contains a crypto_skcipher for 254 275 * this bio's algorithm and key. 255 276 */ 256 - blk_st = blk_crypto_get_keyslot(blk_crypto_fallback_profile, 277 + status = blk_crypto_get_keyslot(blk_crypto_fallback_profile, 257 278 bc->bc_key, &slot); 258 - if (blk_st != BLK_STS_OK) { 259 - src_bio->bi_status = blk_st; 260 - goto out_put_enc_bio; 279 + if (status != BLK_STS_OK) { 280 + src_bio->bi_status = status; 281 + bio_endio(src_bio); 282 + return; 261 283 } 262 284 263 285 /* and then allocate an skcipher_request for it */ 264 286 if (!blk_crypto_fallback_alloc_cipher_req(slot, &ciph_req, &wait)) { 265 287 src_bio->bi_status = BLK_STS_RESOURCE; 288 + bio_endio(src_bio); 266 289 goto out_release_keyslot; 267 290 } 268 291 ··· 262 307 skcipher_request_set_crypt(ciph_req, &src, &dst, data_unit_size, 263 308 iv.bytes); 264 309 265 - /* Encrypt each page in the bounce bio */ 266 - for (i = 0; i < enc_bio->bi_vcnt; i++) { 267 - struct bio_vec *enc_bvec = &enc_bio->bi_io_vec[i]; 268 - struct page *plaintext_page = enc_bvec->bv_page; 269 - struct page *ciphertext_page = 270 - mempool_alloc(blk_crypto_bounce_page_pool, GFP_NOIO); 310 + /* 311 + * Encrypt each page in the source bio. Because the source bio could 312 + * have bio_vecs that span more than a single page, but the encrypted 313 + * bios are limited to a single page per bio_vec, this can generate 314 + * more than a single encrypted bio per source bio. 315 + */ 316 + new_bio: 317 + nr_enc_pages = min(bio_segments(src_bio), BIO_MAX_VECS); 318 + enc_bio = blk_crypto_alloc_enc_bio(src_bio, nr_enc_pages); 319 + enc_idx = 0; 320 + for (;;) { 321 + struct bio_vec src_bv = 322 + bio_iter_iovec(src_bio, src_bio->bi_iter); 323 + struct page *enc_page; 271 324 272 - enc_bvec->bv_page = ciphertext_page; 325 + enc_page = mempool_alloc(blk_crypto_bounce_page_pool, 326 + GFP_NOIO); 327 + __bio_add_page(enc_bio, enc_page, src_bv.bv_len, 328 + src_bv.bv_offset); 273 329 274 - if (!ciphertext_page) { 275 - src_bio->bi_status = BLK_STS_RESOURCE; 276 - goto out_free_bounce_pages; 277 - } 330 + sg_set_page(&src, src_bv.bv_page, data_unit_size, 331 + src_bv.bv_offset); 332 + sg_set_page(&dst, enc_page, data_unit_size, src_bv.bv_offset); 278 333 279 - sg_set_page(&src, plaintext_page, data_unit_size, 280 - enc_bvec->bv_offset); 281 - sg_set_page(&dst, ciphertext_page, data_unit_size, 282 - enc_bvec->bv_offset); 334 + /* 335 + * Increment the index now that the encrypted page is added to 336 + * the bio. This is important for the error unwind path. 337 + */ 338 + enc_idx++; 283 339 284 - /* Encrypt each data unit in this page */ 285 - for (j = 0; j < enc_bvec->bv_len; j += data_unit_size) { 340 + /* 341 + * Encrypt each data unit in this page. 342 + */ 343 + for (i = 0; i < src_bv.bv_len; i += data_unit_size) { 286 344 blk_crypto_dun_to_iv(curr_dun, &iv); 287 345 if (crypto_wait_req(crypto_skcipher_encrypt(ciph_req), 288 346 &wait)) { 289 - i++; 290 - src_bio->bi_status = BLK_STS_IOERR; 291 - goto out_free_bounce_pages; 347 + bio_io_error(enc_bio); 348 + goto out_free_request; 292 349 } 293 350 bio_crypt_dun_increment(curr_dun, 1); 294 351 src.offset += data_unit_size; 295 352 dst.offset += data_unit_size; 296 353 } 354 + 355 + bio_advance_iter_single(src_bio, &src_bio->bi_iter, 356 + src_bv.bv_len); 357 + if (!src_bio->bi_iter.bi_size) 358 + break; 359 + 360 + if (enc_idx == nr_enc_pages) { 361 + /* 362 + * For each additional encrypted bio submitted, 363 + * increment the source bio's remaining count. Each 364 + * encrypted bio's completion handler calls bio_endio on 365 + * the source bio, so this keeps the source bio from 366 + * completing until the last encrypted bio does. 367 + */ 368 + bio_inc_remaining(src_bio); 369 + submit_bio(enc_bio); 370 + goto new_bio; 371 + } 297 372 } 298 373 299 - enc_bio->bi_private = src_bio; 300 - enc_bio->bi_end_io = blk_crypto_fallback_encrypt_endio; 301 - skcipher_request_free(ciph_req); 302 - blk_crypto_put_keyslot(slot); 303 374 submit_bio(enc_bio); 304 - return; 305 - 306 - out_free_bounce_pages: 307 - while (i > 0) 308 - mempool_free(enc_bio->bi_io_vec[--i].bv_page, 309 - blk_crypto_bounce_page_pool); 375 + out_free_request: 310 376 skcipher_request_free(ciph_req); 311 377 out_release_keyslot: 312 378 blk_crypto_put_keyslot(slot); 313 - out_put_enc_bio: 314 - bio_uninit(enc_bio); 315 - kfree(enc_bio); 316 - out_endio: 317 - bio_endio(src_bio); 318 379 } 319 380 320 381 /* ··· 504 533 505 534 get_random_bytes(blank_key, sizeof(blank_key)); 506 535 507 - err = bioset_init(&crypto_bio_split, 64, 0, 0); 536 + err = bioset_init(&enc_bio_set, 64, 0, BIOSET_NEED_BVECS); 508 537 if (err) 509 538 goto out; 510 539 ··· 574 603 fail_free_profile: 575 604 kfree(blk_crypto_fallback_profile); 576 605 fail_free_bioset: 577 - bioset_exit(&crypto_bio_split); 606 + bioset_exit(&enc_bio_set); 578 607 out: 579 608 return err; 580 609 }