Re: [dm-crypt] dm-crypt design and decision

[Milan Broz <mbroz@redhat.com>]

On 01/29/2012 11:30 PM, FAN ZHANG wrote:
> I have two questions with the design of dm-crypt.
> 1. Why need call kcryptd_queue_io() and
> kcryptd_queue_crypt() to put request bio to io_queue and crypt_queue?
> Actually, for read we could clone a bio in crypt_map() and call
> generic_make_request() directly and in crypt_endio(), we simply decrypt the data
> for write, we simply encrypt data in crypt_map() and call generic_make_request() to write encrypted bio to low level block device.

Long time ago it was this way. You cannot block in crypt_map
(allocation etc), basically it is deadlock prevention.

Imagine stacked dmcrypt mappings, if one device
is blocked in crypt_map and waiting for another one to finish
IO, it will deadlock.

> 2. If we really need to queue the bio request, why we need two queues instead combining io_queueand crypt_queuetogether?

the same, deadlock prevention.

> 3. Why need to call kcryptd_crypt_write_io_submit(io, error, 1) in
> kcryptd_async_done() to do async write via io_queue instead of call
> kcryptd_crypt_write_io_submit(io, error, 0) to do sync write?

You are probably missing asynchronous encryption path
where you cannot submit (generic_make_request())
IO from async callback in interrupt mode.

Most of the users use sync path (this include
even AES_NI and similar accelerators) but
if async crypto driver is used, you have to
use separate workqueue for IO processing.

For testing, you can force crypto layer to use cryptd
and use async path im dmcrypt, just patch dmcrypt using this chunk
(and enable proper crypto switches in .config for cryptd)

@@ -1267,7 +1267,7 @@ static int crypt_ctr_cipher(struct dm_target *ti,
                 goto bad_mem;
  
         ret = snprintf(cipher_api, CRYPTO_MAX_ALG_NAME,
-                      "%s(%s)", chainmode, cipher);
+                      "cryptd(%s(%s-generic))", chainmode, cipher);


Milan