Re: [dm-devel] [PATCH 4/6] dm crypt: Revisit the atomic argument passed to crypt_convert().

[Ignat Korchagin <ignat@cloudflare.com>]

On Sat, Feb 13, 2021 at 11:11 AM Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
>
> The atomic argument of crypto_convert() is used to decide if
> cond_resched() may be invoked.
>
> kcryptd_crypt_write_continue() and kcryptd_crypt_read_continue() pass
> true here but both are invoked by a worker where scheduling is possible.
>
> kcryptd_crypt_write_convert() is invoked from preemptible context even
> if DM_CRYPT_NO_WRITE_WORKQUEUE is set.
>
> Set the atomic argument to false in the three cases because
> cond_resched() is not forbidden.

"atomic" parameter name might be confusing here as usually it is
related to the execution context.
But here it has additional meaning - it is also bound to the
DM_NO_*_WORKQUEUE flags. Basically,
as a user, if I set these flags - my intention is that dm-crypt should
process requests ASAP, so I don't
want it to voluntarily yield CPU even if the context is preemptible.

> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>  drivers/md/dm-crypt.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
> index f5eafc32d32c5..1151a0108ae78 100644
> --- a/drivers/md/dm-crypt.c
> +++ b/drivers/md/dm-crypt.c
> @@ -2019,7 +2019,7 @@ static void kcryptd_crypt_write_continue(struct work_struct *work)
>         wait_for_completion(&ctx->restart);
>         reinit_completion(&ctx->restart);
>
> -       r = crypt_convert(cc, &io->ctx, true, false);
> +       r = crypt_convert(cc, &io->ctx, false, false);
>         if (r)
>                 io->error = r;
>         crypt_finished = atomic_dec_and_test(&ctx->cc_pending);
> @@ -2065,8 +2065,7 @@ static void kcryptd_crypt_write_convert(struct dm_crypt_io *io)
>         sector += bio_sectors(clone);
>
>         crypt_inc_pending(io);
> -       r = crypt_convert(cc, ctx,
> -                         test_bit(DM_CRYPT_NO_WRITE_WORKQUEUE, &cc->flags), true);
> +       r = crypt_convert(cc, ctx, false, true);
>         /*
>          * Crypto API backlogged the request, because its queue was full
>          * and we're in softirq context, so continue from a workqueue
> @@ -2110,7 +2109,7 @@ static void kcryptd_crypt_read_continue(struct work_struct *work)
>         wait_for_completion(&io->ctx.restart);
>         reinit_completion(&io->ctx.restart);
>
> -       r = crypt_convert(cc, &io->ctx, true, false);
> +       r = crypt_convert(cc, &io->ctx, false, false);
>         if (r)
>                 io->error = r;
>
> --
> 2.30.0
>

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel