Hi I think the patch is OK, I just suggest these changes:
- set the limit for the number of bytes, rather than the number of blocks, because different dm-verity devices can have different block size - run some benchmarks and set the default values according to the benchmark results - increase the target version number - add a note about this feature to Documentation/admin-guide/device-mapper/verity.rst Mikulas On Tue, 25 Mar 2025, LongPing Wei wrote: > Calling verity_verify_io in bh for IO of all sizes is not suitable for > embedded devices. From our tests, it can improve the performance of 4K > synchronise random reads. > For example: > ./fio --name=rand_read --ioengine=psync --rw=randread --bs=4K \ > --direct=1 --numjobs=8 --runtime=60 --time_based --group_reporting \ > --filename=/dev/block/mapper/xx-verity > > But it will degrade the performance of 512K synchronise sequential reads > on our devices. > For example: > ./fio --name=read --ioengine=psync --rw=read --bs=512K --direct=1 \ > --numjobs=8 --runtime=60 --time_based --group_reporting \ > --filename=/dev/block/mapper/xx-verity > > A parameter array is introduced by this change. And users can modify the > default config by /sys/module/dm_verity/parameters/use_bh_blocks. > > The default limits for NONE/RT/BE/IDLE is set to UINT_MAX to keep the > original behaviour. > > Signed-off-by: LongPing Wei <weilongp...@oppo.com> > --- > drivers/md/dm-verity-target.c | 19 ++++++++++++++++++- > 1 file changed, 18 insertions(+), 1 deletion(-) > > diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c > index e86c1431b108..1e0d0920d6a9 100644 > --- a/drivers/md/dm-verity-target.c > +++ b/drivers/md/dm-verity-target.c > @@ -30,6 +30,7 @@ > #define DM_VERITY_ENV_VAR_NAME "DM_VERITY_ERR_BLOCK_NR" > > #define DM_VERITY_DEFAULT_PREFETCH_SIZE 262144 > +#define DM_VERITY_USE_BH_DEFAULT_BLOCKS UINT_MAX > > #define DM_VERITY_MAX_CORRUPTED_ERRS 100 > > @@ -49,6 +50,15 @@ static unsigned int dm_verity_prefetch_cluster = > DM_VERITY_DEFAULT_PREFETCH_SIZE > > module_param_named(prefetch_cluster, dm_verity_prefetch_cluster, uint, 0644); > > +static unsigned int dm_verity_use_bh_blocks[4] = { > + DM_VERITY_USE_BH_DEFAULT_BLOCKS, // IOPRIO_CLASS_NONE > + DM_VERITY_USE_BH_DEFAULT_BLOCKS, // IOPRIO_CLASS_RT > + DM_VERITY_USE_BH_DEFAULT_BLOCKS, // IOPRIO_CLASS_BE > + DM_VERITY_USE_BH_DEFAULT_BLOCKS // IOPRIO_CLASS_IDLE > +}; > + > +module_param_array_named(use_bh_blocks, dm_verity_use_bh_blocks, uint, NULL, > 0644); > + > static DEFINE_STATIC_KEY_FALSE(use_bh_wq_enabled); > > /* Is at least one dm-verity instance using ahash_tfm instead of shash_tfm? > */ > @@ -652,6 +662,12 @@ static void verity_bh_work(struct work_struct *w) > verity_finish_io(io, errno_to_blk_status(err)); > } > > +static inline bool verity_use_bh(unsigned int n_blocks, unsigned short > ioprio) > +{ > + return ioprio <= IOPRIO_CLASS_IDLE && > + n_blocks <= READ_ONCE(dm_verity_use_bh_blocks[ioprio]); > +} > + > static void verity_end_io(struct bio *bio) > { > struct dm_verity_io *io = bio->bi_private; > @@ -664,7 +680,8 @@ static void verity_end_io(struct bio *bio) > return; > } > > - if (static_branch_unlikely(&use_bh_wq_enabled) && io->v->use_bh_wq) { > + if (static_branch_unlikely(&use_bh_wq_enabled) && io->v->use_bh_wq && > + verity_use_bh(io->n_blocks, IOPRIO_PRIO_CLASS(bio->bi_ioprio))) > { > INIT_WORK(&io->bh_work, verity_bh_work); > queue_work(system_bh_wq, &io->bh_work); > } else { > -- > 2.34.1 >