On 2013-04-09 12:07:59, Tyler Hicks wrote:
> Make the switch from the blkcipher kernel crypto interface to the
> ablkcipher interface.
>
> encrypt_scatterlist() and decrypt_scatterlist() now use the ablkcipher
> interface but, from the eCryptfs standpoint, still treat the crypto
> operation as a synchronous operation. They submit the async request and
> then wait until the operation is finished before they return. Most of
> the changes are contained inside those two functions.
>
> Despite waiting for the completion of the crypto operation, the
> ablkcipher interface provides performance increases in most cases when
> used on AES-NI capable hardware. However, sequential I/O with one or two
> threads on slow storage media does exhibit a sizeable decrease in
> performance.
>
> Signed-off-by: Tyler Hicks <tyhicks@xxxxxxxxxxxxx>
> Cc: Colin King <colin.king@xxxxxxxxxxxxx>
> Cc: Dustin Kirkland <dustin.kirkland@xxxxxxxxxxx>
> Cc: Tim Chen <tim.c.chen@xxxxxxxxx>
> Cc: Ying Huang <ying.huang@xxxxxxxxx>
> Cc: Thieu Le <thieule@xxxxxxxxxx>
> Cc: Li Wang <dragonylffly@xxxxxxx>
> Cc: Zeev Zilberman <zeev@xxxxxxxxxxxxxxxxx>
> Cc: Jarkko Sakkinen <jarkko.sakkinen@xxxxxx>
> ---
>
> * I only made the changes mentioned in RFC v2 to encrypt_scatterlist(). This
> patch also makes those same changes to decrypt_scatterlist():
> - Unlock cs_tfm_mutex if ablkcipher_request_alloc() fails
> - Add CRYPTO_TFM_REQ_MAY_SLEEP flag to the ablkcipher_request_set_callback()
> parameters
>
> I won't have a chance to gather new performance numbers until later tonight or
> tomorrow morning.
I've attached a PDF of the tiobench results using vanilla 3.9-rc5,
3.9-rc5 plus the async patches, and 3.9-rc5 plus the ablkcipher patch.
After looking a bunch of these test results, I've noticed some
variations in the results. For example, the attached numbers show the
ablkcipher patch causing a slow down on the SSD when doing
single-threaded sequential writes. But, I've seen another set of results
that had a similar slow down with the async patches but not the
ablkcipher patch.
So, my take away is that the ablkcipher patch and async patches perform
the same. The ablkcipher patch does not have any effect when using AES
or 3DES when hardware acceleration is not available.
I plan on queuing up the ablkcipher patch for the 3.10 merge window
unless anyone speaks up between now and then. It is simple and performs
the same as the async patches.
Tyler
>
> fs/ecryptfs/crypto.c | 143 ++++++++++++++++++++++++++++++------------
> fs/ecryptfs/ecryptfs_kernel.h | 3 +-
> 2 files changed, 105 insertions(+), 41 deletions(-)
>
> diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c
> index d5c25db..eb50bce 100644
> --- a/fs/ecryptfs/crypto.c
> +++ b/fs/ecryptfs/crypto.c
> @@ -243,7 +243,7 @@ void ecryptfs_destroy_crypt_stat(struct ecryptfs_crypt_stat *crypt_stat)
> struct ecryptfs_key_sig *key_sig, *key_sig_tmp;
>
> if (crypt_stat->tfm)
> - crypto_free_blkcipher(crypt_stat->tfm);
> + crypto_free_ablkcipher(crypt_stat->tfm);
> if (crypt_stat->hash_tfm)
> crypto_free_hash(crypt_stat->hash_tfm);
> list_for_each_entry_safe(key_sig, key_sig_tmp,
> @@ -319,6 +319,22 @@ int virt_to_scatterlist(const void *addr, int size, struct scatterlist *sg,
> return i;
> }
>
> +struct extent_crypt_result {
> + struct completion completion;
> + int rc;
> +};
> +
> +static void extent_crypt_complete(struct crypto_async_request *req, int rc)
> +{
> + struct extent_crypt_result *ecr = req->data;
> +
> + if (rc == -EINPROGRESS)
> + return;
> +
> + ecr->rc = rc;
> + complete(&ecr->completion);
> +}
> +
> /**
> * encrypt_scatterlist
> * @crypt_stat: Pointer to the crypt_stat struct to initialize.
> @@ -334,11 +350,8 @@ static int encrypt_scatterlist(struct ecryptfs_crypt_stat *crypt_stat,
> struct scatterlist *src_sg, int size,
> unsigned char *iv)
> {
> - struct blkcipher_desc desc = {
> - .tfm = crypt_stat->tfm,
> - .info = iv,
> - .flags = CRYPTO_TFM_REQ_MAY_SLEEP
> - };
> + struct ablkcipher_request *req = NULL;
> + struct extent_crypt_result ecr;
> int rc = 0;
>
> BUG_ON(!crypt_stat || !crypt_stat->tfm
> @@ -349,24 +362,48 @@ static int encrypt_scatterlist(struct ecryptfs_crypt_stat *crypt_stat,
> ecryptfs_dump_hex(crypt_stat->key,
> crypt_stat->key_size);
> }
> - /* Consider doing this once, when the file is opened */
> +
> + init_completion(&ecr.completion);
> +
> mutex_lock(&crypt_stat->cs_tfm_mutex);
> - if (!(crypt_stat->flags & ECRYPTFS_KEY_SET)) {
> - rc = crypto_blkcipher_setkey(crypt_stat->tfm, crypt_stat->key,
> - crypt_stat->key_size);
> - crypt_stat->flags |= ECRYPTFS_KEY_SET;
> - }
> - if (rc) {
> - ecryptfs_printk(KERN_ERR, "Error setting key; rc = [%d]\n",
> - rc);
> + req = ablkcipher_request_alloc(crypt_stat->tfm, GFP_NOFS);
> + if (!req) {
> mutex_unlock(&crypt_stat->cs_tfm_mutex);
> - rc = -EINVAL;
> + rc = -ENOMEM;
> goto out;
> }
> - ecryptfs_printk(KERN_DEBUG, "Encrypting [%d] bytes.\n", size);
> - crypto_blkcipher_encrypt_iv(&desc, dest_sg, src_sg, size);
> +
> + ablkcipher_request_set_callback(req,
> + CRYPTO_TFM_REQ_MAY_BACKLOG | CRYPTO_TFM_REQ_MAY_SLEEP,
> + extent_crypt_complete, &ecr);
> + /* Consider doing this once, when the file is opened */
> + if (!(crypt_stat->flags & ECRYPTFS_KEY_SET)) {
> + rc = crypto_ablkcipher_setkey(crypt_stat->tfm, crypt_stat->key,
> + crypt_stat->key_size);
> + if (rc) {
> + ecryptfs_printk(KERN_ERR,
> + "Error setting key; rc = [%d]\n",
> + rc);
> + mutex_unlock(&crypt_stat->cs_tfm_mutex);
> + rc = -EINVAL;
> + goto out;
> + }
> + crypt_stat->flags |= ECRYPTFS_KEY_SET;
> + }
> mutex_unlock(&crypt_stat->cs_tfm_mutex);
> + ecryptfs_printk(KERN_DEBUG, "Encrypting [%d] bytes.\n", size);
> + ablkcipher_request_set_crypt(req, src_sg, dest_sg, size, iv);
> + rc = crypto_ablkcipher_encrypt(req);
> + if (rc == -EINPROGRESS || rc == -EBUSY) {
> + struct extent_crypt_result *ecr = req->base.data;
> +
> + rc = wait_for_completion_interruptible(&ecr->completion);
> + if (!rc)
> + rc = ecr->rc;
> + INIT_COMPLETION(ecr->completion);
> + }
> out:
> + ablkcipher_request_free(req);
> return rc;
> }
>
> @@ -624,35 +661,62 @@ static int decrypt_scatterlist(struct ecryptfs_crypt_stat *crypt_stat,
> struct scatterlist *src_sg, int size,
> unsigned char *iv)
> {
> - struct blkcipher_desc desc = {
> - .tfm = crypt_stat->tfm,
> - .info = iv,
> - .flags = CRYPTO_TFM_REQ_MAY_SLEEP
> - };
> + struct ablkcipher_request *req = NULL;
> + struct extent_crypt_result ecr;
> int rc = 0;
>
> - /* Consider doing this once, when the file is opened */
> + BUG_ON(!crypt_stat || !crypt_stat->tfm
> + || !(crypt_stat->flags & ECRYPTFS_STRUCT_INITIALIZED));
> + if (unlikely(ecryptfs_verbosity > 0)) {
> + ecryptfs_printk(KERN_DEBUG, "Key size [%zd]; key:\n",
> + crypt_stat->key_size);
> + ecryptfs_dump_hex(crypt_stat->key,
> + crypt_stat->key_size);
> + }
> +
> + init_completion(&ecr.completion);
> +
> mutex_lock(&crypt_stat->cs_tfm_mutex);
> - rc = crypto_blkcipher_setkey(crypt_stat->tfm, crypt_stat->key,
> - crypt_stat->key_size);
> - if (rc) {
> - ecryptfs_printk(KERN_ERR, "Error setting key; rc = [%d]\n",
> - rc);
> + req = ablkcipher_request_alloc(crypt_stat->tfm, GFP_NOFS);
> + if (!req) {
> mutex_unlock(&crypt_stat->cs_tfm_mutex);
> - rc = -EINVAL;
> + rc = -ENOMEM;
> goto out;
> }
> - ecryptfs_printk(KERN_DEBUG, "Decrypting [%d] bytes.\n", size);
> - rc = crypto_blkcipher_decrypt_iv(&desc, dest_sg, src_sg, size);
> +
> + ablkcipher_request_set_callback(req,
> + CRYPTO_TFM_REQ_MAY_BACKLOG | CRYPTO_TFM_REQ_MAY_SLEEP,
> + extent_crypt_complete, &ecr);
> + /* Consider doing this once, when the file is opened */
> + if (!(crypt_stat->flags & ECRYPTFS_KEY_SET)) {
> + rc = crypto_ablkcipher_setkey(crypt_stat->tfm, crypt_stat->key,
> + crypt_stat->key_size);
> + if (rc) {
> + ecryptfs_printk(KERN_ERR,
> + "Error setting key; rc = [%d]\n",
> + rc);
> + mutex_unlock(&crypt_stat->cs_tfm_mutex);
> + rc = -EINVAL;
> + goto out;
> + }
> + crypt_stat->flags |= ECRYPTFS_KEY_SET;
> + }
> mutex_unlock(&crypt_stat->cs_tfm_mutex);
> - if (rc) {
> - ecryptfs_printk(KERN_ERR, "Error decrypting; rc = [%d]\n",
> - rc);
> - goto out;
> + ecryptfs_printk(KERN_DEBUG, "Decrypting [%d] bytes.\n", size);
> + ablkcipher_request_set_crypt(req, src_sg, dest_sg, size, iv);
> + rc = crypto_ablkcipher_decrypt(req);
> + if (rc == -EINPROGRESS || rc == -EBUSY) {
> + struct extent_crypt_result *ecr = req->base.data;
> +
> + rc = wait_for_completion_interruptible(&ecr->completion);
> + if (!rc)
> + rc = ecr->rc;
> + INIT_COMPLETION(ecr->completion);
> }
> - rc = size;
> out:
> + ablkcipher_request_free(req);
> return rc;
> +
> }
>
> /**
> @@ -746,8 +810,7 @@ int ecryptfs_init_crypt_ctx(struct ecryptfs_crypt_stat *crypt_stat)
> crypt_stat->cipher, "cbc");
> if (rc)
> goto out_unlock;
> - crypt_stat->tfm = crypto_alloc_blkcipher(full_alg_name, 0,
> - CRYPTO_ALG_ASYNC);
> + crypt_stat->tfm = crypto_alloc_ablkcipher(full_alg_name, 0, 0);
> kfree(full_alg_name);
> if (IS_ERR(crypt_stat->tfm)) {
> rc = PTR_ERR(crypt_stat->tfm);
> @@ -757,7 +820,7 @@ int ecryptfs_init_crypt_ctx(struct ecryptfs_crypt_stat *crypt_stat)
> crypt_stat->cipher);
> goto out_unlock;
> }
> - crypto_blkcipher_set_flags(crypt_stat->tfm, CRYPTO_TFM_REQ_WEAK_KEY);
> + crypto_ablkcipher_set_flags(crypt_stat->tfm, CRYPTO_TFM_REQ_WEAK_KEY);
> rc = 0;
> out_unlock:
> mutex_unlock(&crypt_stat->cs_tfm_mutex);
> diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h
> index dd299b3..f622a73 100644
> --- a/fs/ecryptfs/ecryptfs_kernel.h
> +++ b/fs/ecryptfs/ecryptfs_kernel.h
> @@ -38,6 +38,7 @@
> #include <linux/nsproxy.h>
> #include <linux/backing-dev.h>
> #include <linux/ecryptfs.h>
> +#include <linux/crypto.h>
>
> #define ECRYPTFS_DEFAULT_IV_BYTES 16
> #define ECRYPTFS_DEFAULT_EXTENT_SIZE 4096
> @@ -233,7 +234,7 @@ struct ecryptfs_crypt_stat {
> size_t extent_shift;
> unsigned int extent_mask;
> struct ecryptfs_mount_crypt_stat *mount_crypt_stat;
> - struct crypto_blkcipher *tfm;
> + struct crypto_ablkcipher *tfm;
> struct crypto_hash *hash_tfm; /* Crypto context for generating
> * the initialization vectors */
> unsigned char cipher[ECRYPTFS_MAX_CIPHER_NAME_SIZE];
> --
> 1.8.1.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe ecryptfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Attachment:
3.9-rc5-async-ablkcipher-tiobench.pdf
Description: Adobe PDF document
Attachment:
signature.asc
Description: Digital signature
