Re: [PATCH 8/8] dm-verity: Convert from tasklet to BH workqueue

From: Mikulas Patocka
Date: Wed Jan 31 2024 - 16:24:02 EST




On Mon, 29 Jan 2024, Tejun Heo wrote:

> The only generic interface to execute asynchronously in the BH context is
> tasklet; however, it's marked deprecated and has some design flaws. To
> replace tasklets, BH workqueue support was recently added. A BH workqueue
> behaves similarly to regular workqueues except that the queued work items
> are executed in the BH context.
>
> This patch converts dm-verity from tasklet to BH workqueue.
>
> This is a minimal conversion which doesn't rename the related names
> including the "try_verify_in_tasklet" option. If this patch is applied, a
> follow-up patch would be necessary. I couldn't decide whether the option
> name would need to be updated too.
>
> Only compile tested. I don't know how to verity.

Download the cryptsetup package with "git clone
https://gitlab.com/cryptsetup/cryptsetup"; and run the testsuite:
/autogen.sh && ./configure && make && make check

> Signed-off-by: Tejun Heo <tj@xxxxxxxxxx>
> Cc: Alasdair Kergon <agk@xxxxxxxxxx>
> Cc: Mike Snitzer <snitzer@xxxxxxxxxx>
> Cc: Mikulas Patocka <mpatocka@xxxxxxxxxx>
> Cc: dm-devel@xxxxxxxxxxxxxxx
> ---
> drivers/md/dm-verity-target.c | 8 ++++----
> drivers/md/dm-verity.h | 2 +-
> 2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c
> index 14e58ae70521..911261de2d08 100644
> --- a/drivers/md/dm-verity-target.c
> +++ b/drivers/md/dm-verity-target.c
> @@ -645,9 +645,9 @@ static void verity_work(struct work_struct *w)
> verity_finish_io(io, errno_to_blk_status(verity_verify_io(io)));
> }
>
> -static void verity_tasklet(unsigned long data)
> +static void verity_bh_work(struct work_struct *w)
> {
> - struct dm_verity_io *io = (struct dm_verity_io *)data;
> + struct dm_verity_io *io = container_of(w, struct dm_verity_io, bh_work);
> int err;
>
> io->in_tasklet = true;
> @@ -675,8 +675,8 @@ static void verity_end_io(struct bio *bio)
> }
>
> if (static_branch_unlikely(&use_tasklet_enabled) && io->v->use_tasklet) {
> - tasklet_init(&io->tasklet, verity_tasklet, (unsigned long)io);
> - tasklet_schedule(&io->tasklet);
> + INIT_WORK(&io->bh_work, verity_bh_work);
> + queue_work(system_bh_wq, &io->bh_work);
> } else {
> INIT_WORK(&io->work, verity_work);
> queue_work(io->v->verify_wq, &io->work);
> diff --git a/drivers/md/dm-verity.h b/drivers/md/dm-verity.h
> index f9d522c870e6..7c16f834f31a 100644
> --- a/drivers/md/dm-verity.h
> +++ b/drivers/md/dm-verity.h
> @@ -83,7 +83,7 @@ struct dm_verity_io {
> struct bvec_iter iter;
>
> struct work_struct work;
> - struct tasklet_struct tasklet;
> + struct work_struct bh_work;
>
> /*
> * Three variably-size fields follow this struct:

Do we really need two separate work_structs here? They are never submitted
concurrently, so I think that one would be enough. Or, am I missing
something?

Mikulas