diff mbox series

[SRU,B,F,G,1/1] dm crypt: add flags to optionally bypass kcryptd workqueues

Message ID 20210518025946.744283-2-gerald.yang@canonical.com
State New
Headers show
Series dm crypt: add flags to optionally | expand

Commit Message

Gerald Yang May 18, 2021, 2:59 a.m. UTC 
From: Ignat Korchagin <ignat@cloudflare.com>

BugLink: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1910976

This is a follow up to [1] that detailed latency problems associated
with dm-crypt's use of workqueues when processing IO.

Current dm-crypt implementation creates a significant IO performance
overhead (at least on small IO block sizes) for both latency and
throughput. We suspect offloading IO request processing into
workqueues and async threads is more harmful these days with the
modern fast storage. I also did some digging into the dm-crypt git
history and much of this async processing is not needed anymore,
because the reasons it was added are mostly gone from the kernel. More
details can be found in [2] (see "Git archeology" section).

This change adds DM_CRYPT_NO_READ_WORKQUEUE and
DM_CRYPT_NO_WRITE_WORKQUEUE flags for read and write BIOs, which
direct dm-crypt to not offload crypto operations into kcryptd
workqueues.  In addition, writes are not buffered to be sorted in the
dm-crypt red-black tree, but dispatched immediately. For cases, where
crypto operations cannot happen (hard interrupt context, for example
the read path of some NVME drivers), we offload the work to a tasklet
rather than a workqueue.

These flags only ensure no async BIO processing in the dm-crypt
module. It is worth noting that some Crypto API implementations may
offload encryption into their own workqueues, which are independent of
the dm-crypt and its configuration. However upon enabling these new
flags dm-crypt will instruct Crypto API not to backlog crypto
requests.

To give an idea of the performance gains for certain workloads,
consider the script, and results when tested against various
devices, detailed here:
https://www.redhat.com/archives/dm-devel/2020-July/msg00138.html

[1]: https://www.spinics.net/lists/dm-crypt/msg07516.html
[2]: https://blog.cloudflare.com/speeding-up-linux-disk-encryption/

Signed-off-by: Ignat Korchagin <ignat@cloudflare.com>
Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>
Reviewed-by: Bob Liu <bob.liu@oracle.com>
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
(cherry picked from commit 39d42fa96ba1b7d2544db3f8ed5da8fb0d5cb877)
Signed-off-by: Gerald Yang <gerald.yang@canonical.com>
---
 drivers/md/dm-crypt.c | 50 ++++++++++++++++++++++++++++++++++++-------
 1 file changed, 42 insertions(+), 8 deletions(-)

Comments

Gerald Yang May 18, 2021, 3:04 a.m. UTC | #1 
I sent out the same SRU email twice, sorry about that
please check the first one and ignore the second one

Thanks,
Gerald

On Tue, May 18, 2021 at 11:00 AM Gerald Yang <gerald.yang@canonical.com>
wrote:

> From: Ignat Korchagin <ignat@cloudflare.com>
>
> BugLink: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1910976
>
> This is a follow up to [1] that detailed latency problems associated
> with dm-crypt's use of workqueues when processing IO.
>
> Current dm-crypt implementation creates a significant IO performance
> overhead (at least on small IO block sizes) for both latency and
> throughput. We suspect offloading IO request processing into
> workqueues and async threads is more harmful these days with the
> modern fast storage. I also did some digging into the dm-crypt git
> history and much of this async processing is not needed anymore,
> because the reasons it was added are mostly gone from the kernel. More
> details can be found in [2] (see "Git archeology" section).
>
> This change adds DM_CRYPT_NO_READ_WORKQUEUE and
> DM_CRYPT_NO_WRITE_WORKQUEUE flags for read and write BIOs, which
> direct dm-crypt to not offload crypto operations into kcryptd
> workqueues.  In addition, writes are not buffered to be sorted in the
> dm-crypt red-black tree, but dispatched immediately. For cases, where
> crypto operations cannot happen (hard interrupt context, for example
> the read path of some NVME drivers), we offload the work to a tasklet
> rather than a workqueue.
>
> These flags only ensure no async BIO processing in the dm-crypt
> module. It is worth noting that some Crypto API implementations may
> offload encryption into their own workqueues, which are independent of
> the dm-crypt and its configuration. However upon enabling these new
> flags dm-crypt will instruct Crypto API not to backlog crypto
> requests.
>
> To give an idea of the performance gains for certain workloads,
> consider the script, and results when tested against various
> devices, detailed here:
> https://www.redhat.com/archives/dm-devel/2020-July/msg00138.html
>
> [1]: https://www.spinics.net/lists/dm-crypt/msg07516.html
> [2]: https://blog.cloudflare.com/speeding-up-linux-disk-encryption/
>
> Signed-off-by: Ignat Korchagin <ignat@cloudflare.com>
> Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>
> Reviewed-by: Bob Liu <bob.liu@oracle.com>
> Signed-off-by: Mike Snitzer <snitzer@redhat.com>
> (cherry picked from commit 39d42fa96ba1b7d2544db3f8ed5da8fb0d5cb877)
> Signed-off-by: Gerald Yang <gerald.yang@canonical.com>
> ---
>  drivers/md/dm-crypt.c | 50 ++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 42 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
> index d85648b9c247..91aa99e1d698 100644
> --- a/drivers/md/dm-crypt.c
> +++ b/drivers/md/dm-crypt.c
> @@ -67,6 +67,7 @@ struct dm_crypt_io {
>         u8 *integrity_metadata;
>         bool integrity_metadata_from_pool;
>         struct work_struct work;
> +       struct tasklet_struct tasklet;
>
>         struct convert_context ctx;
>
> @@ -120,7 +121,8 @@ struct iv_tcw_private {
>   * and encrypts / decrypts at the same time.
>   */
>  enum flags { DM_CRYPT_SUSPENDED, DM_CRYPT_KEY_VALID,
> -            DM_CRYPT_SAME_CPU, DM_CRYPT_NO_OFFLOAD };
> +            DM_CRYPT_SAME_CPU, DM_CRYPT_NO_OFFLOAD,
> +            DM_CRYPT_NO_READ_WORKQUEUE, DM_CRYPT_NO_WRITE_WORKQUEUE };
>
>  enum cipher_flags {
>         CRYPT_MODE_INTEGRITY_AEAD,      /* Use authenticated mode for
> cihper */
> @@ -1211,7 +1213,7 @@ static void crypt_free_req(struct crypt_config *cc,
> void *req, struct bio *base_
>   * Encrypt / decrypt data from one bio to another one (can be the same
> one)
>   */
>  static blk_status_t crypt_convert(struct crypt_config *cc,
> -                        struct convert_context *ctx)
> +                        struct convert_context *ctx, bool atomic)
>  {
>         unsigned int tag_offset = 0;
>         unsigned int sector_step = cc->sector_size >> SECTOR_SHIFT;
> @@ -1254,7 +1256,8 @@ static blk_status_t crypt_convert(struct
> crypt_config *cc,
>                         atomic_dec(&ctx->cc_pending);
>                         ctx->cc_sector += sector_step;
>                         tag_offset++;
> -                       cond_resched();
> +                       if (!atomic)
> +                               cond_resched();
>                         continue;
>                 /*
>                  * There was a data integrity error.
> @@ -1580,7 +1583,8 @@ static void kcryptd_crypt_write_io_submit(struct
> dm_crypt_io *io, int async)
>
>         clone->bi_iter.bi_sector = cc->start + io->sector;
>
> -       if (likely(!async) && test_bit(DM_CRYPT_NO_OFFLOAD, &cc->flags)) {
> +       if ((likely(!async) && test_bit(DM_CRYPT_NO_OFFLOAD, &cc->flags))
> ||
> +           test_bit(DM_CRYPT_NO_WRITE_WORKQUEUE, &cc->flags)) {
>                 generic_make_request(clone);
>                 return;
>         }
> @@ -1629,7 +1633,8 @@ static void kcryptd_crypt_write_convert(struct
> dm_crypt_io *io)
>         sector += bio_sectors(clone);
>
>         crypt_inc_pending(io);
> -       r = crypt_convert(cc, &io->ctx);
> +       r = crypt_convert(cc, &io->ctx,
> +                         test_bit(DM_CRYPT_NO_WRITE_WORKQUEUE,
> &cc->flags));
>         if (r)
>                 io->error = r;
>         crypt_finished = atomic_dec_and_test(&io->ctx.cc_pending);
> @@ -1659,7 +1664,8 @@ static void kcryptd_crypt_read_convert(struct
> dm_crypt_io *io)
>         crypt_convert_init(cc, &io->ctx, io->base_bio, io->base_bio,
>                            io->sector);
>
> -       r = crypt_convert(cc, &io->ctx);
> +       r = crypt_convert(cc, &io->ctx,
> +                         test_bit(DM_CRYPT_NO_READ_WORKQUEUE,
> &cc->flags));
>         if (r)
>                 io->error = r;
>
> @@ -1719,10 +1725,28 @@ static void kcryptd_crypt(struct work_struct *work)
>                 kcryptd_crypt_write_convert(io);
>  }
>
> +static void kcryptd_crypt_tasklet(unsigned long work)
> +{
> +       kcryptd_crypt((struct work_struct *)work);
> +}
> +
>  static void kcryptd_queue_crypt(struct dm_crypt_io *io)
>  {
>         struct crypt_config *cc = io->cc;
>
> +       if ((bio_data_dir(io->base_bio) == READ &&
> test_bit(DM_CRYPT_NO_READ_WORKQUEUE, &cc->flags)) ||
> +           (bio_data_dir(io->base_bio) == WRITE &&
> test_bit(DM_CRYPT_NO_WRITE_WORKQUEUE, &cc->flags))) {
> +               if (in_irq()) {
> +                       /* Crypto API's "skcipher_walk_first() refuses to
> work in hard IRQ context */
> +                       tasklet_init(&io->tasklet, kcryptd_crypt_tasklet,
> (unsigned long)&io->work);
> +                       tasklet_schedule(&io->tasklet);
> +                       return;
> +               }
> +
> +               kcryptd_crypt(&io->work);
> +               return;
> +       }
> +
>         INIT_WORK(&io->work, kcryptd_crypt);
>         queue_work(cc->crypt_queue, &io->work);
>  }
> @@ -2483,7 +2507,7 @@ static int crypt_ctr_optional(struct dm_target *ti,
> unsigned int argc, char **ar
>         struct crypt_config *cc = ti->private;
>         struct dm_arg_set as;
>         static const struct dm_arg _args[] = {
> -               {0, 6, "Invalid number of feature args"},
> +               {0, 8, "Invalid number of feature args"},
>         };
>         unsigned int opt_params, val;
>         const char *opt_string, *sval;
> @@ -2513,6 +2537,10 @@ static int crypt_ctr_optional(struct dm_target *ti,
> unsigned int argc, char **ar
>
>                 else if (!strcasecmp(opt_string, "submit_from_crypt_cpus"))
>                         set_bit(DM_CRYPT_NO_OFFLOAD, &cc->flags);
> +               else if (!strcasecmp(opt_string, "no_read_workqueue"))
> +                       set_bit(DM_CRYPT_NO_READ_WORKQUEUE, &cc->flags);
> +               else if (!strcasecmp(opt_string, "no_write_workqueue"))
> +                       set_bit(DM_CRYPT_NO_WRITE_WORKQUEUE, &cc->flags);
>                 else if (sscanf(opt_string, "integrity:%u:", &val) == 1) {
>                         if (val == 0 || val > MAX_TAG_SIZE) {
>                                 ti->error = "Invalid integrity arguments";
> @@ -2842,6 +2870,8 @@ static void crypt_status(struct dm_target *ti,
> status_type_t type,
>                 num_feature_args += !!ti->num_discard_bios;
>                 num_feature_args += test_bit(DM_CRYPT_SAME_CPU,
> &cc->flags);
>                 num_feature_args += test_bit(DM_CRYPT_NO_OFFLOAD,
> &cc->flags);
> +               num_feature_args += test_bit(DM_CRYPT_NO_READ_WORKQUEUE,
> &cc->flags);
> +               num_feature_args += test_bit(DM_CRYPT_NO_WRITE_WORKQUEUE,
> &cc->flags);
>                 num_feature_args += cc->sector_size != (1 << SECTOR_SHIFT);
>                 num_feature_args += test_bit(CRYPT_IV_LARGE_SECTORS,
> &cc->cipher_flags);
>                 if (cc->on_disk_tag_size)
> @@ -2854,6 +2884,10 @@ static void crypt_status(struct dm_target *ti,
> status_type_t type,
>                                 DMEMIT(" same_cpu_crypt");
>                         if (test_bit(DM_CRYPT_NO_OFFLOAD, &cc->flags))
>                                 DMEMIT(" submit_from_crypt_cpus");
> +                       if (test_bit(DM_CRYPT_NO_READ_WORKQUEUE,
> &cc->flags))
> +                               DMEMIT(" no_read_workqueue");
> +                       if (test_bit(DM_CRYPT_NO_WRITE_WORKQUEUE,
> &cc->flags))
> +                               DMEMIT(" no_write_workqueue");
>                         if (cc->on_disk_tag_size)
>                                 DMEMIT(" integrity:%u:%s",
> cc->on_disk_tag_size, cc->cipher_auth);
>                         if (cc->sector_size != (1 << SECTOR_SHIFT))
> @@ -2966,7 +3000,7 @@ static void crypt_io_hints(struct dm_target *ti,
> struct queue_limits *limits)
>
>  static struct target_type crypt_target = {
>         .name   = "crypt",
> -       .version = {1, 19, 0},
> +       .version = {1, 22, 0},
>         .module = THIS_MODULE,
>         .ctr    = crypt_ctr,
>         .dtr    = crypt_dtr,
> --
> 2.25.1
>
>
Kleber Sacilotto de Souza May 18, 2021, 10:02 a.m. UTC | #2 
On 18.05.21 05:04, Gerald Yang wrote:
> I sent out the same SRU email twice, sorry about that
> please check the first one and ignore the second one
> 
> Thanks,
> Gerald

Hi Gerald,

Thank you for the note.

NAK'ing this thread.


Thanks,
Kleber

> 
> On Tue, May 18, 2021 at 11:00 AM Gerald Yang <gerald.yang@canonical.com <mailto:gerald.yang@canonical.com>> wrote:
> 
>     From: Ignat Korchagin <ignat@cloudflare.com <mailto:ignat@cloudflare.com>>
> 
>     BugLink: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1910976 <https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1910976>
> 
>     This is a follow up to [1] that detailed latency problems associated
>     with dm-crypt's use of workqueues when processing IO.
> 
>     Current dm-crypt implementation creates a significant IO performance
>     overhead (at least on small IO block sizes) for both latency and
>     throughput. We suspect offloading IO request processing into
>     workqueues and async threads is more harmful these days with the
>     modern fast storage. I also did some digging into the dm-crypt git
>     history and much of this async processing is not needed anymore,
>     because the reasons it was added are mostly gone from the kernel. More
>     details can be found in [2] (see "Git archeology" section).
> 
>     This change adds DM_CRYPT_NO_READ_WORKQUEUE and
>     DM_CRYPT_NO_WRITE_WORKQUEUE flags for read and write BIOs, which
>     direct dm-crypt to not offload crypto operations into kcryptd
>     workqueues.  In addition, writes are not buffered to be sorted in the
>     dm-crypt red-black tree, but dispatched immediately. For cases, where
>     crypto operations cannot happen (hard interrupt context, for example
>     the read path of some NVME drivers), we offload the work to a tasklet
>     rather than a workqueue.
> 
>     These flags only ensure no async BIO processing in the dm-crypt
>     module. It is worth noting that some Crypto API implementations may
>     offload encryption into their own workqueues, which are independent of
>     the dm-crypt and its configuration. However upon enabling these new
>     flags dm-crypt will instruct Crypto API not to backlog crypto
>     requests.
> 
>     To give an idea of the performance gains for certain workloads,
>     consider the script, and results when tested against various
>     devices, detailed here:
>     https://www.redhat.com/archives/dm-devel/2020-July/msg00138.html <https://www.redhat.com/archives/dm-devel/2020-July/msg00138.html>
> 
>     [1]: https://www.spinics.net/lists/dm-crypt/msg07516.html <https://www.spinics.net/lists/dm-crypt/msg07516.html>
>     [2]: https://blog.cloudflare.com/speeding-up-linux-disk-encryption/ <https://blog.cloudflare.com/speeding-up-linux-disk-encryption/>
> 
>     Signed-off-by: Ignat Korchagin <ignat@cloudflare.com <mailto:ignat@cloudflare.com>>
>     Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com <mailto:damien.lemoal@wdc.com>>
>     Reviewed-by: Bob Liu <bob.liu@oracle.com <mailto:bob.liu@oracle.com>>
>     Signed-off-by: Mike Snitzer <snitzer@redhat.com <mailto:snitzer@redhat.com>>
>     (cherry picked from commit 39d42fa96ba1b7d2544db3f8ed5da8fb0d5cb877)
>     Signed-off-by: Gerald Yang <gerald.yang@canonical.com <mailto:gerald.yang@canonical.com>>
>     ---
>       drivers/md/dm-crypt.c | 50 ++++++++++++++++++++++++++++++++++++-------
>       1 file changed, 42 insertions(+), 8 deletions(-)
> 
>     diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
>     index d85648b9c247..91aa99e1d698 100644
>     --- a/drivers/md/dm-crypt.c
>     +++ b/drivers/md/dm-crypt.c
>     @@ -67,6 +67,7 @@ struct dm_crypt_io {
>              u8 *integrity_metadata;
>              bool integrity_metadata_from_pool;
>              struct work_struct work;
>     +       struct tasklet_struct tasklet;
> 
>              struct convert_context ctx;
> 
>     @@ -120,7 +121,8 @@ struct iv_tcw_private {
>        * and encrypts / decrypts at the same time.
>        */
>       enum flags { DM_CRYPT_SUSPENDED, DM_CRYPT_KEY_VALID,
>     -            DM_CRYPT_SAME_CPU, DM_CRYPT_NO_OFFLOAD };
>     +            DM_CRYPT_SAME_CPU, DM_CRYPT_NO_OFFLOAD,
>     +            DM_CRYPT_NO_READ_WORKQUEUE, DM_CRYPT_NO_WRITE_WORKQUEUE };
> 
>       enum cipher_flags {
>              CRYPT_MODE_INTEGRITY_AEAD,      /* Use authenticated mode for cihper */
>     @@ -1211,7 +1213,7 @@ static void crypt_free_req(struct crypt_config *cc, void *req, struct bio *base_
>        * Encrypt / decrypt data from one bio to another one (can be the same one)
>        */
>       static blk_status_t crypt_convert(struct crypt_config *cc,
>     -                        struct convert_context *ctx)
>     +                        struct convert_context *ctx, bool atomic)
>       {
>              unsigned int tag_offset = 0;
>              unsigned int sector_step = cc->sector_size >> SECTOR_SHIFT;
>     @@ -1254,7 +1256,8 @@ static blk_status_t crypt_convert(struct crypt_config *cc,
>                              atomic_dec(&ctx->cc_pending);
>                              ctx->cc_sector += sector_step;
>                              tag_offset++;
>     -                       cond_resched();
>     +                       if (!atomic)
>     +                               cond_resched();
>                              continue;
>                      /*
>                       * There was a data integrity error.
>     @@ -1580,7 +1583,8 @@ static void kcryptd_crypt_write_io_submit(struct dm_crypt_io *io, int async)
> 
>              clone->bi_iter.bi_sector = cc->start + io->sector;
> 
>     -       if (likely(!async) && test_bit(DM_CRYPT_NO_OFFLOAD, &cc->flags)) {
>     +       if ((likely(!async) && test_bit(DM_CRYPT_NO_OFFLOAD, &cc->flags)) ||
>     +           test_bit(DM_CRYPT_NO_WRITE_WORKQUEUE, &cc->flags)) {
>                      generic_make_request(clone);
>                      return;
>              }
>     @@ -1629,7 +1633,8 @@ static void kcryptd_crypt_write_convert(struct dm_crypt_io *io)
>              sector += bio_sectors(clone);
> 
>              crypt_inc_pending(io);
>     -       r = crypt_convert(cc, &io->ctx);
>     +       r = crypt_convert(cc, &io->ctx,
>     +                         test_bit(DM_CRYPT_NO_WRITE_WORKQUEUE, &cc->flags));
>              if (r)
>                      io->error = r;
>              crypt_finished = atomic_dec_and_test(&io->ctx.cc_pending);
>     @@ -1659,7 +1664,8 @@ static void kcryptd_crypt_read_convert(struct dm_crypt_io *io)
>              crypt_convert_init(cc, &io->ctx, io->base_bio, io->base_bio,
>                                 io->sector);
> 
>     -       r = crypt_convert(cc, &io->ctx);
>     +       r = crypt_convert(cc, &io->ctx,
>     +                         test_bit(DM_CRYPT_NO_READ_WORKQUEUE, &cc->flags));
>              if (r)
>                      io->error = r;
> 
>     @@ -1719,10 +1725,28 @@ static void kcryptd_crypt(struct work_struct *work)
>                      kcryptd_crypt_write_convert(io);
>       }
> 
>     +static void kcryptd_crypt_tasklet(unsigned long work)
>     +{
>     +       kcryptd_crypt((struct work_struct *)work);
>     +}
>     +
>       static void kcryptd_queue_crypt(struct dm_crypt_io *io)
>       {
>              struct crypt_config *cc = io->cc;
> 
>     +       if ((bio_data_dir(io->base_bio) == READ && test_bit(DM_CRYPT_NO_READ_WORKQUEUE, &cc->flags)) ||
>     +           (bio_data_dir(io->base_bio) == WRITE && test_bit(DM_CRYPT_NO_WRITE_WORKQUEUE, &cc->flags))) {
>     +               if (in_irq()) {
>     +                       /* Crypto API's "skcipher_walk_first() refuses to work in hard IRQ context */
>     +                       tasklet_init(&io->tasklet, kcryptd_crypt_tasklet, (unsigned long)&io->work);
>     +                       tasklet_schedule(&io->tasklet);
>     +                       return;
>     +               }
>     +
>     +               kcryptd_crypt(&io->work);
>     +               return;
>     +       }
>     +
>              INIT_WORK(&io->work, kcryptd_crypt);
>              queue_work(cc->crypt_queue, &io->work);
>       }
>     @@ -2483,7 +2507,7 @@ static int crypt_ctr_optional(struct dm_target *ti, unsigned int argc, char **ar
>              struct crypt_config *cc = ti->private;
>              struct dm_arg_set as;
>              static const struct dm_arg _args[] = {
>     -               {0, 6, "Invalid number of feature args"},
>     +               {0, 8, "Invalid number of feature args"},
>              };
>              unsigned int opt_params, val;
>              const char *opt_string, *sval;
>     @@ -2513,6 +2537,10 @@ static int crypt_ctr_optional(struct dm_target *ti, unsigned int argc, char **ar
> 
>                      else if (!strcasecmp(opt_string, "submit_from_crypt_cpus"))
>                              set_bit(DM_CRYPT_NO_OFFLOAD, &cc->flags);
>     +               else if (!strcasecmp(opt_string, "no_read_workqueue"))
>     +                       set_bit(DM_CRYPT_NO_READ_WORKQUEUE, &cc->flags);
>     +               else if (!strcasecmp(opt_string, "no_write_workqueue"))
>     +                       set_bit(DM_CRYPT_NO_WRITE_WORKQUEUE, &cc->flags);
>                      else if (sscanf(opt_string, "integrity:%u:", &val) == 1) {
>                              if (val == 0 || val > MAX_TAG_SIZE) {
>                                      ti->error = "Invalid integrity arguments";
>     @@ -2842,6 +2870,8 @@ static void crypt_status(struct dm_target *ti, status_type_t type,
>                      num_feature_args += !!ti->num_discard_bios;
>                      num_feature_args += test_bit(DM_CRYPT_SAME_CPU, &cc->flags);
>                      num_feature_args += test_bit(DM_CRYPT_NO_OFFLOAD, &cc->flags);
>     +               num_feature_args += test_bit(DM_CRYPT_NO_READ_WORKQUEUE, &cc->flags);
>     +               num_feature_args += test_bit(DM_CRYPT_NO_WRITE_WORKQUEUE, &cc->flags);
>                      num_feature_args += cc->sector_size != (1 << SECTOR_SHIFT);
>                      num_feature_args += test_bit(CRYPT_IV_LARGE_SECTORS, &cc->cipher_flags);
>                      if (cc->on_disk_tag_size)
>     @@ -2854,6 +2884,10 @@ static void crypt_status(struct dm_target *ti, status_type_t type,
>                                      DMEMIT(" same_cpu_crypt");
>                              if (test_bit(DM_CRYPT_NO_OFFLOAD, &cc->flags))
>                                      DMEMIT(" submit_from_crypt_cpus");
>     +                       if (test_bit(DM_CRYPT_NO_READ_WORKQUEUE, &cc->flags))
>     +                               DMEMIT(" no_read_workqueue");
>     +                       if (test_bit(DM_CRYPT_NO_WRITE_WORKQUEUE, &cc->flags))
>     +                               DMEMIT(" no_write_workqueue");
>                              if (cc->on_disk_tag_size)
>                                      DMEMIT(" integrity:%u:%s", cc->on_disk_tag_size, cc->cipher_auth);
>                              if (cc->sector_size != (1 << SECTOR_SHIFT))
>     @@ -2966,7 +3000,7 @@ static void crypt_io_hints(struct dm_target *ti, struct queue_limits *limits)
> 
>       static struct target_type crypt_target = {
>              .name   = "crypt",
>     -       .version = {1, 19, 0},
>     +       .version = {1, 22, 0},
>              .module = THIS_MODULE,
>              .ctr    = crypt_ctr,
>              .dtr    = crypt_dtr,
>     -- 
>     2.25.1
> 
>
diff mbox series

Patch

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index d85648b9c247..91aa99e1d698 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -67,6 +67,7 @@  struct dm_crypt_io {
 	u8 *integrity_metadata;
 	bool integrity_metadata_from_pool;
 	struct work_struct work;
+	struct tasklet_struct tasklet;
 
 	struct convert_context ctx;
 
@@ -120,7 +121,8 @@  struct iv_tcw_private {
  * and encrypts / decrypts at the same time.
  */
 enum flags { DM_CRYPT_SUSPENDED, DM_CRYPT_KEY_VALID,
-	     DM_CRYPT_SAME_CPU, DM_CRYPT_NO_OFFLOAD };
+	     DM_CRYPT_SAME_CPU, DM_CRYPT_NO_OFFLOAD,
+	     DM_CRYPT_NO_READ_WORKQUEUE, DM_CRYPT_NO_WRITE_WORKQUEUE };
 
 enum cipher_flags {
 	CRYPT_MODE_INTEGRITY_AEAD,	/* Use authenticated mode for cihper */
@@ -1211,7 +1213,7 @@  static void crypt_free_req(struct crypt_config *cc, void *req, struct bio *base_
  * Encrypt / decrypt data from one bio to another one (can be the same one)
  */
 static blk_status_t crypt_convert(struct crypt_config *cc,
-			 struct convert_context *ctx)
+			 struct convert_context *ctx, bool atomic)
 {
 	unsigned int tag_offset = 0;
 	unsigned int sector_step = cc->sector_size >> SECTOR_SHIFT;
@@ -1254,7 +1256,8 @@  static blk_status_t crypt_convert(struct crypt_config *cc,
 			atomic_dec(&ctx->cc_pending);
 			ctx->cc_sector += sector_step;
 			tag_offset++;
-			cond_resched();
+			if (!atomic)
+				cond_resched();
 			continue;
 		/*
 		 * There was a data integrity error.
@@ -1580,7 +1583,8 @@  static void kcryptd_crypt_write_io_submit(struct dm_crypt_io *io, int async)
 
 	clone->bi_iter.bi_sector = cc->start + io->sector;
 
-	if (likely(!async) && test_bit(DM_CRYPT_NO_OFFLOAD, &cc->flags)) {
+	if ((likely(!async) && test_bit(DM_CRYPT_NO_OFFLOAD, &cc->flags)) ||
+	    test_bit(DM_CRYPT_NO_WRITE_WORKQUEUE, &cc->flags)) {
 		generic_make_request(clone);
 		return;
 	}
@@ -1629,7 +1633,8 @@  static void kcryptd_crypt_write_convert(struct dm_crypt_io *io)
 	sector += bio_sectors(clone);
 
 	crypt_inc_pending(io);
-	r = crypt_convert(cc, &io->ctx);
+	r = crypt_convert(cc, &io->ctx,
+			  test_bit(DM_CRYPT_NO_WRITE_WORKQUEUE, &cc->flags));
 	if (r)
 		io->error = r;
 	crypt_finished = atomic_dec_and_test(&io->ctx.cc_pending);
@@ -1659,7 +1664,8 @@  static void kcryptd_crypt_read_convert(struct dm_crypt_io *io)
 	crypt_convert_init(cc, &io->ctx, io->base_bio, io->base_bio,
 			   io->sector);
 
-	r = crypt_convert(cc, &io->ctx);
+	r = crypt_convert(cc, &io->ctx,
+			  test_bit(DM_CRYPT_NO_READ_WORKQUEUE, &cc->flags));
 	if (r)
 		io->error = r;
 
@@ -1719,10 +1725,28 @@  static void kcryptd_crypt(struct work_struct *work)
 		kcryptd_crypt_write_convert(io);
 }
 
+static void kcryptd_crypt_tasklet(unsigned long work)
+{
+	kcryptd_crypt((struct work_struct *)work);
+}
+
 static void kcryptd_queue_crypt(struct dm_crypt_io *io)
 {
 	struct crypt_config *cc = io->cc;
 
+	if ((bio_data_dir(io->base_bio) == READ && test_bit(DM_CRYPT_NO_READ_WORKQUEUE, &cc->flags)) ||
+	    (bio_data_dir(io->base_bio) == WRITE && test_bit(DM_CRYPT_NO_WRITE_WORKQUEUE, &cc->flags))) {
+		if (in_irq()) {
+			/* Crypto API's "skcipher_walk_first() refuses to work in hard IRQ context */
+			tasklet_init(&io->tasklet, kcryptd_crypt_tasklet, (unsigned long)&io->work);
+			tasklet_schedule(&io->tasklet);
+			return;
+		}
+
+		kcryptd_crypt(&io->work);
+		return;
+	}
+
 	INIT_WORK(&io->work, kcryptd_crypt);
 	queue_work(cc->crypt_queue, &io->work);
 }
@@ -2483,7 +2507,7 @@  static int crypt_ctr_optional(struct dm_target *ti, unsigned int argc, char **ar
 	struct crypt_config *cc = ti->private;
 	struct dm_arg_set as;
 	static const struct dm_arg _args[] = {
-		{0, 6, "Invalid number of feature args"},
+		{0, 8, "Invalid number of feature args"},
 	};
 	unsigned int opt_params, val;
 	const char *opt_string, *sval;
@@ -2513,6 +2537,10 @@  static int crypt_ctr_optional(struct dm_target *ti, unsigned int argc, char **ar
 
 		else if (!strcasecmp(opt_string, "submit_from_crypt_cpus"))
 			set_bit(DM_CRYPT_NO_OFFLOAD, &cc->flags);
+		else if (!strcasecmp(opt_string, "no_read_workqueue"))
+			set_bit(DM_CRYPT_NO_READ_WORKQUEUE, &cc->flags);
+		else if (!strcasecmp(opt_string, "no_write_workqueue"))
+			set_bit(DM_CRYPT_NO_WRITE_WORKQUEUE, &cc->flags);
 		else if (sscanf(opt_string, "integrity:%u:", &val) == 1) {
 			if (val == 0 || val > MAX_TAG_SIZE) {
 				ti->error = "Invalid integrity arguments";
@@ -2842,6 +2870,8 @@  static void crypt_status(struct dm_target *ti, status_type_t type,
 		num_feature_args += !!ti->num_discard_bios;
 		num_feature_args += test_bit(DM_CRYPT_SAME_CPU, &cc->flags);
 		num_feature_args += test_bit(DM_CRYPT_NO_OFFLOAD, &cc->flags);
+		num_feature_args += test_bit(DM_CRYPT_NO_READ_WORKQUEUE, &cc->flags);
+		num_feature_args += test_bit(DM_CRYPT_NO_WRITE_WORKQUEUE, &cc->flags);
 		num_feature_args += cc->sector_size != (1 << SECTOR_SHIFT);
 		num_feature_args += test_bit(CRYPT_IV_LARGE_SECTORS, &cc->cipher_flags);
 		if (cc->on_disk_tag_size)
@@ -2854,6 +2884,10 @@  static void crypt_status(struct dm_target *ti, status_type_t type,
 				DMEMIT(" same_cpu_crypt");
 			if (test_bit(DM_CRYPT_NO_OFFLOAD, &cc->flags))
 				DMEMIT(" submit_from_crypt_cpus");
+			if (test_bit(DM_CRYPT_NO_READ_WORKQUEUE, &cc->flags))
+				DMEMIT(" no_read_workqueue");
+			if (test_bit(DM_CRYPT_NO_WRITE_WORKQUEUE, &cc->flags))
+				DMEMIT(" no_write_workqueue");
 			if (cc->on_disk_tag_size)
 				DMEMIT(" integrity:%u:%s", cc->on_disk_tag_size, cc->cipher_auth);
 			if (cc->sector_size != (1 << SECTOR_SHIFT))
@@ -2966,7 +3000,7 @@  static void crypt_io_hints(struct dm_target *ti, struct queue_limits *limits)
 
 static struct target_type crypt_target = {
 	.name   = "crypt",
-	.version = {1, 19, 0},
+	.version = {1, 22, 0},
 	.module = THIS_MODULE,
 	.ctr    = crypt_ctr,
 	.dtr    = crypt_dtr,