crypto: allow to assign gfp_t for __crypto_alloc_tfm

PATCH In review Jaegeuk Kim 5 participants 20 messages May 19, 2015 – May 20, 2015

Subsystems (10)

x86 Arm i.MX and Layerscape Crypto API Arm STM32 +6 more subsystems

lore

Share & tools

lore

Jaegeuk Kim

author maintainer

2015-05-19 05:46 UTC #0 lore raw

PATCH 1/1 crypto: allow to assign gfp_t for __crypto_alloc_tfm

This patch adds a parameter, gfp_t, for __crypto_alloc_tfm.
Now EXT4 and F2FS use the crypto engine to encrypt a page when writing it to the disk.
That happens during ->writepage and it needs to allocate memory with GFP_NOFS.

Otherwise, in the f2fs case, kernel reports such the following warning.

RECLAIM_FS-ON-R at:

  [<ffffffff810e44da>] mark_held_locks+0x6a/0x90
  [<ffffffff810e516f>] lockdep_trace_alloc+0xcf/0x120
  [<ffffffff8121c573>] __kmalloc+0x53/0x3d0
  [<ffffffff81356df5>] __crypto_alloc_tfm+0x45/0x170
  [<ffffffff8135aff0>] crypto_alloc_ablkcipher+0x60/0xb0
  [<ffffffffa03e5548>] f2fs_get_crypto_ctx+0x118/0x220 [f2fs]
  [<ffffffffa03e589a>] f2fs_encrypt+0x2a/0x160 [f2fs]
  [<ffffffffa03d3eac>] do_write_data_page+0x21c/0x6f0 [f2fs]
  [<ffffffffa03d480b>] f2fs_write_data_page+0x48b/0x5c0 [f2fs]
  [<ffffffffa03cd79a>] __f2fs_writepage+0x1a/0x50 [f2fs]
  [<ffffffff811c7e44>] write_cache_pages+0x274/0x6f0
  [<ffffffffa03cf1ba>] f2fs_write_data_pages+0xea/0x3b0 [f2fs]
  [<ffffffff811c9b61>] do_writepages+0x21/0x50
  [<ffffffff812710e6>] __writeback_single_inode+0x76/0xbf0
  [<ffffffff81271f8a>] writeback_sb_inodes+0x32a/0x720
  [<ffffffff81272571>] wb_writeback+0x121/0x850
  [<ffffffff81273398>] bdi_writeback_workfn+0x148/0x980
  [<ffffffff810a74a2>] process_one_work+0x1e2/0x840
  [<ffffffff810a7c21>] worker_thread+0x121/0x470
  [<ffffffff810ae268>] kthread+0xf8/0x110
  [<ffffffff8180b9a2>] ret_from_fork+0x42/0x70

Signed-off-by Jaegeuk Kim <[email protected]>

---
 arch/x86/crypto/aesni-intel_glue.c      | 2 +-
 crypto/ablkcipher.c                     | 4 ++--
 crypto/aead.c                           | 2 +-
 crypto/algapi.c                         | 2 +-
 crypto/algif_skcipher.c                 | 2 +-
 crypto/api.c                            | 6 +++---
 crypto/internal.h                       | 2 +-
 crypto/tcrypt.c                         | 2 +-
 crypto/testmgr.c                        | 3 ++-
 drivers/crypto/ccp/ccp-crypto-aes-xts.c | 3 ++-
 drivers/crypto/mxs-dcp.c                | 2 +-
 drivers/crypto/picoxcell_crypto.c       | 3 ++-
 drivers/crypto/qce/ablkcipher.c         | 3 ++-
 drivers/crypto/sahara.c                 | 3 ++-
 drivers/md/dm-crypt.c                   | 3 ++-
 fs/ecryptfs/crypto.c                    | 3 ++-
 fs/ext4/crypto.c                        | 3 ++-
 fs/ext4/crypto_fname.c                  | 2 +-
 fs/f2fs/crypto.c                        | 3 ++-
 fs/f2fs/crypto_fname.c                  | 2 +-
 fs/f2fs/crypto_key.c                    | 2 +-
 include/linux/crypto.h                  | 2 +-
 22 files changed, 34 insertions(+), 25 deletions(-)

arch/x86/crypto/aesni-intel_glue.c +1 -1

index 112cefa..5a7fe76 100644
@@ -841,7 +841,7 @@ rfc4106_set_hash_subkey(u8 *hash_subkey, const u8 *key, unsigned int key_len)
 	int ret = -EINVAL;
 	struct aesni_hash_subkey_req_data *req_data;
 
-	ctr_tfm = crypto_alloc_ablkcipher("ctr(aes)", 0, 0);
+	ctr_tfm = crypto_alloc_ablkcipher("ctr(aes)", 0, 0, GFP_KERNEL);
 	if (IS_ERR(ctr_tfm))
 		return PTR_ERR(ctr_tfm);

crypto/ablkcipher.c +2 -2

index db201bca..3706e4a 100644
@@ -671,7 +671,7 @@ int crypto_grab_skcipher(struct crypto_skcipher_spawn *spawn, const char *name,
 EXPORT_SYMBOL_GPL(crypto_grab_skcipher);
 
 struct crypto_ablkcipher *crypto_alloc_ablkcipher(const char *alg_name,
-						  u32 type, u32 mask)
+						  u32 type, u32 mask, gfp_t gfp)
 {
 	struct crypto_tfm *tfm;
 	int err;

@@ -688,7 +688,7 @@ struct crypto_ablkcipher *crypto_alloc_ablkcipher(const char *alg_name,
 			goto err;
 		}
 
-		tfm = __crypto_alloc_tfm(alg, type, mask);
+		tfm = __crypto_alloc_tfm(alg, type, mask, gfp);
 		if (!IS_ERR(tfm))
 			return __crypto_ablkcipher_cast(tfm);

crypto/aead.c +1 -1

index 2222710..b220a0dd 100644
@@ -542,7 +542,7 @@ struct crypto_aead *crypto_alloc_aead(const char *alg_name, u32 type, u32 mask)
 			goto err;
 		}
 
-		tfm = __crypto_alloc_tfm(alg, type, mask);
+		tfm = __crypto_alloc_tfm(alg, type, mask, GFP_KERNEL);
 		if (!IS_ERR(tfm))
 			return __crypto_aead_cast(tfm);

crypto/algapi.c +1 -1

index d2627a3..1a00274 100644
@@ -660,7 +660,7 @@ struct crypto_tfm *crypto_spawn_tfm(struct crypto_spawn *spawn, u32 type,
 	if (unlikely((alg->cra_flags ^ type) & mask))
 		goto out_put_alg;
 
-	tfm = __crypto_alloc_tfm(alg, type, mask);
+	tfm = __crypto_alloc_tfm(alg, type, mask, GFP_KERNEL);
 	if (IS_ERR(tfm))
 		goto out_put_alg;

crypto/algif_skcipher.c +1 -1

index 9450752..89730a7 100644
@@ -751,7 +751,7 @@ static struct proto_ops algif_skcipher_ops = {
 
 static void *skcipher_bind(const char *name, u32 type, u32 mask)
 {
-	return crypto_alloc_ablkcipher(name, type, mask);
+	return crypto_alloc_ablkcipher(name, type, mask, GFP_KERNEL);
 }
 
 static void skcipher_release(void *private)

crypto/api.c +3 -3

index afe4610..887346b 100644
@@ -364,14 +364,14 @@ void crypto_shoot_alg(struct crypto_alg *alg)
 EXPORT_SYMBOL_GPL(crypto_shoot_alg);
 
 struct crypto_tfm *__crypto_alloc_tfm(struct crypto_alg *alg, u32 type,
-				      u32 mask)
+				      u32 mask, gfp_t gfp)
 {
 	struct crypto_tfm *tfm = NULL;
 	unsigned int tfm_size;
 	int err = -ENOMEM;
 
 	tfm_size = sizeof(*tfm) + crypto_ctxsize(alg, type, mask);
-	tfm = kzalloc(tfm_size, GFP_KERNEL);
+	tfm = kzalloc(tfm_size, gfp);
 	if (tfm == NULL)
 		goto out_err;

@@ -435,7 +435,7 @@ struct crypto_tfm *crypto_alloc_base(const char *alg_name, u32 type, u32 mask)
 			goto err;
 		}
 
-		tfm = __crypto_alloc_tfm(alg, type, mask);
+		tfm = __crypto_alloc_tfm(alg, type, mask, GFP_KERNEL);
 		if (!IS_ERR(tfm))
 			return tfm;

crypto/internal.h +1 -1

index bd39bfc..bd88be7 100644
@@ -90,7 +90,7 @@ void crypto_remove_spawns(struct crypto_alg *alg, struct list_head *list,
 void crypto_remove_final(struct list_head *list);
 void crypto_shoot_alg(struct crypto_alg *alg);
 struct crypto_tfm *__crypto_alloc_tfm(struct crypto_alg *alg, u32 type,
-				      u32 mask);
+				      u32 mask, gfp_t);
 void *crypto_create_tfm(struct crypto_alg *alg,
 			const struct crypto_type *frontend);
 struct crypto_alg *crypto_find_alg(const char *alg_name,

crypto/tcrypt.c +1 -1

index 1a28001..e6986e6 100644
@@ -1097,7 +1097,7 @@ static void test_acipher_speed(const char *algo, int enc, unsigned int secs,
 
 	init_completion(&tresult.completion);
 
-	tfm = crypto_alloc_ablkcipher(algo, 0, 0);
+	tfm = crypto_alloc_ablkcipher(algo, 0, 0, GFP_KERNEL);
 
 	if (IS_ERR(tfm)) {
 		pr_err("failed to load transform for %s: %ld\n", algo,

crypto/testmgr.c +2 -1

index f9bce3d..076369f 100644
@@ -1563,7 +1563,8 @@ static int alg_test_skcipher(const struct alg_test_desc *desc,
 	struct crypto_ablkcipher *tfm;
 	int err = 0;
 
-	tfm = crypto_alloc_ablkcipher(driver, type | CRYPTO_ALG_INTERNAL, mask);
+	tfm = crypto_alloc_ablkcipher(driver, type | CRYPTO_ALG_INTERNAL, mask,
+								GFP_KERNEL);
 	if (IS_ERR(tfm)) {
 		printk(KERN_ERR "alg: skcipher: Failed to load transform for "
 		       "%s: %ld\n", driver, PTR_ERR(tfm));

drivers/crypto/ccp/ccp-crypto-aes-xts.c +2 -1

index 52c7395..54753ee 100644
@@ -192,7 +192,8 @@ static int ccp_aes_xts_cra_init(struct crypto_tfm *tfm)
 
 	fallback_tfm = crypto_alloc_ablkcipher(crypto_tfm_alg_name(tfm), 0,
 					       CRYPTO_ALG_ASYNC |
-					       CRYPTO_ALG_NEED_FALLBACK);
+					       CRYPTO_ALG_NEED_FALLBACK,
+					       GFP_KERNEL);
 	if (IS_ERR(fallback_tfm)) {
 		pr_warn("could not load fallback driver %s\n",
 			crypto_tfm_alg_name(tfm));

drivers/crypto/mxs-dcp.c +1 -1

index 59ed54e..4cac3a2 100644
@@ -486,7 +486,7 @@ static int mxs_dcp_aes_fallback_init(struct crypto_tfm *tfm)
 	struct dcp_async_ctx *actx = crypto_tfm_ctx(tfm);
 	struct crypto_ablkcipher *blk;
 
-	blk = crypto_alloc_ablkcipher(name, 0, flags);
+	blk = crypto_alloc_ablkcipher(name, 0, flags, GFP_KERNEL);
 	if (IS_ERR(blk))
 		return PTR_ERR(blk);

drivers/crypto/picoxcell_crypto.c +2 -1

index 5da5b98..148458e 100644
@@ -1085,7 +1085,8 @@ static int spacc_ablk_cra_init(struct crypto_tfm *tfm)
 	ctx->generic.engine = engine;
 	if (alg->cra_flags & CRYPTO_ALG_NEED_FALLBACK) {
 		ctx->sw_cipher = crypto_alloc_ablkcipher(alg->cra_name, 0,
-				CRYPTO_ALG_ASYNC | CRYPTO_ALG_NEED_FALLBACK);
+				CRYPTO_ALG_ASYNC | CRYPTO_ALG_NEED_FALLBACK,
+				GFP_KERNEL);
 		if (IS_ERR(ctx->sw_cipher)) {
 			dev_warn(engine->dev, "failed to allocate fallback for %s\n",
 				 alg->cra_name);

drivers/crypto/qce/ablkcipher.c +2 -1

index ad592de..e1742d8 100644
@@ -244,7 +244,8 @@ static int qce_ablkcipher_init(struct crypto_tfm *tfm)
 	ctx->fallback = crypto_alloc_ablkcipher(crypto_tfm_alg_name(tfm),
 						CRYPTO_ALG_TYPE_ABLKCIPHER,
 						CRYPTO_ALG_ASYNC |
-						CRYPTO_ALG_NEED_FALLBACK);
+						CRYPTO_ALG_NEED_FALLBACK,
+						GFP_KERNEL);
 	if (IS_ERR(ctx->fallback))
 		return PTR_ERR(ctx->fallback);

drivers/crypto/sahara.c +2 -1

index 6be377f..50a19c1 100644
@@ -765,7 +765,8 @@ static int sahara_aes_cra_init(struct crypto_tfm *tfm)
 	struct sahara_ctx *ctx = crypto_tfm_ctx(tfm);
 
 	ctx->fallback = crypto_alloc_ablkcipher(name, 0,
-				CRYPTO_ALG_ASYNC | CRYPTO_ALG_NEED_FALLBACK);
+				CRYPTO_ALG_ASYNC | CRYPTO_ALG_NEED_FALLBACK,
+				GFP_KERNEL);
 	if (IS_ERR(ctx->fallback)) {
 		pr_err("Error allocating fallback algo %s\n", name);
 		return PTR_ERR(ctx->fallback);

drivers/md/dm-crypt.c +2 -1

index 9eeea19..bde80c3 100644
@@ -1438,7 +1438,8 @@ static int crypt_alloc_tfms(struct crypt_config *cc, char *ciphermode)
 		return -ENOMEM;
 
 	for (i = 0; i < cc->tfms_count; i++) {
-		cc->tfms[i] = crypto_alloc_ablkcipher(ciphermode, 0, 0);
+		cc->tfms[i] = crypto_alloc_ablkcipher(ciphermode, 0, 0,
+							GFP_KERNEL);
 		if (IS_ERR(cc->tfms[i])) {
 			err = PTR_ERR(cc->tfms[i]);
 			crypt_free_tfms(cc);

fs/ecryptfs/crypto.c +2 -1

index 97315f2..7d0d9b2 100644
@@ -623,7 +623,8 @@ 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_ablkcipher(full_alg_name, 0, 0);
+	crypt_stat->tfm = crypto_alloc_ablkcipher(full_alg_name, 0, 0,
+							GFP_KERNEL);
 	if (IS_ERR(crypt_stat->tfm)) {
 		rc = PTR_ERR(crypt_stat->tfm);
 		crypt_stat->tfm = NULL;

fs/ext4/crypto.c +2 -1

index 8ff1527..28cbe92 100644
@@ -162,7 +162,8 @@ struct ext4_crypto_ctx *ext4_get_crypto_ctx(struct inode *inode)
 		switch (key->mode) {
 		case EXT4_ENCRYPTION_MODE_AES_256_XTS:
 			ctx->tfm = crypto_ablkcipher_tfm(
-				crypto_alloc_ablkcipher("xts(aes)", 0, 0));
+				crypto_alloc_ablkcipher("xts(aes)", 0, 0,
+							GFP_NOFS));
 			break;
 		case EXT4_ENCRYPTION_MODE_AES_256_GCM:
 			/* TODO(mhalcrow): AEAD w/ gcm(aes);

fs/ext4/crypto_fname.c +1 -1

index fded02f..cdd07c7 100644
@@ -372,7 +372,7 @@ struct ext4_fname_crypto_ctx *ext4_get_fname_crypto_ctx(
 		 * re-used */
 		if (ctx->ctfm == NULL) {
 			ctx->ctfm = crypto_alloc_ablkcipher("cts(cbc(aes))",
-					0, 0);
+					0, 0, GFP_KERNEL);
 		}
 		if (IS_ERR(ctx->ctfm)) {
 			res = PTR_ERR(ctx->ctfm);

fs/f2fs/crypto.c +2 -1

index c6d1122..173727e 100644
@@ -161,7 +161,8 @@ struct f2fs_crypto_ctx *f2fs_get_crypto_ctx(struct inode *inode)
 		switch (ci->ci_data_mode) {
 		case F2FS_ENCRYPTION_MODE_AES_256_XTS:
 			ctx->tfm = crypto_ablkcipher_tfm(
-				crypto_alloc_ablkcipher("xts(aes)", 0, 0));
+				crypto_alloc_ablkcipher("xts(aes)", 0, 0,
+							GFP_NOFS));
 			break;
 		case F2FS_ENCRYPTION_MODE_AES_256_GCM:
 			/*

fs/f2fs/crypto_fname.c +1 -1

index 016c4b6..47e8e05 100644
@@ -275,7 +275,7 @@ int f2fs_setup_fname_crypto(struct inode *inode)
 		return -ENOKEY;
 	}
 
-	ctfm = crypto_alloc_ablkcipher("cts(cbc(aes))", 0, 0);
+	ctfm = crypto_alloc_ablkcipher("cts(cbc(aes))", 0, 0, GFP_KERNEL);
 	if (!ctfm || IS_ERR(ctfm)) {
 		res = ctfm ? PTR_ERR(ctfm) : -ENOMEM;
 		printk(KERN_DEBUG "%s: error (%d) allocating crypto tfm\n",

fs/f2fs/crypto_key.c +1 -1

index a25b164..6be5c9f 100644
@@ -48,7 +48,7 @@ static int f2fs_derive_key_aes(char deriving_key[F2FS_AES_128_ECB_KEY_SIZE],
 	DECLARE_F2FS_COMPLETION_RESULT(ecr);
 	struct scatterlist src_sg, dst_sg;
 	struct crypto_ablkcipher *tfm = crypto_alloc_ablkcipher("ecb(aes)", 0,
-								0);
+								0, GFP_KERNEL);
 
 	if (IS_ERR(tfm)) {
 		res = PTR_ERR(tfm);

include/linux/crypto.h +1 -1

index 10df5d2..b26ac44 100644
@@ -885,7 +885,7 @@ static inline u32 crypto_skcipher_mask(u32 mask)
  *	   of an error, PTR_ERR() returns the error code.
  */
 struct crypto_ablkcipher *crypto_alloc_ablkcipher(const char *alg_name,
-						  u32 type, u32 mask);
+					  u32 type, u32 mask, gfp_t gfp);
 
 static inline struct crypto_tfm *crypto_ablkcipher_tfm(
 	struct crypto_ablkcipher *tfm)

Signature · 1 line

2.1.1

Herbert Xu

maintainer

2015-05-19 05:49 UTC #1 lore raw

Quoted 7 lines · Jaegeuk Kim

This patch adds a parameter, gfp_t, for __crypto_alloc_tfm.
Now EXT4 and F2FS use the crypto engine to encrypt a page when writing it to the disk.
That happens during ->writepage and it needs to allocate memory with GFP_NOFS.
Otherwise, in the f2fs case, kernel reports such the following warning.

Normally crypto structures should only be allocated on control paths where sleeping or swapping is not an issue. Why is ext4 doing crypto allocations on the data path?

Cheers,

Signature · 3 lines

Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Jaegeuk Kim

author maintainer

2015-05-19 06:24 UTC #2 lore raw

Quoted 7 lines · Herbert Xu

Quoted 6 lines · Jaegeuk Kim
This patch adds a parameter, gfp_t, for __crypto_alloc_tfm.
Now EXT4 and F2FS use the crypto engine to encrypt a page when writing it to the disk.
That happens during ->writepage and it needs to allocate memory with GFP_NOFS.

Quoted 6 lines

Quoted 1 line
Otherwise, in the f2fs case, kernel reports such the following warning.
Normally crypto structures should only be allocated on control paths where sleeping or swapping is not an issue. Why is ext4 doing crypto allocations on the data path?

Recently, Ted introduced per-file encryption feature as follows.

https://lwn.net/Articles/639427/

The call path in fs/ext4/crypto.c is:
- writepage
- ext4_encrypt
- ext4_get_crypto_ctx
- crypto_alloc_ablkcipher

AFAIK, this way can achieve to reduce memory footprint gracefully. Just before submitting bios, fs allocates the required memory, and then end_io will free them in pair.

Thanks,

Quoted 10 lines

Cheers,

Signature · 8 lines

Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Jaegeuk Kim

author maintainer

2015-05-19 06:31 UTC #3 lore raw

Change log from v1:
- fix missing change

-- >8 --

Otherwise, in the f2fs case, kernel reports such the following warning.

RECLAIM_FS-ON-R at:

  [<ffffffff810e44da>] mark_held_locks+0x6a/0x90
  [<ffffffff810e516f>] lockdep_trace_alloc+0xcf/0x120
  [<ffffffff8121c573>] __kmalloc+0x53/0x3d0
  [<ffffffff81356df5>] __crypto_alloc_tfm+0x45/0x170
  [<ffffffff8135aff0>] crypto_alloc_ablkcipher+0x60/0xb0
  [<ffffffffa03e5548>] f2fs_get_crypto_ctx+0x118/0x220 [f2fs]
  [<ffffffffa03e589a>] f2fs_encrypt+0x2a/0x160 [f2fs]
  [<ffffffffa03d3eac>] do_write_data_page+0x21c/0x6f0 [f2fs]
  [<ffffffffa03d480b>] f2fs_write_data_page+0x48b/0x5c0 [f2fs]
  [<ffffffffa03cd79a>] __f2fs_writepage+0x1a/0x50 [f2fs]
  [<ffffffff811c7e44>] write_cache_pages+0x274/0x6f0
  [<ffffffffa03cf1ba>] f2fs_write_data_pages+0xea/0x3b0 [f2fs]
  [<ffffffff811c9b61>] do_writepages+0x21/0x50
  [<ffffffff812710e6>] __writeback_single_inode+0x76/0xbf0
  [<ffffffff81271f8a>] writeback_sb_inodes+0x32a/0x720
  [<ffffffff81272571>] wb_writeback+0x121/0x850
  [<ffffffff81273398>] bdi_writeback_workfn+0x148/0x980
  [<ffffffff810a74a2>] process_one_work+0x1e2/0x840
  [<ffffffff810a7c21>] worker_thread+0x121/0x470
  [<ffffffff810ae268>] kthread+0xf8/0x110
  [<ffffffff8180b9a2>] ret_from_fork+0x42/0x70

Signed-off-by Jaegeuk Kim <[email protected]>

---
 arch/x86/crypto/aesni-intel_glue.c      | 2 +-
 crypto/ablkcipher.c                     | 4 ++--
 crypto/aead.c                           | 2 +-
 crypto/algapi.c                         | 2 +-
 crypto/algif_skcipher.c                 | 2 +-
 crypto/api.c                            | 6 +++---
 crypto/internal.h                       | 2 +-
 crypto/tcrypt.c                         | 2 +-
 crypto/testmgr.c                        | 3 ++-
 drivers/crypto/ccp/ccp-crypto-aes-xts.c | 3 ++-
 drivers/crypto/mxs-dcp.c                | 2 +-
 drivers/crypto/picoxcell_crypto.c       | 3 ++-
 drivers/crypto/qce/ablkcipher.c         | 3 ++-
 drivers/crypto/sahara.c                 | 3 ++-
 drivers/md/dm-crypt.c                   | 3 ++-
 fs/ecryptfs/crypto.c                    | 3 ++-
 fs/ext4/crypto.c                        | 3 ++-
 fs/ext4/crypto_fname.c                  | 2 +-
 fs/ext4/crypto_key.c                    | 2 +-
 fs/f2fs/crypto.c                        | 3 ++-
 fs/f2fs/crypto_fname.c                  | 2 +-
 fs/f2fs/crypto_key.c                    | 2 +-
 include/linux/crypto.h                  | 2 +-
 23 files changed, 35 insertions(+), 26 deletions(-)

arch/x86/crypto/aesni-intel_glue.c +1 -1

index 112cefa..5a7fe76 100644
@@ -841,7 +841,7 @@ rfc4106_set_hash_subkey(u8 *hash_subkey, const u8 *key, unsigned int key_len)
 	int ret = -EINVAL;
 	struct aesni_hash_subkey_req_data *req_data;
 
-	ctr_tfm = crypto_alloc_ablkcipher("ctr(aes)", 0, 0);
+	ctr_tfm = crypto_alloc_ablkcipher("ctr(aes)", 0, 0, GFP_KERNEL);
 	if (IS_ERR(ctr_tfm))
 		return PTR_ERR(ctr_tfm);

crypto/ablkcipher.c +2 -2

index db201bca..3706e4a 100644
@@ -671,7 +671,7 @@ int crypto_grab_skcipher(struct crypto_skcipher_spawn *spawn, const char *name,
 EXPORT_SYMBOL_GPL(crypto_grab_skcipher);
 
 struct crypto_ablkcipher *crypto_alloc_ablkcipher(const char *alg_name,
-						  u32 type, u32 mask)
+						  u32 type, u32 mask, gfp_t gfp)
 {
 	struct crypto_tfm *tfm;
 	int err;

@@ -688,7 +688,7 @@ struct crypto_ablkcipher *crypto_alloc_ablkcipher(const char *alg_name,
 			goto err;
 		}
 
-		tfm = __crypto_alloc_tfm(alg, type, mask);
+		tfm = __crypto_alloc_tfm(alg, type, mask, gfp);
 		if (!IS_ERR(tfm))
 			return __crypto_ablkcipher_cast(tfm);

crypto/aead.c +1 -1

index 2222710..b220a0dd 100644
@@ -542,7 +542,7 @@ struct crypto_aead *crypto_alloc_aead(const char *alg_name, u32 type, u32 mask)
 			goto err;
 		}
 
-		tfm = __crypto_alloc_tfm(alg, type, mask);
+		tfm = __crypto_alloc_tfm(alg, type, mask, GFP_KERNEL);
 		if (!IS_ERR(tfm))
 			return __crypto_aead_cast(tfm);

crypto/algapi.c +1 -1

index d2627a3..1a00274 100644
@@ -660,7 +660,7 @@ struct crypto_tfm *crypto_spawn_tfm(struct crypto_spawn *spawn, u32 type,
 	if (unlikely((alg->cra_flags ^ type) & mask))
 		goto out_put_alg;
 
-	tfm = __crypto_alloc_tfm(alg, type, mask);
+	tfm = __crypto_alloc_tfm(alg, type, mask, GFP_KERNEL);
 	if (IS_ERR(tfm))
 		goto out_put_alg;

crypto/algif_skcipher.c +1 -1

index 9450752..89730a7 100644
@@ -751,7 +751,7 @@ static struct proto_ops algif_skcipher_ops = {
 
 static void *skcipher_bind(const char *name, u32 type, u32 mask)
 {
-	return crypto_alloc_ablkcipher(name, type, mask);
+	return crypto_alloc_ablkcipher(name, type, mask, GFP_KERNEL);
 }
 
 static void skcipher_release(void *private)

crypto/api.c +3 -3

index afe4610..887346b 100644
@@ -364,14 +364,14 @@ void crypto_shoot_alg(struct crypto_alg *alg)
 EXPORT_SYMBOL_GPL(crypto_shoot_alg);
 
 struct crypto_tfm *__crypto_alloc_tfm(struct crypto_alg *alg, u32 type,
-				      u32 mask)
+				      u32 mask, gfp_t gfp)
 {
 	struct crypto_tfm *tfm = NULL;
 	unsigned int tfm_size;
 	int err = -ENOMEM;
 
 	tfm_size = sizeof(*tfm) + crypto_ctxsize(alg, type, mask);
-	tfm = kzalloc(tfm_size, GFP_KERNEL);
+	tfm = kzalloc(tfm_size, gfp);
 	if (tfm == NULL)
 		goto out_err;

@@ -435,7 +435,7 @@ struct crypto_tfm *crypto_alloc_base(const char *alg_name, u32 type, u32 mask)
 			goto err;
 		}
 
-		tfm = __crypto_alloc_tfm(alg, type, mask);
+		tfm = __crypto_alloc_tfm(alg, type, mask, GFP_KERNEL);
 		if (!IS_ERR(tfm))
 			return tfm;

crypto/internal.h +1 -1

index bd39bfc..bd88be7 100644
@@ -90,7 +90,7 @@ void crypto_remove_spawns(struct crypto_alg *alg, struct list_head *list,
 void crypto_remove_final(struct list_head *list);
 void crypto_shoot_alg(struct crypto_alg *alg);
 struct crypto_tfm *__crypto_alloc_tfm(struct crypto_alg *alg, u32 type,
-				      u32 mask);
+				      u32 mask, gfp_t);
 void *crypto_create_tfm(struct crypto_alg *alg,
 			const struct crypto_type *frontend);
 struct crypto_alg *crypto_find_alg(const char *alg_name,

crypto/tcrypt.c +1 -1

index 1a28001..e6986e6 100644
@@ -1097,7 +1097,7 @@ static void test_acipher_speed(const char *algo, int enc, unsigned int secs,
 
 	init_completion(&tresult.completion);
 
-	tfm = crypto_alloc_ablkcipher(algo, 0, 0);
+	tfm = crypto_alloc_ablkcipher(algo, 0, 0, GFP_KERNEL);
 
 	if (IS_ERR(tfm)) {
 		pr_err("failed to load transform for %s: %ld\n", algo,

crypto/testmgr.c +2 -1

index f9bce3d..076369f 100644
@@ -1563,7 +1563,8 @@ static int alg_test_skcipher(const struct alg_test_desc *desc,
 	struct crypto_ablkcipher *tfm;
 	int err = 0;
 
-	tfm = crypto_alloc_ablkcipher(driver, type | CRYPTO_ALG_INTERNAL, mask);
+	tfm = crypto_alloc_ablkcipher(driver, type | CRYPTO_ALG_INTERNAL, mask,
+								GFP_KERNEL);
 	if (IS_ERR(tfm)) {
 		printk(KERN_ERR "alg: skcipher: Failed to load transform for "
 		       "%s: %ld\n", driver, PTR_ERR(tfm));

drivers/crypto/ccp/ccp-crypto-aes-xts.c +2 -1

index 52c7395..54753ee 100644
@@ -192,7 +192,8 @@ static int ccp_aes_xts_cra_init(struct crypto_tfm *tfm)
 
 	fallback_tfm = crypto_alloc_ablkcipher(crypto_tfm_alg_name(tfm), 0,
 					       CRYPTO_ALG_ASYNC |
-					       CRYPTO_ALG_NEED_FALLBACK);
+					       CRYPTO_ALG_NEED_FALLBACK,
+					       GFP_KERNEL);
 	if (IS_ERR(fallback_tfm)) {
 		pr_warn("could not load fallback driver %s\n",
 			crypto_tfm_alg_name(tfm));

drivers/crypto/mxs-dcp.c +1 -1

index 59ed54e..4cac3a2 100644
@@ -486,7 +486,7 @@ static int mxs_dcp_aes_fallback_init(struct crypto_tfm *tfm)
 	struct dcp_async_ctx *actx = crypto_tfm_ctx(tfm);
 	struct crypto_ablkcipher *blk;
 
-	blk = crypto_alloc_ablkcipher(name, 0, flags);
+	blk = crypto_alloc_ablkcipher(name, 0, flags, GFP_KERNEL);
 	if (IS_ERR(blk))
 		return PTR_ERR(blk);

drivers/crypto/picoxcell_crypto.c +2 -1

index 5da5b98..148458e 100644
@@ -1085,7 +1085,8 @@ static int spacc_ablk_cra_init(struct crypto_tfm *tfm)
 	ctx->generic.engine = engine;
 	if (alg->cra_flags & CRYPTO_ALG_NEED_FALLBACK) {
 		ctx->sw_cipher = crypto_alloc_ablkcipher(alg->cra_name, 0,
-				CRYPTO_ALG_ASYNC | CRYPTO_ALG_NEED_FALLBACK);
+				CRYPTO_ALG_ASYNC | CRYPTO_ALG_NEED_FALLBACK,
+				GFP_KERNEL);
 		if (IS_ERR(ctx->sw_cipher)) {
 			dev_warn(engine->dev, "failed to allocate fallback for %s\n",
 				 alg->cra_name);

drivers/crypto/qce/ablkcipher.c +2 -1

index ad592de..e1742d8 100644
@@ -244,7 +244,8 @@ static int qce_ablkcipher_init(struct crypto_tfm *tfm)
 	ctx->fallback = crypto_alloc_ablkcipher(crypto_tfm_alg_name(tfm),
 						CRYPTO_ALG_TYPE_ABLKCIPHER,
 						CRYPTO_ALG_ASYNC |
-						CRYPTO_ALG_NEED_FALLBACK);
+						CRYPTO_ALG_NEED_FALLBACK,
+						GFP_KERNEL);
 	if (IS_ERR(ctx->fallback))
 		return PTR_ERR(ctx->fallback);

drivers/crypto/sahara.c +2 -1

index 6be377f..50a19c1 100644
@@ -765,7 +765,8 @@ static int sahara_aes_cra_init(struct crypto_tfm *tfm)
 	struct sahara_ctx *ctx = crypto_tfm_ctx(tfm);
 
 	ctx->fallback = crypto_alloc_ablkcipher(name, 0,
-				CRYPTO_ALG_ASYNC | CRYPTO_ALG_NEED_FALLBACK);
+				CRYPTO_ALG_ASYNC | CRYPTO_ALG_NEED_FALLBACK,
+				GFP_KERNEL);
 	if (IS_ERR(ctx->fallback)) {
 		pr_err("Error allocating fallback algo %s\n", name);
 		return PTR_ERR(ctx->fallback);

drivers/md/dm-crypt.c +2 -1

index 9eeea19..bde80c3 100644
@@ -1438,7 +1438,8 @@ static int crypt_alloc_tfms(struct crypt_config *cc, char *ciphermode)
 		return -ENOMEM;
 
 	for (i = 0; i < cc->tfms_count; i++) {
-		cc->tfms[i] = crypto_alloc_ablkcipher(ciphermode, 0, 0);
+		cc->tfms[i] = crypto_alloc_ablkcipher(ciphermode, 0, 0,
+							GFP_KERNEL);
 		if (IS_ERR(cc->tfms[i])) {
 			err = PTR_ERR(cc->tfms[i]);
 			crypt_free_tfms(cc);

fs/ecryptfs/crypto.c +2 -1

index 97315f2..7d0d9b2 100644
@@ -623,7 +623,8 @@ 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_ablkcipher(full_alg_name, 0, 0);
+	crypt_stat->tfm = crypto_alloc_ablkcipher(full_alg_name, 0, 0,
+							GFP_KERNEL);
 	if (IS_ERR(crypt_stat->tfm)) {
 		rc = PTR_ERR(crypt_stat->tfm);
 		crypt_stat->tfm = NULL;

fs/ext4/crypto.c +2 -1

index 8ff1527..28cbe92 100644
@@ -162,7 +162,8 @@ struct ext4_crypto_ctx *ext4_get_crypto_ctx(struct inode *inode)
 		switch (key->mode) {
 		case EXT4_ENCRYPTION_MODE_AES_256_XTS:
 			ctx->tfm = crypto_ablkcipher_tfm(
-				crypto_alloc_ablkcipher("xts(aes)", 0, 0));
+				crypto_alloc_ablkcipher("xts(aes)", 0, 0,
+							GFP_NOFS));
 			break;
 		case EXT4_ENCRYPTION_MODE_AES_256_GCM:
 			/* TODO(mhalcrow): AEAD w/ gcm(aes);

fs/ext4/crypto_fname.c +1 -1

index fded02f..cdd07c7 100644
@@ -372,7 +372,7 @@ struct ext4_fname_crypto_ctx *ext4_get_fname_crypto_ctx(
 		 * re-used */
 		if (ctx->ctfm == NULL) {
 			ctx->ctfm = crypto_alloc_ablkcipher("cts(cbc(aes))",
-					0, 0);
+					0, 0, GFP_KERNEL);
 		}
 		if (IS_ERR(ctx->ctfm)) {
 			res = PTR_ERR(ctx->ctfm);

fs/ext4/crypto_key.c +1 -1

index 52170d0..9ae7058 100644
@@ -45,7 +45,7 @@ static int ext4_derive_key_aes(char deriving_key[EXT4_AES_128_ECB_KEY_SIZE],
 	DECLARE_EXT4_COMPLETION_RESULT(ecr);
 	struct scatterlist src_sg, dst_sg;
 	struct crypto_ablkcipher *tfm = crypto_alloc_ablkcipher("ecb(aes)", 0,
-								0);
+								0, GFP_KERNEL);
 
 	if (IS_ERR(tfm)) {
 		res = PTR_ERR(tfm);

fs/f2fs/crypto.c +2 -1

index c6d1122..173727e 100644
@@ -161,7 +161,8 @@ struct f2fs_crypto_ctx *f2fs_get_crypto_ctx(struct inode *inode)
 		switch (ci->ci_data_mode) {
 		case F2FS_ENCRYPTION_MODE_AES_256_XTS:
 			ctx->tfm = crypto_ablkcipher_tfm(
-				crypto_alloc_ablkcipher("xts(aes)", 0, 0));
+				crypto_alloc_ablkcipher("xts(aes)", 0, 0,
+							GFP_NOFS));
 			break;
 		case F2FS_ENCRYPTION_MODE_AES_256_GCM:
 			/*

fs/f2fs/crypto_fname.c +1 -1

index 016c4b6..47e8e05 100644
@@ -275,7 +275,7 @@ int f2fs_setup_fname_crypto(struct inode *inode)
 		return -ENOKEY;
 	}
 
-	ctfm = crypto_alloc_ablkcipher("cts(cbc(aes))", 0, 0);
+	ctfm = crypto_alloc_ablkcipher("cts(cbc(aes))", 0, 0, GFP_KERNEL);
 	if (!ctfm || IS_ERR(ctfm)) {
 		res = ctfm ? PTR_ERR(ctfm) : -ENOMEM;
 		printk(KERN_DEBUG "%s: error (%d) allocating crypto tfm\n",

fs/f2fs/crypto_key.c +1 -1

index a25b164..6be5c9f 100644
@@ -48,7 +48,7 @@ static int f2fs_derive_key_aes(char deriving_key[F2FS_AES_128_ECB_KEY_SIZE],
 	DECLARE_F2FS_COMPLETION_RESULT(ecr);
 	struct scatterlist src_sg, dst_sg;
 	struct crypto_ablkcipher *tfm = crypto_alloc_ablkcipher("ecb(aes)", 0,
-								0);
+								0, GFP_KERNEL);
 
 	if (IS_ERR(tfm)) {
 		res = PTR_ERR(tfm);

include/linux/crypto.h +1 -1

index 10df5d2..b26ac44 100644
@@ -885,7 +885,7 @@ static inline u32 crypto_skcipher_mask(u32 mask)
  *	   of an error, PTR_ERR() returns the error code.
  */
 struct crypto_ablkcipher *crypto_alloc_ablkcipher(const char *alg_name,
-						  u32 type, u32 mask);
+					  u32 type, u32 mask, gfp_t gfp);
 
 static inline struct crypto_tfm *crypto_ablkcipher_tfm(
 	struct crypto_ablkcipher *tfm)

Signature · 1 line

2.1.1

Herbert Xu

maintainer

2015-05-19 06:32 UTC #4 lore raw

Quoted 2 lines · Jaegeuk Kim

Change log from v1:
- fix missing change

Please do not resend your patch until you have addressed my questions.

Signature · 10 lines

Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud 
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y

Herbert Xu

maintainer

2015-05-19 06:32 UTC #5 lore raw

In reply to Jaegeuk Kim

Quoted 10 lines · Jaegeuk Kim

The call path in fs/ext4/crypto.c is:
- writepage
- ext4_encrypt
- ext4_get_crypto_ctx
- crypto_alloc_ablkcipher
AFAIK, this way can achieve to reduce memory footprint gracefully. Just before submitting bios, fs allocates the required memory, and then end_io will free them in pair.

So where does the key get generated? The crypto tfm should be allocated when you generate the key.

Cheers,

Signature · 3 lines

Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Jaegeuk Kim

author maintainer

2015-05-19 06:58 UTC #6 lore raw

Quoted 8 lines · Herbert Xu

Quoted 7 lines · Jaegeuk Kim
The call path in fs/ext4/crypto.c is:
- writepage
- ext4_encrypt
- ext4_get_crypto_ctx
- crypto_alloc_ablkcipher

Quoted 6 lines

Quoted 3 lines
AFAIK, this way can achieve to reduce memory footprint gracefully. Just before submitting bios, fs allocates the required memory, and then end_io will free them in pair.
So where does the key get generated? The crypto tfm should be allocated when you generate the key.

In fs/ext4/crypto.c,

- writepage
- ext4_encrypt
- ext4_get_crypto_ctx
- crypto_alloc_ablkcipher
- ext4_page_crypto
- crypto_ablkcipher_setkey
- ablkcipher_request_alloc(GFP_NOFS)
- ablkcipher_request_set_crypt(PAGE_CACHE_SIZE)
- crypto_ablkcipher_encrypt

- end_io
- crypto_free_tfm

Thanks,

Quoted 6 lines

Cheers,

Signature · 3 lines

Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Herbert Xu

maintainer

2015-05-19 06:59 UTC #7 lore raw

Quoted 16 lines · Jaegeuk Kim

Quoted 3 lines
So where does the key get generated? The crypto tfm should be allocated when you generate the key.
In fs/ext4/crypto.c,
- writepage
- ext4_encrypt
- ext4_get_crypto_ctx
- crypto_alloc_ablkcipher
- ext4_page_crypto
- crypto_ablkcipher_setkey
- ablkcipher_request_alloc(GFP_NOFS)
- ablkcipher_request_set_crypt(PAGE_CACHE_SIZE)
- crypto_ablkcipher_encrypt

Quoted 2 lines

- end_io
- crypto_free_tfm

No that's where you set the key, not where you generate the key.

Signature · 3 lines

Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Jaegeuk Kim

author maintainer

2015-05-19 07:13 UTC #8 lore raw

Quoted 17 lines · Herbert Xu

Quoted 16 lines · Jaegeuk Kim
Quoted 3 lines
So where does the key get generated? The crypto tfm should be allocated when you generate the key.
In fs/ext4/crypto.c,
- writepage
- ext4_encrypt
- ext4_get_crypto_ctx
- crypto_alloc_ablkcipher
- ext4_page_crypto
- crypto_ablkcipher_setkey
- ablkcipher_request_alloc(GFP_NOFS)
- ablkcipher_request_set_crypt(PAGE_CACHE_SIZE)
- crypto_ablkcipher_encrypt

Quoted 4 lines

Quoted 2 lines
- end_io
- crypto_free_tfm
No that's where you set the key, not where you generate the key.

The key generation is done by ext4_generate_encryption_key in fs/ext4/crypto_key.c.
And, ext4_file_mmap and ext4_file_open trigger it.

Quoted 4 lines

Signature · 3 lines

Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Herbert Xu

maintainer

2015-05-19 07:15 UTC #9 lore raw

Quoted 4 lines · Jaegeuk Kim

The key generation is done by ext4_generate_encryption_key in fs/ext4/crypto_key.c.
And, ext4_file_mmap and ext4_file_open trigger it.

Well that's where you should be doing crypto_alloc_ablkcipher and crypto_ablkcipher_setkey.

The whole point of a crypto tfm is to represent a key so any time you get one you should create a crypto tfm. Carrying around a raw key is just wrong.

Cheers,

Signature · 3 lines

Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Jaegeuk Kim

author maintainer

2015-05-19 07:27 UTC #10 lore raw

Quoted 6 lines · Herbert Xu

Quoted 4 lines · Jaegeuk Kim
The key generation is done by ext4_generate_encryption_key in fs/ext4/crypto_key.c.
And, ext4_file_mmap and ext4_file_open trigger it.

Quoted 6 lines

Well that's where you should be doing crypto_alloc_ablkcipher and crypto_ablkcipher_setkey.
The whole point of a crypto tfm is to represent a key so any time you get one you should create a crypto tfm. Carrying around a raw key is just wrong.

So, IMHO, it can consume memory too much, since tfm should be allocated for every inodes and be alive until inode eviction.
Apart from giving GFP_NOFS, do you mean that it is a wrong approach?

Thanks,

Quoted 6 lines

Cheers,

Signature · 3 lines

Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Herbert Xu

maintainer

2015-05-19 07:30 UTC #11 lore raw

Quoted 3 lines · Jaegeuk Kim

So, IMHO, it can consume memory too much, since tfm should be allocated for every inodes and be alive until inode eviction.

Are you sure this is a real problem? Have you actually looked at how much memory it consumes?

Quoted 1 line

Apart from giving GFP_NOFS, do you mean that it is a wrong approach?

Allocating the tfm and setting a key on the data path is not acceptable.

Cheers,

Signature · 3 lines

Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Jaegeuk Kim

author maintainer

2015-05-19 07:59 UTC #12 lore raw

Quoted 7 lines · Herbert Xu

Quoted 3 lines · Jaegeuk Kim
So, IMHO, it can consume memory too much, since tfm should be allocated for every inodes and be alive until inode eviction.
Are you sure this is a real problem? Have you actually looked at how much memory it consumes?

I didn't measure the memory footprint. I can't tell it is a real problem, but at least, I hope it can reduce somewhat memory consumption. Because, already per-inode crypto structure consumes at least 84 bytes.

Quoted 5 lines

Quoted 2 lines
Apart from giving GFP_NOFS, do you mean that it is a wrong approach?
Allocating the tfm and setting a key on the data path is not acceptable.

Ok, I'll check it again and investigate in more detail how not to break this rule.

Thank you very much for the comments.

Quoted 6 lines

Cheers,

Signature · 3 lines

Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Theodore Ts'o

maintainer

2015-05-19 14:14 UTC #13 lore raw

In reply to Herbert Xu

Quoted 6 lines · Herbert Xu

Quoted 4 lines · Jaegeuk Kim
The key generation is done by ext4_generate_encryption_key in fs/ext4/crypto_key.c.
And, ext4_file_mmap and ext4_file_open trigger it.

Quoted 6 lines

Well that's where you should be doing crypto_alloc_ablkcipher and crypto_ablkcipher_setkey.
The whole point of a crypto tfm is to represent a key so any time you get one you should create a crypto tfm. Carrying around a raw key is just wrong.

There can be multiple reads going on in parallel, so we're currently creating tfm's as necessary. In fact one of the things that we've talked about doing is since there are some ARM cores where their "hardware acceleration" is slower than optimized software (sigh), and there are some Android applications (such as Facebook) that read *vast* quantities of data from flash on startup before painting a single pixel, that we might want to consider in some cases, parallelizing the decryption across multiple ARM cores. Figuring out when to do this, both in terms of the workload, how many cores to use to balance off against power utilization, how much (if ever) to use the hardware "accelerator", and just plain lack of time caused us not to go down that particular path.

We do have a tfm pointer hanging off the inode (currently only used for directories and file name encryption, where i_mutex is serializing us anyway), and in theory we could use that for the data path as well. We'd have to serialize access to it, which could be performance problem, and if the tfm is significantly larger than the raw key, we'd need to know when we should nuke the tfm.

After all, we don't want to have users waiting for their Facebook app to launch. :-)

- Ted

Herbert Xu

maintainer

2015-05-19 14:27 UTC #14 lore raw

Quoted 3 lines · Theodore Ts'o

There can be multiple reads going on in parallel, so we're currently creating tfm's as necessary. In fact one of the things that we've

A single tfm is fully-reentrant (as long as you don't change the key). So multiple reads/writes on a single file can all use one tfm with no locking at all.

There should be a single tfm per key. As your code appears to use one key per inode, that translates to one tfm per inode.

Quoted 10 lines

talked about doing is since there are some ARM cores where their "hardware acceleration" is slower than optimized software (sigh), and there are some Android applications (such as Facebook) that read *vast* quantities of data from flash on startup before painting a single pixel, that we might want to consider in some cases, parallelizing the decryption across multiple ARM cores. Figuring out when to do this, both in terms of the workload, how many cores to use to balance off against power utilization, how much (if ever) to use the hardware "accelerator", and just plain lack of time caused us not to go down that particular path.

We already have some support for such parallelisation in the form of pcrypt. It has been used on IPsec and I believe dmcrypt.

Quoted 6 lines

We do have a tfm pointer hanging off the inode (currently only used for directories and file name encryption, where i_mutex is serializing us anyway), and in theory we could use that for the data path as well. We'd have to serialize access to it, which could be performance problem, and if the tfm is significantly larger than the raw key, we'd need to know when we should nuke the tfm.

As long as an inode only has one key, you don't need any serialisation.

Cheers,

Signature · 3 lines

Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Theodore Ts'o

maintainer

2015-05-19 14:39 UTC #15 lore raw

Quoted 8 lines · Herbert Xu

Quoted 3 lines · Theodore Ts'o
There can be multiple reads going on in parallel, so we're currently creating tfm's as necessary. In fact one of the things that we've
A single tfm is fully-reentrant (as long as you don't change the key). So multiple reads/writes on a single file can all use one tfm with no locking at all.

Cool, we didn't realize this was the case. Excellent, this makes life much simpler for us! :-)

Thanks,

- Ted

Steffen Klassert

2015-05-20 07:21 UTC #16 lore raw

In reply to Herbert Xu

Quoted 23 lines · Herbert Xu

Quoted 3 lines · Theodore Ts'o
There can be multiple reads going on in parallel, so we're currently creating tfm's as necessary. In fact one of the things that we've
A single tfm is fully-reentrant (as long as you don't change the key). So multiple reads/writes on a single file can all use one tfm with no locking at all.
There should be a single tfm per key. As your code appears to use one key per inode, that translates to one tfm per inode.
Quoted 10 lines
talked about doing is since there are some ARM cores where their "hardware acceleration" is slower than optimized software (sigh), and there are some Android applications (such as Facebook) that read *vast* quantities of data from flash on startup before painting a single pixel, that we might want to consider in some cases, parallelizing the decryption across multiple ARM cores. Figuring out when to do this, both in terms of the workload, how many cores to use to balance off against power utilization, how much (if ever) to use the hardware "accelerator", and just plain lack of time caused us not to go down that particular path.

Quoted 2 lines

We already have some support for such parallelisation in the form of pcrypt. It has been used on IPsec and I believe dmcrypt.

The current pcrypt version is used just for IPsec because it supports only AEAD type algorithms and does not support request backlog. But I have patches to support ablkcipher algorithms and request backlog. I could provide them if there is interest in it.

Theodore Ts'o

maintainer

2015-05-20 14:59 UTC #17 lore raw

Quoted 4 lines · Steffen Klassert

The current pcrypt version is used just for IPsec because it supports only AEAD type algorithms and does not support request backlog. But I have patches to support ablkcipher algorithms and request backlog. I could provide them if there is interest in it.

I don't know the crypto layer well enough, and I certainly don't know how to deal with things like ARM CPU's with "big-little" architectures to understand what we might need to do to power optimize things for mobile handsets. But if someone has time to look at this, that would be great.

- Ted

Ard Biesheuvel

2015-05-20 15:30 UTC #18 lore raw

Quoted 6 lines · Theodore Ts'o

Quoted 4 lines · Steffen Klassert
The current pcrypt version is used just for IPsec because it supports only AEAD type algorithms and does not support request backlog. But I have patches to support ablkcipher algorithms and request backlog. I could provide them if there is interest in it.

Quoted 6 lines

I don't know the crypto layer well enough, and I certainly don't know how to deal with things like ARM CPU's with "big-little" architectures to understand what we might need to do to power optimize things for mobile handsets. But if someone has time to look at this, that would be great.

I'd be interested in getting involved: I'm fairly intimate with the crypto layer and the ARM/arm64 architectures, since I wrote a big chunk of the ARM/arm64 CPU based crypto drivers.

However, it's quite a can of worms you're opening here. Speed is easily quantified, and it may even be feasible to introduce some kind of boottime benchmark to select the fastest implementation available (like already exists for xor and raid6, for instance). @Herbert: was something like this ever proposed? And would you consider merging it if it were implemented adequately?

Power efficiency is *much* harder, since you're not only dealing with big.LITTLE, but also NEON versus ALU (i.e., SIMD math versus integer math) and CPU versus hw accelerator based crypto. (We have the NEON based bit sliced AES as an example: we know it does fundamentally more work by calculating values that the table based AES implementation reads from a lookup table, so while it is a lot faster on most NEON implementations, it is most likely not as power efficient) Even if it were possible to quantify the power efficiency in the first place, whether the most power efficient implementation should be preferred over the fastest one is a policy decision which does not really belong inside the kernel.

Are there any background materials or other references you can share that shed a bit more light on this?

Herbert Xu

maintainer

2015-05-20 15:42 UTC #19 lore raw

Quoted 7 lines · Ard Biesheuvel

However, it's quite a can of worms you're opening here. Speed is easily quantified, and it may even be feasible to introduce some kind of boottime benchmark to select the fastest implementation available (like already exists for xor and raid6, for instance). @Herbert: was something like this ever proposed? And would you consider merging it if it were implemented adequately?

Up until now static priorities have been sufficient in selecting the best implementation. However, if you can show us a case where a given implementation is slower on one host but faster on another then sure we can add a run-time test and priority adjustment for it.

Cheers,

Signature · 3 lines

Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt