Hi Chao, On Tue, Apr 17, 2018 at 05:13:12PM +0800, Chao Yu wrote: > > + > > +static void bio_post_read_processing(struct bio_post_read_ctx *ctx); > > + > > +static void decrypt_work(struct work_struct *work) > > +{ > > + struct bio_post_read_ctx *ctx = > > + container_of(work, struct bio_post_read_ctx, work); > > + > > + fscrypt_decrypt_bio(ctx->bio); > > + > > + bio_post_read_processing(ctx); > > +} > > + > > +static void bio_post_read_processing(struct bio_post_read_ctx *ctx) > > +{ > > + switch (++ctx->cur_step) { > > + case STEP_DECRYPT: > > + if (ctx->enabled_steps & (1 << STEP_DECRYPT)) { > > + INIT_WORK(&ctx->work, decrypt_work); > > + fscrypt_enqueue_decrypt_work(&ctx->work); > > + return; > > + } > > + ctx->cur_step++; > > + /* fall-through */ > > + default: > > + __read_end_io(ctx->bio); > > + } > > How about introducing __bio_post_read_processing() > > switch (step) { > case STEP_DECRYPT: > ... > break; > case STEP_COMPRESS: > ... > break; > case STEP_GENERIC: > __read_end_io; > break; > ... > } > > Then we can customize flexible read processes like: > > bio_post_read_processing() > { > if (encrypt_enabled) > __bio_post_read_processing(, STEP_DECRYPT); > if (compress_enabled) > __bio_post_read_processing(, STEP_COMPRESS); > __bio_post_read_processing(, STEP_GENERIC); > } > > Or other flow. If I understand correctly, you're suggesting that all the steps be done in a single workqueue item? The problem with that is that the verity work will require I/O to the file to read hashes, which may need STEP_DECRYPT. Hence, decryption and verity will need separate workqueues. > > @@ -481,29 +537,33 @@ static struct bio *f2fs_grab_read_bio(struct inode *inode, block_t blkaddr, > > unsigned nr_pages) > > { > > struct f2fs_sb_info *sbi = F2FS_I_SB(inode); > > - struct fscrypt_ctx *ctx = NULL; > > struct bio *bio; > > - > > - if (f2fs_encrypted_file(inode)) { > > - ctx = fscrypt_get_ctx(inode, GFP_NOFS); > > - if (IS_ERR(ctx)) > > - return ERR_CAST(ctx); > > - > > - /* wait the page to be moved by cleaning */ > > - f2fs_wait_on_block_writeback(sbi, blkaddr); > > - } > > + struct bio_post_read_ctx *ctx; > > + unsigned int post_read_steps = 0; > > > > bio = f2fs_bio_alloc(sbi, min_t(int, nr_pages, BIO_MAX_PAGES), false); > > - if (!bio) { > > - if (ctx) > > - fscrypt_release_ctx(ctx); > > + if (!bio) > > return ERR_PTR(-ENOMEM); > > - } > > f2fs_target_device(sbi, blkaddr, bio); > > bio->bi_end_io = f2fs_read_end_io; > > - bio->bi_private = ctx; > > bio->bi_private = NULL; > I don't see why. ->bi_private is NULL by default. > > + bio_post_read_ctx_pool = > > + mempool_create_slab_pool(128, bio_post_read_ctx_cache); > > #define MAX_POST_READ_CACHE_SIZE 128 > Yes, that makes sense. - Eric -- To unsubscribe from this list: send the line "unsubscribe linux-fscrypt" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
![]() |