All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Biggers <ebiggers@kernel.org>
To: Ignat Korchagin <ignat@cloudflare.com>
Cc: agk@redhat.com, Mike Snitzer <snitzer@redhat.com>,
	dm-devel@redhat.com, dm-crypt@saout.de,
	linux-kernel <linux-kernel@vger.kernel.org>,
	kernel-team <kernel-team@cloudflare.com>
Subject: Re: [dm-crypt] [RFC PATCH 1/1] Add DM_CRYPT_FORCE_INLINE flag to dm-crypt target
Date: Wed, 24 Jun 2020 09:24:07 -0700	[thread overview]
Message-ID: <20200624162407.GB200774@gmail.com> (raw)
In-Reply-To: <CALrw=nFduv_X83V1Dfz+bt4bZqT19OSi3q5f7umhty1-DQ2SPg@mail.gmail.com>

On Wed, Jun 24, 2020 at 09:24:07AM +0100, Ignat Korchagin wrote:
> On Wed, Jun 24, 2020 at 6:04 AM Eric Biggers <ebiggers@kernel.org> wrote:
> >
> > On Fri, Jun 19, 2020 at 05:41:32PM +0100, Ignat Korchagin wrote:
> > > Sometimes extra thread offloading imposed by dm-crypt hurts IO latency. This is
> > > especially visible on busy systems with many processes/threads. Moreover, most
> > > Crypto API implementaions are async, that is they offload crypto operations on
> > > their own, so this dm-crypt offloading is excessive.
> >
> > This really should say "some Crypto API implementations are async" instead of
> > "most Crypto API implementations are async".
> 
> The most accurate would probably be: most hardware-accelerated Crypto
> API implementations are async
> 
> > Notably, the AES-NI implementation of AES-XTS is synchronous if you call it in a
> > context where SIMD instructions are usable.  It's only asynchronous when SIMD is
> > not usable.  (This seems to have been missed in your blog post.)
> 
> No, it was not. This is exactly why we made xts-proxy Crypto API
> module as a second patch. But it seems now it does not make a big
> difference if a used Crypto API implementation is synchronous as well
> (based on some benchmarks outlined in the cover letter to this patch).
> I think the v2 of this patch will not require a synchronous Crypto
> API. This is probably a right thing to do, as the "inline" flag should
> control the way how dm-crypt itself handles requests, not how Crypto
> API handles requests. If a user wants to ensure a particular
> synchronous Crypto API implementation, they can already reconfigure
> dm-crypt and specify the implementation with a "capi:" prefix in the
> the dm table description.

I think you're missing the point.  Although xts-aes-aesni has the
CRYPTO_ALG_ASYNC bit set, the actual implementation processes the request
synchronously if SIMD instructions are currently usable.  That's always the case
in dm-crypt, as far as I can tell.  This algorithm has the ASYNC flag only
because it's not synchronous when called in hardIRQ context.

That's why your "xts-proxy" doesn't make a difference, and why it's misleading
to suggest that the crypto API is doing its own queueing when you're primarily
talking about xts-aes-aesni.  The crypto API definitely can do its own queueing,
mainly with hardware drivers.  But it doesn't in this common and relevant case.

- Eric

  reply	other threads:[~2020-06-24 16:24 UTC|newest]

Thread overview: 65+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-19 16:41 [dm-crypt] [RFC PATCH 0/1] dm-crypt excessive overhead Ignat Korchagin
2020-06-19 16:41 ` Ignat Korchagin
2020-06-19 16:41 ` [dm-crypt] [RFC PATCH 1/1] Add DM_CRYPT_FORCE_INLINE flag to dm-crypt target Ignat Korchagin
2020-06-19 16:41   ` Ignat Korchagin
2020-06-24  5:04   ` [dm-crypt] " Eric Biggers
2020-06-24  5:21     ` [dm-crypt] [dm-devel] " Damien Le Moal
2020-06-24  5:21       ` [dm-devel] [dm-crypt] " Damien Le Moal
2020-06-24  5:27       ` [dm-crypt] [dm-devel] " Eric Biggers
2020-06-24  5:27         ` [dm-devel] [dm-crypt] " Eric Biggers
2020-06-24  6:46         ` [dm-crypt] [dm-devel] " Damien Le Moal
2020-06-24  6:46           ` [dm-devel] [dm-crypt] " Damien Le Moal
2020-06-24  7:24         ` [dm-crypt] [dm-devel] " Damien Le Moal
2020-06-24  7:24           ` [dm-devel] [dm-crypt] " Damien Le Moal
2020-06-24  7:49     ` [dm-crypt] [dm-devel] " Damien Le Moal
2020-06-24  7:49       ` [dm-devel] [dm-crypt] " Damien Le Moal
2020-06-24  8:24     ` Ignat Korchagin
2020-06-24 16:24       ` Eric Biggers [this message]
2020-06-24 17:00         ` Ignat Korchagin
2020-06-24  5:12   ` [dm-crypt] [dm-devel] " Damien Le Moal
2020-06-24  5:12     ` Damien Le Moal
2020-06-19 16:55 ` [dm-crypt] [RFC PATCH 0/1] dm-crypt excessive overhead Mike Snitzer
2020-06-19 16:55   ` Mike Snitzer
2020-06-19 18:39   ` [dm-crypt] " Mikulas Patocka
2020-06-19 18:39     ` Mikulas Patocka
2020-06-19 19:44     ` [dm-crypt] " Ignat Korchagin
2020-06-19 19:44       ` Ignat Korchagin
2020-06-20  1:23     ` [dm-crypt] " Herbert Xu
2020-06-20  1:23       ` Herbert Xu
2020-06-20 19:36       ` [dm-crypt] " Mikulas Patocka
2020-06-20 19:36         ` Mikulas Patocka
2020-06-20 19:36         ` Mikulas Patocka
2020-06-20 21:02         ` [dm-crypt] " Ignat Korchagin
2020-06-20 21:02           ` Ignat Korchagin
2020-06-20 21:02           ` Ignat Korchagin
2020-06-23 15:33       ` [dm-crypt] " Mike Snitzer
2020-06-23 15:33         ` Mike Snitzer
2020-06-23 15:33         ` Mike Snitzer
2020-06-23 16:24         ` [dm-crypt] " Ignat Korchagin
2020-06-23 16:24           ` Ignat Korchagin
2020-06-24  0:23           ` [dm-crypt] " Herbert Xu
2020-06-24  0:23             ` Herbert Xu
2020-06-22  0:45   ` [dm-crypt] [dm-devel] " Damien Le Moal
2020-06-22  0:45     ` Damien Le Moal
2020-06-22  0:45     ` Damien Le Moal
2020-06-22  7:55     ` [dm-crypt] [dm-devel] " Ignat Korchagin
2020-06-22  7:55       ` Ignat Korchagin
2020-06-22  8:08       ` [dm-crypt] " Damien Le Moal
2020-06-22  8:08         ` Damien Le Moal
2020-06-22  8:08         ` Damien Le Moal
2020-06-23 15:01     ` [dm-crypt] " Mike Snitzer
2020-06-23 15:01       ` Mike Snitzer
2020-06-23 15:07       ` [dm-crypt] " Ignat Korchagin
2020-06-23 15:07         ` Ignat Korchagin
2020-06-23 15:22         ` [dm-crypt] " Mike Snitzer
2020-06-23 15:22           ` Mike Snitzer
2020-06-24  4:54           ` [dm-crypt] [dm-devel] " Damien Le Moal
2020-06-24  4:54             ` Damien Le Moal
2020-06-24  5:22             ` [dm-crypt] " Mike Snitzer
2020-06-24  5:22               ` Mike Snitzer
2020-06-24  5:22               ` Mike Snitzer
2020-06-24  8:02               ` [dm-crypt] " Ignat Korchagin
2020-06-24  8:02                 ` Ignat Korchagin
2020-06-24  4:28       ` [dm-crypt] " Damien Le Moal
2020-06-24  4:28         ` Damien Le Moal
2020-06-24  4:28         ` Damien Le Moal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200624162407.GB200774@gmail.com \
    --to=ebiggers@kernel.org \
    --cc=agk@redhat.com \
    --cc=dm-crypt@saout.de \
    --cc=dm-devel@redhat.com \
    --cc=ignat@cloudflare.com \
    --cc=kernel-team@cloudflare.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=snitzer@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.