All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
@ 2015-05-19  5:46 ` Jaegeuk Kim
  0 siblings, 0 replies; 44+ messages in thread
From: Jaegeuk Kim @ 2015-05-19  5:46 UTC (permalink / raw)
  To: herbert, davem, linux-crypto, linux-ext4, linux-kernel,
	linux-f2fs-devel, ecryptfs, linux-arm-kernel
  Cc: 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.

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 <jaegeuk@kernel.org>
---
 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(-)

diff --git a/arch/x86/crypto/aesni-intel_glue.c b/arch/x86/crypto/aesni-intel_glue.c
index 112cefa..5a7fe76 100644
--- a/arch/x86/crypto/aesni-intel_glue.c
+++ b/arch/x86/crypto/aesni-intel_glue.c
@@ -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);
 
diff --git a/crypto/ablkcipher.c b/crypto/ablkcipher.c
index db201bca..3706e4a 100644
--- a/crypto/ablkcipher.c
+++ b/crypto/ablkcipher.c
@@ -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);
 
diff --git a/crypto/aead.c b/crypto/aead.c
index 2222710..b220a0dd 100644
--- a/crypto/aead.c
+++ b/crypto/aead.c
@@ -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);
 
diff --git a/crypto/algapi.c b/crypto/algapi.c
index d2627a3..1a00274 100644
--- a/crypto/algapi.c
+++ b/crypto/algapi.c
@@ -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;
 
diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 9450752..89730a7 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -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)
diff --git a/crypto/api.c b/crypto/api.c
index afe4610..887346b 100644
--- a/crypto/api.c
+++ b/crypto/api.c
@@ -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;
 
diff --git a/crypto/internal.h b/crypto/internal.h
index bd39bfc..bd88be7 100644
--- a/crypto/internal.h
+++ b/crypto/internal.h
@@ -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,
diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
index 1a28001..e6986e6 100644
--- a/crypto/tcrypt.c
+++ b/crypto/tcrypt.c
@@ -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,
diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index f9bce3d..076369f 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -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));
diff --git a/drivers/crypto/ccp/ccp-crypto-aes-xts.c b/drivers/crypto/ccp/ccp-crypto-aes-xts.c
index 52c7395..54753ee 100644
--- a/drivers/crypto/ccp/ccp-crypto-aes-xts.c
+++ b/drivers/crypto/ccp/ccp-crypto-aes-xts.c
@@ -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));
diff --git a/drivers/crypto/mxs-dcp.c b/drivers/crypto/mxs-dcp.c
index 59ed54e..4cac3a2 100644
--- a/drivers/crypto/mxs-dcp.c
+++ b/drivers/crypto/mxs-dcp.c
@@ -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);
 
diff --git a/drivers/crypto/picoxcell_crypto.c b/drivers/crypto/picoxcell_crypto.c
index 5da5b98..148458e 100644
--- a/drivers/crypto/picoxcell_crypto.c
+++ b/drivers/crypto/picoxcell_crypto.c
@@ -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);
diff --git a/drivers/crypto/qce/ablkcipher.c b/drivers/crypto/qce/ablkcipher.c
index ad592de..e1742d8 100644
--- a/drivers/crypto/qce/ablkcipher.c
+++ b/drivers/crypto/qce/ablkcipher.c
@@ -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);
 
diff --git a/drivers/crypto/sahara.c b/drivers/crypto/sahara.c
index 6be377f..50a19c1 100644
--- a/drivers/crypto/sahara.c
+++ b/drivers/crypto/sahara.c
@@ -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);
diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 9eeea19..bde80c3 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -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);
diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c
index 97315f2..7d0d9b2 100644
--- a/fs/ecryptfs/crypto.c
+++ b/fs/ecryptfs/crypto.c
@@ -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;
diff --git a/fs/ext4/crypto.c b/fs/ext4/crypto.c
index 8ff1527..28cbe92 100644
--- a/fs/ext4/crypto.c
+++ b/fs/ext4/crypto.c
@@ -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);
diff --git a/fs/ext4/crypto_fname.c b/fs/ext4/crypto_fname.c
index fded02f..cdd07c7 100644
--- a/fs/ext4/crypto_fname.c
+++ b/fs/ext4/crypto_fname.c
@@ -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);
diff --git a/fs/f2fs/crypto.c b/fs/f2fs/crypto.c
index c6d1122..173727e 100644
--- a/fs/f2fs/crypto.c
+++ b/fs/f2fs/crypto.c
@@ -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:
 			/*
diff --git a/fs/f2fs/crypto_fname.c b/fs/f2fs/crypto_fname.c
index 016c4b6..47e8e05 100644
--- a/fs/f2fs/crypto_fname.c
+++ b/fs/f2fs/crypto_fname.c
@@ -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",
diff --git a/fs/f2fs/crypto_key.c b/fs/f2fs/crypto_key.c
index a25b164..6be5c9f 100644
--- a/fs/f2fs/crypto_key.c
+++ b/fs/f2fs/crypto_key.c
@@ -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);
diff --git a/include/linux/crypto.h b/include/linux/crypto.h
index 10df5d2..b26ac44 100644
--- a/include/linux/crypto.h
+++ b/include/linux/crypto.h
@@ -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)
-- 
2.1.1

^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
@ 2015-05-19  5:46 ` Jaegeuk Kim
  0 siblings, 0 replies; 44+ messages in thread
From: Jaegeuk Kim @ 2015-05-19  5:46 UTC (permalink / raw)
  To: linux-arm-kernel

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 <jaegeuk@kernel.org>
---
 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(-)

diff --git a/arch/x86/crypto/aesni-intel_glue.c b/arch/x86/crypto/aesni-intel_glue.c
index 112cefa..5a7fe76 100644
--- a/arch/x86/crypto/aesni-intel_glue.c
+++ b/arch/x86/crypto/aesni-intel_glue.c
@@ -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);
 
diff --git a/crypto/ablkcipher.c b/crypto/ablkcipher.c
index db201bca..3706e4a 100644
--- a/crypto/ablkcipher.c
+++ b/crypto/ablkcipher.c
@@ -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);
 
diff --git a/crypto/aead.c b/crypto/aead.c
index 2222710..b220a0dd 100644
--- a/crypto/aead.c
+++ b/crypto/aead.c
@@ -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);
 
diff --git a/crypto/algapi.c b/crypto/algapi.c
index d2627a3..1a00274 100644
--- a/crypto/algapi.c
+++ b/crypto/algapi.c
@@ -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;
 
diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 9450752..89730a7 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -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)
diff --git a/crypto/api.c b/crypto/api.c
index afe4610..887346b 100644
--- a/crypto/api.c
+++ b/crypto/api.c
@@ -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;
 
diff --git a/crypto/internal.h b/crypto/internal.h
index bd39bfc..bd88be7 100644
--- a/crypto/internal.h
+++ b/crypto/internal.h
@@ -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,
diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
index 1a28001..e6986e6 100644
--- a/crypto/tcrypt.c
+++ b/crypto/tcrypt.c
@@ -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,
diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index f9bce3d..076369f 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -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));
diff --git a/drivers/crypto/ccp/ccp-crypto-aes-xts.c b/drivers/crypto/ccp/ccp-crypto-aes-xts.c
index 52c7395..54753ee 100644
--- a/drivers/crypto/ccp/ccp-crypto-aes-xts.c
+++ b/drivers/crypto/ccp/ccp-crypto-aes-xts.c
@@ -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));
diff --git a/drivers/crypto/mxs-dcp.c b/drivers/crypto/mxs-dcp.c
index 59ed54e..4cac3a2 100644
--- a/drivers/crypto/mxs-dcp.c
+++ b/drivers/crypto/mxs-dcp.c
@@ -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);
 
diff --git a/drivers/crypto/picoxcell_crypto.c b/drivers/crypto/picoxcell_crypto.c
index 5da5b98..148458e 100644
--- a/drivers/crypto/picoxcell_crypto.c
+++ b/drivers/crypto/picoxcell_crypto.c
@@ -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);
diff --git a/drivers/crypto/qce/ablkcipher.c b/drivers/crypto/qce/ablkcipher.c
index ad592de..e1742d8 100644
--- a/drivers/crypto/qce/ablkcipher.c
+++ b/drivers/crypto/qce/ablkcipher.c
@@ -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);
 
diff --git a/drivers/crypto/sahara.c b/drivers/crypto/sahara.c
index 6be377f..50a19c1 100644
--- a/drivers/crypto/sahara.c
+++ b/drivers/crypto/sahara.c
@@ -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);
diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 9eeea19..bde80c3 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -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);
diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c
index 97315f2..7d0d9b2 100644
--- a/fs/ecryptfs/crypto.c
+++ b/fs/ecryptfs/crypto.c
@@ -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;
diff --git a/fs/ext4/crypto.c b/fs/ext4/crypto.c
index 8ff1527..28cbe92 100644
--- a/fs/ext4/crypto.c
+++ b/fs/ext4/crypto.c
@@ -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);
diff --git a/fs/ext4/crypto_fname.c b/fs/ext4/crypto_fname.c
index fded02f..cdd07c7 100644
--- a/fs/ext4/crypto_fname.c
+++ b/fs/ext4/crypto_fname.c
@@ -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);
diff --git a/fs/f2fs/crypto.c b/fs/f2fs/crypto.c
index c6d1122..173727e 100644
--- a/fs/f2fs/crypto.c
+++ b/fs/f2fs/crypto.c
@@ -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:
 			/*
diff --git a/fs/f2fs/crypto_fname.c b/fs/f2fs/crypto_fname.c
index 016c4b6..47e8e05 100644
--- a/fs/f2fs/crypto_fname.c
+++ b/fs/f2fs/crypto_fname.c
@@ -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",
diff --git a/fs/f2fs/crypto_key.c b/fs/f2fs/crypto_key.c
index a25b164..6be5c9f 100644
--- a/fs/f2fs/crypto_key.c
+++ b/fs/f2fs/crypto_key.c
@@ -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);
diff --git a/include/linux/crypto.h b/include/linux/crypto.h
index 10df5d2..b26ac44 100644
--- a/include/linux/crypto.h
+++ b/include/linux/crypto.h
@@ -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)
-- 
2.1.1

^ permalink raw reply related	[flat|nested] 44+ messages in thread

* Re: [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
  2015-05-19  5:46 ` Jaegeuk Kim
@ 2015-05-19  5:49   ` Herbert Xu
  -1 siblings, 0 replies; 44+ messages in thread
From: Herbert Xu @ 2015-05-19  5:49 UTC (permalink / raw)
  To: Jaegeuk Kim
  Cc: davem, linux-crypto, linux-ext4, linux-kernel, linux-f2fs-devel,
	ecryptfs, linux-arm-kernel

On Mon, May 18, 2015 at 10:46:56PM -0700, Jaegeuk Kim wrote:
> 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,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
@ 2015-05-19  5:49   ` Herbert Xu
  0 siblings, 0 replies; 44+ messages in thread
From: Herbert Xu @ 2015-05-19  5:49 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, May 18, 2015 at 10:46:56PM -0700, Jaegeuk Kim wrote:
> 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,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
  2015-05-19  5:49   ` Herbert Xu
@ 2015-05-19  6:24     ` Jaegeuk Kim
  -1 siblings, 0 replies; 44+ messages in thread
From: Jaegeuk Kim @ 2015-05-19  6:24 UTC (permalink / raw)
  To: Herbert Xu
  Cc: davem, linux-crypto, linux-ext4, linux-kernel, linux-f2fs-devel,
	ecryptfs, linux-arm-kernel

On Tue, May 19, 2015 at 01:49:45PM +0800, Herbert Xu wrote:
> On Mon, May 18, 2015 at 10:46:56PM -0700, Jaegeuk Kim wrote:
> > 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?
> 

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,

> Cheers,
> -- 
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> 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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
@ 2015-05-19  6:24     ` Jaegeuk Kim
  0 siblings, 0 replies; 44+ messages in thread
From: Jaegeuk Kim @ 2015-05-19  6:24 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, May 19, 2015 at 01:49:45PM +0800, Herbert Xu wrote:
> On Mon, May 18, 2015 at 10:46:56PM -0700, Jaegeuk Kim wrote:
> > 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?
> 

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,

> Cheers,
> -- 
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> 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/

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [f2fs-dev] [PATCH v2] crypto: allow to assign gfp_t for __crypto_alloc_tfm
  2015-05-19  6:24     ` Jaegeuk Kim
@ 2015-05-19  6:31       ` Jaegeuk Kim
  -1 siblings, 0 replies; 44+ messages in thread
From: Jaegeuk Kim @ 2015-05-19  6:31 UTC (permalink / raw)
  To: Herbert Xu
  Cc: ecryptfs, linux-kernel, linux-f2fs-devel, linux-crypto,
	linux-ext4, davem, linux-arm-kernel

 Change log from v1:
  - fix missing change

-- >8 --

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 <jaegeuk@kernel.org>
---
 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(-)

diff --git a/arch/x86/crypto/aesni-intel_glue.c b/arch/x86/crypto/aesni-intel_glue.c
index 112cefa..5a7fe76 100644
--- a/arch/x86/crypto/aesni-intel_glue.c
+++ b/arch/x86/crypto/aesni-intel_glue.c
@@ -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);
 
diff --git a/crypto/ablkcipher.c b/crypto/ablkcipher.c
index db201bca..3706e4a 100644
--- a/crypto/ablkcipher.c
+++ b/crypto/ablkcipher.c
@@ -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);
 
diff --git a/crypto/aead.c b/crypto/aead.c
index 2222710..b220a0dd 100644
--- a/crypto/aead.c
+++ b/crypto/aead.c
@@ -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);
 
diff --git a/crypto/algapi.c b/crypto/algapi.c
index d2627a3..1a00274 100644
--- a/crypto/algapi.c
+++ b/crypto/algapi.c
@@ -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;
 
diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 9450752..89730a7 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -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)
diff --git a/crypto/api.c b/crypto/api.c
index afe4610..887346b 100644
--- a/crypto/api.c
+++ b/crypto/api.c
@@ -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;
 
diff --git a/crypto/internal.h b/crypto/internal.h
index bd39bfc..bd88be7 100644
--- a/crypto/internal.h
+++ b/crypto/internal.h
@@ -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,
diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
index 1a28001..e6986e6 100644
--- a/crypto/tcrypt.c
+++ b/crypto/tcrypt.c
@@ -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,
diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index f9bce3d..076369f 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -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));
diff --git a/drivers/crypto/ccp/ccp-crypto-aes-xts.c b/drivers/crypto/ccp/ccp-crypto-aes-xts.c
index 52c7395..54753ee 100644
--- a/drivers/crypto/ccp/ccp-crypto-aes-xts.c
+++ b/drivers/crypto/ccp/ccp-crypto-aes-xts.c
@@ -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));
diff --git a/drivers/crypto/mxs-dcp.c b/drivers/crypto/mxs-dcp.c
index 59ed54e..4cac3a2 100644
--- a/drivers/crypto/mxs-dcp.c
+++ b/drivers/crypto/mxs-dcp.c
@@ -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);
 
diff --git a/drivers/crypto/picoxcell_crypto.c b/drivers/crypto/picoxcell_crypto.c
index 5da5b98..148458e 100644
--- a/drivers/crypto/picoxcell_crypto.c
+++ b/drivers/crypto/picoxcell_crypto.c
@@ -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);
diff --git a/drivers/crypto/qce/ablkcipher.c b/drivers/crypto/qce/ablkcipher.c
index ad592de..e1742d8 100644
--- a/drivers/crypto/qce/ablkcipher.c
+++ b/drivers/crypto/qce/ablkcipher.c
@@ -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);
 
diff --git a/drivers/crypto/sahara.c b/drivers/crypto/sahara.c
index 6be377f..50a19c1 100644
--- a/drivers/crypto/sahara.c
+++ b/drivers/crypto/sahara.c
@@ -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);
diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 9eeea19..bde80c3 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -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);
diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c
index 97315f2..7d0d9b2 100644
--- a/fs/ecryptfs/crypto.c
+++ b/fs/ecryptfs/crypto.c
@@ -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;
diff --git a/fs/ext4/crypto.c b/fs/ext4/crypto.c
index 8ff1527..28cbe92 100644
--- a/fs/ext4/crypto.c
+++ b/fs/ext4/crypto.c
@@ -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);
diff --git a/fs/ext4/crypto_fname.c b/fs/ext4/crypto_fname.c
index fded02f..cdd07c7 100644
--- a/fs/ext4/crypto_fname.c
+++ b/fs/ext4/crypto_fname.c
@@ -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);
diff --git a/fs/ext4/crypto_key.c b/fs/ext4/crypto_key.c
index 52170d0..9ae7058 100644
--- a/fs/ext4/crypto_key.c
+++ b/fs/ext4/crypto_key.c
@@ -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);
diff --git a/fs/f2fs/crypto.c b/fs/f2fs/crypto.c
index c6d1122..173727e 100644
--- a/fs/f2fs/crypto.c
+++ b/fs/f2fs/crypto.c
@@ -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:
 			/*
diff --git a/fs/f2fs/crypto_fname.c b/fs/f2fs/crypto_fname.c
index 016c4b6..47e8e05 100644
--- a/fs/f2fs/crypto_fname.c
+++ b/fs/f2fs/crypto_fname.c
@@ -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",
diff --git a/fs/f2fs/crypto_key.c b/fs/f2fs/crypto_key.c
index a25b164..6be5c9f 100644
--- a/fs/f2fs/crypto_key.c
+++ b/fs/f2fs/crypto_key.c
@@ -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);
diff --git a/include/linux/crypto.h b/include/linux/crypto.h
index 10df5d2..b26ac44 100644
--- a/include/linux/crypto.h
+++ b/include/linux/crypto.h
@@ -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)
-- 
2.1.1

^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [f2fs-dev] [PATCH v2] crypto: allow to assign gfp_t for __crypto_alloc_tfm
@ 2015-05-19  6:31       ` Jaegeuk Kim
  0 siblings, 0 replies; 44+ messages in thread
From: Jaegeuk Kim @ 2015-05-19  6:31 UTC (permalink / raw)
  To: linux-arm-kernel

 Change log from v1:
  - fix missing change

-- >8 --

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 <jaegeuk@kernel.org>
---
 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(-)

diff --git a/arch/x86/crypto/aesni-intel_glue.c b/arch/x86/crypto/aesni-intel_glue.c
index 112cefa..5a7fe76 100644
--- a/arch/x86/crypto/aesni-intel_glue.c
+++ b/arch/x86/crypto/aesni-intel_glue.c
@@ -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);
 
diff --git a/crypto/ablkcipher.c b/crypto/ablkcipher.c
index db201bca..3706e4a 100644
--- a/crypto/ablkcipher.c
+++ b/crypto/ablkcipher.c
@@ -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);
 
diff --git a/crypto/aead.c b/crypto/aead.c
index 2222710..b220a0dd 100644
--- a/crypto/aead.c
+++ b/crypto/aead.c
@@ -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);
 
diff --git a/crypto/algapi.c b/crypto/algapi.c
index d2627a3..1a00274 100644
--- a/crypto/algapi.c
+++ b/crypto/algapi.c
@@ -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;
 
diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 9450752..89730a7 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -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)
diff --git a/crypto/api.c b/crypto/api.c
index afe4610..887346b 100644
--- a/crypto/api.c
+++ b/crypto/api.c
@@ -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;
 
diff --git a/crypto/internal.h b/crypto/internal.h
index bd39bfc..bd88be7 100644
--- a/crypto/internal.h
+++ b/crypto/internal.h
@@ -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,
diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
index 1a28001..e6986e6 100644
--- a/crypto/tcrypt.c
+++ b/crypto/tcrypt.c
@@ -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,
diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index f9bce3d..076369f 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -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));
diff --git a/drivers/crypto/ccp/ccp-crypto-aes-xts.c b/drivers/crypto/ccp/ccp-crypto-aes-xts.c
index 52c7395..54753ee 100644
--- a/drivers/crypto/ccp/ccp-crypto-aes-xts.c
+++ b/drivers/crypto/ccp/ccp-crypto-aes-xts.c
@@ -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));
diff --git a/drivers/crypto/mxs-dcp.c b/drivers/crypto/mxs-dcp.c
index 59ed54e..4cac3a2 100644
--- a/drivers/crypto/mxs-dcp.c
+++ b/drivers/crypto/mxs-dcp.c
@@ -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);
 
diff --git a/drivers/crypto/picoxcell_crypto.c b/drivers/crypto/picoxcell_crypto.c
index 5da5b98..148458e 100644
--- a/drivers/crypto/picoxcell_crypto.c
+++ b/drivers/crypto/picoxcell_crypto.c
@@ -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);
diff --git a/drivers/crypto/qce/ablkcipher.c b/drivers/crypto/qce/ablkcipher.c
index ad592de..e1742d8 100644
--- a/drivers/crypto/qce/ablkcipher.c
+++ b/drivers/crypto/qce/ablkcipher.c
@@ -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);
 
diff --git a/drivers/crypto/sahara.c b/drivers/crypto/sahara.c
index 6be377f..50a19c1 100644
--- a/drivers/crypto/sahara.c
+++ b/drivers/crypto/sahara.c
@@ -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);
diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 9eeea19..bde80c3 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -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);
diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c
index 97315f2..7d0d9b2 100644
--- a/fs/ecryptfs/crypto.c
+++ b/fs/ecryptfs/crypto.c
@@ -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;
diff --git a/fs/ext4/crypto.c b/fs/ext4/crypto.c
index 8ff1527..28cbe92 100644
--- a/fs/ext4/crypto.c
+++ b/fs/ext4/crypto.c
@@ -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);
diff --git a/fs/ext4/crypto_fname.c b/fs/ext4/crypto_fname.c
index fded02f..cdd07c7 100644
--- a/fs/ext4/crypto_fname.c
+++ b/fs/ext4/crypto_fname.c
@@ -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);
diff --git a/fs/ext4/crypto_key.c b/fs/ext4/crypto_key.c
index 52170d0..9ae7058 100644
--- a/fs/ext4/crypto_key.c
+++ b/fs/ext4/crypto_key.c
@@ -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);
diff --git a/fs/f2fs/crypto.c b/fs/f2fs/crypto.c
index c6d1122..173727e 100644
--- a/fs/f2fs/crypto.c
+++ b/fs/f2fs/crypto.c
@@ -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:
 			/*
diff --git a/fs/f2fs/crypto_fname.c b/fs/f2fs/crypto_fname.c
index 016c4b6..47e8e05 100644
--- a/fs/f2fs/crypto_fname.c
+++ b/fs/f2fs/crypto_fname.c
@@ -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",
diff --git a/fs/f2fs/crypto_key.c b/fs/f2fs/crypto_key.c
index a25b164..6be5c9f 100644
--- a/fs/f2fs/crypto_key.c
+++ b/fs/f2fs/crypto_key.c
@@ -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);
diff --git a/include/linux/crypto.h b/include/linux/crypto.h
index 10df5d2..b26ac44 100644
--- a/include/linux/crypto.h
+++ b/include/linux/crypto.h
@@ -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)
-- 
2.1.1

^ permalink raw reply related	[flat|nested] 44+ messages in thread

* Re: [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
  2015-05-19  6:24     ` Jaegeuk Kim
@ 2015-05-19  6:32       ` Herbert Xu
  -1 siblings, 0 replies; 44+ messages in thread
From: Herbert Xu @ 2015-05-19  6:32 UTC (permalink / raw)
  To: Jaegeuk Kim
  Cc: davem, linux-crypto, linux-ext4, linux-kernel, linux-f2fs-devel,
	ecryptfs, linux-arm-kernel

On Mon, May 18, 2015 at 11:24:30PM -0700, Jaegeuk Kim wrote:
>
> 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,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
@ 2015-05-19  6:32       ` Herbert Xu
  0 siblings, 0 replies; 44+ messages in thread
From: Herbert Xu @ 2015-05-19  6:32 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, May 18, 2015 at 11:24:30PM -0700, Jaegeuk Kim wrote:
>
> 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,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v2] crypto: allow to assign gfp_t for __crypto_alloc_tfm
  2015-05-19  6:31       ` Jaegeuk Kim
  (?)
@ 2015-05-19  6:32         ` Herbert Xu
  -1 siblings, 0 replies; 44+ messages in thread
From: Herbert Xu @ 2015-05-19  6:32 UTC (permalink / raw)
  To: Jaegeuk Kim
  Cc: ecryptfs, linux-kernel, linux-f2fs-devel, linux-crypto,
	linux-ext4, davem, linux-arm-kernel

On Mon, May 18, 2015 at 11:31:42PM -0700, Jaegeuk Kim wrote:
>  Change log from v1:
>   - fix missing change

Please do not resend your patch until you have addressed my
questions.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
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

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [f2fs-dev] [PATCH v2] crypto: allow to assign gfp_t for __crypto_alloc_tfm
@ 2015-05-19  6:32         ` Herbert Xu
  0 siblings, 0 replies; 44+ messages in thread
From: Herbert Xu @ 2015-05-19  6:32 UTC (permalink / raw)
  To: Jaegeuk Kim
  Cc: ecryptfs, linux-kernel, linux-f2fs-devel, linux-crypto,
	linux-ext4, davem, linux-arm-kernel

On Mon, May 18, 2015 at 11:31:42PM -0700, Jaegeuk Kim wrote:
>  Change log from v1:
>   - fix missing change

Please do not resend your patch until you have addressed my
questions.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [f2fs-dev] [PATCH v2] crypto: allow to assign gfp_t for __crypto_alloc_tfm
@ 2015-05-19  6:32         ` Herbert Xu
  0 siblings, 0 replies; 44+ messages in thread
From: Herbert Xu @ 2015-05-19  6:32 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, May 18, 2015 at 11:31:42PM -0700, Jaegeuk Kim wrote:
>  Change log from v1:
>   - fix missing change

Please do not resend your patch until you have addressed my
questions.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
  2015-05-19  6:32       ` Herbert Xu
@ 2015-05-19  6:58         ` Jaegeuk Kim
  -1 siblings, 0 replies; 44+ messages in thread
From: Jaegeuk Kim @ 2015-05-19  6:58 UTC (permalink / raw)
  To: Herbert Xu
  Cc: davem, linux-crypto, linux-ext4, linux-kernel, linux-f2fs-devel,
	ecryptfs, linux-arm-kernel

On Tue, May 19, 2015 at 02:32:11PM +0800, Herbert Xu wrote:
> On Mon, May 18, 2015 at 11:24:30PM -0700, Jaegeuk Kim wrote:
> >
> > 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.

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,

> 
> Cheers,
> -- 
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
@ 2015-05-19  6:58         ` Jaegeuk Kim
  0 siblings, 0 replies; 44+ messages in thread
From: Jaegeuk Kim @ 2015-05-19  6:58 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, May 19, 2015 at 02:32:11PM +0800, Herbert Xu wrote:
> On Mon, May 18, 2015 at 11:24:30PM -0700, Jaegeuk Kim wrote:
> >
> > 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.

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,

> 
> Cheers,
> -- 
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
  2015-05-19  6:58         ` Jaegeuk Kim
@ 2015-05-19  6:59           ` Herbert Xu
  -1 siblings, 0 replies; 44+ messages in thread
From: Herbert Xu @ 2015-05-19  6:59 UTC (permalink / raw)
  To: Jaegeuk Kim
  Cc: davem, linux-crypto, linux-ext4, linux-kernel, linux-f2fs-devel,
	ecryptfs, linux-arm-kernel

On Mon, May 18, 2015 at 11:58:12PM -0700, Jaegeuk Kim wrote:
>
> > 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

No that's where you set the key, not where you generate the key.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
@ 2015-05-19  6:59           ` Herbert Xu
  0 siblings, 0 replies; 44+ messages in thread
From: Herbert Xu @ 2015-05-19  6:59 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, May 18, 2015 at 11:58:12PM -0700, Jaegeuk Kim wrote:
>
> > 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

No that's where you set the key, not where you generate the key.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
  2015-05-19  6:59           ` Herbert Xu
@ 2015-05-19  7:13             ` Jaegeuk Kim
  -1 siblings, 0 replies; 44+ messages in thread
From: Jaegeuk Kim @ 2015-05-19  7:13 UTC (permalink / raw)
  To: Herbert Xu
  Cc: davem, linux-crypto, linux-ext4, linux-kernel, linux-f2fs-devel,
	ecryptfs, linux-arm-kernel

On Tue, May 19, 2015 at 02:59:29PM +0800, Herbert Xu wrote:
> On Mon, May 18, 2015 at 11:58:12PM -0700, Jaegeuk Kim wrote:
> >
> > > 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
> 
> 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.

> -- 
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
@ 2015-05-19  7:13             ` Jaegeuk Kim
  0 siblings, 0 replies; 44+ messages in thread
From: Jaegeuk Kim @ 2015-05-19  7:13 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, May 19, 2015 at 02:59:29PM +0800, Herbert Xu wrote:
> On Mon, May 18, 2015 at 11:58:12PM -0700, Jaegeuk Kim wrote:
> >
> > > 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
> 
> 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.

> -- 
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
  2015-05-19  7:13             ` Jaegeuk Kim
@ 2015-05-19  7:15               ` Herbert Xu
  -1 siblings, 0 replies; 44+ messages in thread
From: Herbert Xu @ 2015-05-19  7:15 UTC (permalink / raw)
  To: Jaegeuk Kim
  Cc: davem, linux-crypto, linux-ext4, linux-kernel, linux-f2fs-devel,
	ecryptfs, linux-arm-kernel

On Tue, May 19, 2015 at 12:13:17AM -0700, Jaegeuk Kim wrote:
>
> 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,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
@ 2015-05-19  7:15               ` Herbert Xu
  0 siblings, 0 replies; 44+ messages in thread
From: Herbert Xu @ 2015-05-19  7:15 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, May 19, 2015 at 12:13:17AM -0700, Jaegeuk Kim wrote:
>
> 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,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
  2015-05-19  7:15               ` Herbert Xu
@ 2015-05-19  7:27                 ` Jaegeuk Kim
  -1 siblings, 0 replies; 44+ messages in thread
From: Jaegeuk Kim @ 2015-05-19  7:27 UTC (permalink / raw)
  To: Herbert Xu
  Cc: davem, linux-crypto, linux-ext4, linux-kernel, linux-f2fs-devel,
	ecryptfs, linux-arm-kernel

On Tue, May 19, 2015 at 03:15:21PM +0800, Herbert Xu wrote:
> On Tue, May 19, 2015 at 12:13:17AM -0700, Jaegeuk Kim wrote:
> >
> > 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.

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,

> 
> Cheers,
> -- 
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
@ 2015-05-19  7:27                 ` Jaegeuk Kim
  0 siblings, 0 replies; 44+ messages in thread
From: Jaegeuk Kim @ 2015-05-19  7:27 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, May 19, 2015 at 03:15:21PM +0800, Herbert Xu wrote:
> On Tue, May 19, 2015 at 12:13:17AM -0700, Jaegeuk Kim wrote:
> >
> > 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.

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,

> 
> Cheers,
> -- 
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
  2015-05-19  7:27                 ` Jaegeuk Kim
@ 2015-05-19  7:30                   ` Herbert Xu
  -1 siblings, 0 replies; 44+ messages in thread
From: Herbert Xu @ 2015-05-19  7:30 UTC (permalink / raw)
  To: Jaegeuk Kim
  Cc: davem, linux-crypto, linux-ext4, linux-kernel, linux-f2fs-devel,
	ecryptfs, linux-arm-kernel

On Tue, May 19, 2015 at 12:27:40AM -0700, Jaegeuk Kim wrote:
>
> 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?

> 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,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
@ 2015-05-19  7:30                   ` Herbert Xu
  0 siblings, 0 replies; 44+ messages in thread
From: Herbert Xu @ 2015-05-19  7:30 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, May 19, 2015 at 12:27:40AM -0700, Jaegeuk Kim wrote:
>
> 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?

> 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,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
  2015-05-19  7:30                   ` Herbert Xu
@ 2015-05-19  7:59                     ` Jaegeuk Kim
  -1 siblings, 0 replies; 44+ messages in thread
From: Jaegeuk Kim @ 2015-05-19  7:59 UTC (permalink / raw)
  To: Herbert Xu
  Cc: davem, linux-crypto, linux-ext4, linux-kernel, linux-f2fs-devel,
	ecryptfs, linux-arm-kernel

On Tue, May 19, 2015 at 03:30:12PM +0800, Herbert Xu wrote:
> On Tue, May 19, 2015 at 12:27:40AM -0700, Jaegeuk Kim wrote:
> >
> > 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.

> 
> > 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.

> 
> Cheers,
> -- 
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
@ 2015-05-19  7:59                     ` Jaegeuk Kim
  0 siblings, 0 replies; 44+ messages in thread
From: Jaegeuk Kim @ 2015-05-19  7:59 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, May 19, 2015 at 03:30:12PM +0800, Herbert Xu wrote:
> On Tue, May 19, 2015 at 12:27:40AM -0700, Jaegeuk Kim wrote:
> >
> > 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.

> 
> > 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.

> 
> Cheers,
> -- 
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
  2015-05-19  7:15               ` Herbert Xu
@ 2015-05-19 14:14                 ` Theodore Ts'o
  -1 siblings, 0 replies; 44+ messages in thread
From: Theodore Ts'o @ 2015-05-19 14:14 UTC (permalink / raw)
  To: Herbert Xu
  Cc: Jaegeuk Kim, davem, linux-crypto, linux-ext4, linux-kernel,
	linux-f2fs-devel, ecryptfs, linux-arm-kernel

On Tue, May 19, 2015 at 03:15:21PM +0800, Herbert Xu wrote:
> On Tue, May 19, 2015 at 12:13:17AM -0700, Jaegeuk Kim wrote:
> >
> > 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.

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

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
@ 2015-05-19 14:14                 ` Theodore Ts'o
  0 siblings, 0 replies; 44+ messages in thread
From: Theodore Ts'o @ 2015-05-19 14:14 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, May 19, 2015 at 03:15:21PM +0800, Herbert Xu wrote:
> On Tue, May 19, 2015 at 12:13:17AM -0700, Jaegeuk Kim wrote:
> >
> > 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.

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

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
  2015-05-19 14:14                 ` Theodore Ts'o
@ 2015-05-19 14:27                   ` Herbert Xu
  -1 siblings, 0 replies; 44+ messages in thread
From: Herbert Xu @ 2015-05-19 14:27 UTC (permalink / raw)
  To: Theodore Ts'o, Jaegeuk Kim, davem, linux-crypto, linux-ext4,
	linux-kernel, linux-f2fs-devel, ecryptfs, linux-arm-kernel
  Cc: Steffen Klassert

On Tue, May 19, 2015 at 10:14:30AM -0400, Theodore Ts'o wrote:
> 
> 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.

> 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.

> 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,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
@ 2015-05-19 14:27                   ` Herbert Xu
  0 siblings, 0 replies; 44+ messages in thread
From: Herbert Xu @ 2015-05-19 14:27 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, May 19, 2015 at 10:14:30AM -0400, Theodore Ts'o wrote:
> 
> 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.

> 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.

> 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,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
  2015-05-19 14:27                   ` Herbert Xu
@ 2015-05-19 14:39                     ` Theodore Ts'o
  -1 siblings, 0 replies; 44+ messages in thread
From: Theodore Ts'o @ 2015-05-19 14:39 UTC (permalink / raw)
  To: Herbert Xu
  Cc: Jaegeuk Kim, davem, linux-crypto, linux-ext4, linux-kernel,
	linux-f2fs-devel, ecryptfs, linux-arm-kernel, Steffen Klassert,
	mhalcrow

On Tue, May 19, 2015 at 10:27:55PM +0800, Herbert Xu wrote:
> On Tue, May 19, 2015 at 10:14:30AM -0400, Theodore Ts'o wrote:
> > 
> > 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

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
@ 2015-05-19 14:39                     ` Theodore Ts'o
  0 siblings, 0 replies; 44+ messages in thread
From: Theodore Ts'o @ 2015-05-19 14:39 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, May 19, 2015 at 10:27:55PM +0800, Herbert Xu wrote:
> On Tue, May 19, 2015 at 10:14:30AM -0400, Theodore Ts'o wrote:
> > 
> > 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

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
  2015-05-19 14:27                   ` Herbert Xu
  (?)
@ 2015-05-20  7:21                     ` Steffen Klassert
  -1 siblings, 0 replies; 44+ messages in thread
From: Steffen Klassert @ 2015-05-20  7:21 UTC (permalink / raw)
  To: Herbert Xu
  Cc: Theodore Ts'o, Jaegeuk Kim, davem, linux-crypto, linux-ext4,
	linux-kernel, linux-f2fs-devel, ecryptfs, linux-arm-kernel

On Tue, May 19, 2015 at 10:27:55PM +0800, Herbert Xu wrote:
> On Tue, May 19, 2015 at 10:14:30AM -0400, Theodore Ts'o wrote:
> > 
> > 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.
> 
> > 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.

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.

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
@ 2015-05-20  7:21                     ` Steffen Klassert
  0 siblings, 0 replies; 44+ messages in thread
From: Steffen Klassert @ 2015-05-20  7:21 UTC (permalink / raw)
  To: Herbert Xu
  Cc: Theodore Ts'o, Jaegeuk Kim, davem, linux-crypto, linux-ext4,
	linux-kernel, linux-f2fs-devel, ecryptfs, linux-arm-kernel

On Tue, May 19, 2015 at 10:27:55PM +0800, Herbert Xu wrote:
> On Tue, May 19, 2015 at 10:14:30AM -0400, Theodore Ts'o wrote:
> > 
> > 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.
> 
> > 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.

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.

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
@ 2015-05-20  7:21                     ` Steffen Klassert
  0 siblings, 0 replies; 44+ messages in thread
From: Steffen Klassert @ 2015-05-20  7:21 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, May 19, 2015 at 10:27:55PM +0800, Herbert Xu wrote:
> On Tue, May 19, 2015 at 10:14:30AM -0400, Theodore Ts'o wrote:
> > 
> > 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.
> 
> > 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.

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.

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
  2015-05-20  7:21                     ` Steffen Klassert
@ 2015-05-20 14:59                       ` Theodore Ts'o
  -1 siblings, 0 replies; 44+ messages in thread
From: Theodore Ts'o @ 2015-05-20 14:59 UTC (permalink / raw)
  To: Steffen Klassert
  Cc: Herbert Xu, Jaegeuk Kim, davem, linux-crypto, linux-ext4,
	linux-kernel, linux-f2fs-devel, ecryptfs, linux-arm-kernel

On Wed, May 20, 2015 at 09:21:20AM +0200, Steffen Klassert wrote:
> 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

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
@ 2015-05-20 14:59                       ` Theodore Ts'o
  0 siblings, 0 replies; 44+ messages in thread
From: Theodore Ts'o @ 2015-05-20 14:59 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, May 20, 2015 at 09:21:20AM +0200, Steffen Klassert wrote:
> 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

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
  2015-05-20 14:59                       ` Theodore Ts'o
  (?)
@ 2015-05-20 15:30                         ` Ard Biesheuvel
  -1 siblings, 0 replies; 44+ messages in thread
From: Ard Biesheuvel @ 2015-05-20 15:30 UTC (permalink / raw)
  To: Theodore Ts'o, Steffen Klassert, Herbert Xu, Jaegeuk Kim,
	David S. Miller, linux-crypto, linux-ext4, linux-kernel,
	linux-f2fs-devel, ecryptfs, linux-arm-kernel

On 20 May 2015 at 16:59, Theodore Ts'o <tytso@mit.edu> wrote:
> On Wed, May 20, 2015 at 09:21:20AM +0200, Steffen Klassert wrote:
>> 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.
>

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?

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
@ 2015-05-20 15:30                         ` Ard Biesheuvel
  0 siblings, 0 replies; 44+ messages in thread
From: Ard Biesheuvel @ 2015-05-20 15:30 UTC (permalink / raw)
  To: Theodore Ts'o, Steffen Klassert, Herbert Xu, Jaegeuk Kim,
	David S. Miller, linux-crypto, linux-ext4, linux-kernel,
	linux-f2fs-devel, ecryptfs, linux-arm-kernel

On 20 May 2015 at 16:59, Theodore Ts'o <tytso@mit.edu> wrote:
> On Wed, May 20, 2015 at 09:21:20AM +0200, Steffen Klassert wrote:
>> 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.
>

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?

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
@ 2015-05-20 15:30                         ` Ard Biesheuvel
  0 siblings, 0 replies; 44+ messages in thread
From: Ard Biesheuvel @ 2015-05-20 15:30 UTC (permalink / raw)
  To: linux-arm-kernel

On 20 May 2015 at 16:59, Theodore Ts'o <tytso@mit.edu> wrote:
> On Wed, May 20, 2015 at 09:21:20AM +0200, Steffen Klassert wrote:
>> 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.
>

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?

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
  2015-05-20 15:30                         ` Ard Biesheuvel
  (?)
@ 2015-05-20 15:42                           ` Herbert Xu
  -1 siblings, 0 replies; 44+ messages in thread
From: Herbert Xu @ 2015-05-20 15:42 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Theodore Ts'o, Steffen Klassert, Jaegeuk Kim,
	David S. Miller, linux-crypto, linux-ext4, linux-kernel,
	linux-f2fs-devel, ecryptfs, linux-arm-kernel

On Wed, May 20, 2015 at 05:30:14PM +0200, Ard Biesheuvel wrote:
> 
> 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,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
@ 2015-05-20 15:42                           ` Herbert Xu
  0 siblings, 0 replies; 44+ messages in thread
From: Herbert Xu @ 2015-05-20 15:42 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Theodore Ts'o, Steffen Klassert, Jaegeuk Kim,
	David S. Miller, linux-crypto, linux-ext4, linux-kernel,
	linux-f2fs-devel, ecryptfs, linux-arm-kernel

On Wed, May 20, 2015 at 05:30:14PM +0200, Ard Biesheuvel wrote:
> 
> 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,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
@ 2015-05-20 15:42                           ` Herbert Xu
  0 siblings, 0 replies; 44+ messages in thread
From: Herbert Xu @ 2015-05-20 15:42 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, May 20, 2015 at 05:30:14PM +0200, Ard Biesheuvel wrote:
> 
> 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,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 44+ messages in thread

end of thread, other threads:[~2015-05-20 15:42 UTC | newest]

Thread overview: 44+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-19  5:46 [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm Jaegeuk Kim
2015-05-19  5:46 ` Jaegeuk Kim
2015-05-19  5:49 ` Herbert Xu
2015-05-19  5:49   ` Herbert Xu
2015-05-19  6:24   ` Jaegeuk Kim
2015-05-19  6:24     ` Jaegeuk Kim
2015-05-19  6:31     ` [f2fs-dev] [PATCH v2] " Jaegeuk Kim
2015-05-19  6:31       ` Jaegeuk Kim
2015-05-19  6:32       ` Herbert Xu
2015-05-19  6:32         ` [f2fs-dev] " Herbert Xu
2015-05-19  6:32         ` Herbert Xu
2015-05-19  6:32     ` [PATCH] " Herbert Xu
2015-05-19  6:32       ` Herbert Xu
2015-05-19  6:58       ` Jaegeuk Kim
2015-05-19  6:58         ` Jaegeuk Kim
2015-05-19  6:59         ` Herbert Xu
2015-05-19  6:59           ` Herbert Xu
2015-05-19  7:13           ` Jaegeuk Kim
2015-05-19  7:13             ` Jaegeuk Kim
2015-05-19  7:15             ` Herbert Xu
2015-05-19  7:15               ` Herbert Xu
2015-05-19  7:27               ` Jaegeuk Kim
2015-05-19  7:27                 ` Jaegeuk Kim
2015-05-19  7:30                 ` Herbert Xu
2015-05-19  7:30                   ` Herbert Xu
2015-05-19  7:59                   ` Jaegeuk Kim
2015-05-19  7:59                     ` Jaegeuk Kim
2015-05-19 14:14               ` Theodore Ts'o
2015-05-19 14:14                 ` Theodore Ts'o
2015-05-19 14:27                 ` Herbert Xu
2015-05-19 14:27                   ` Herbert Xu
2015-05-19 14:39                   ` Theodore Ts'o
2015-05-19 14:39                     ` Theodore Ts'o
2015-05-20  7:21                   ` Steffen Klassert
2015-05-20  7:21                     ` Steffen Klassert
2015-05-20  7:21                     ` Steffen Klassert
2015-05-20 14:59                     ` Theodore Ts'o
2015-05-20 14:59                       ` Theodore Ts'o
2015-05-20 15:30                       ` Ard Biesheuvel
2015-05-20 15:30                         ` Ard Biesheuvel
2015-05-20 15:30                         ` Ard Biesheuvel
2015-05-20 15:42                         ` Herbert Xu
2015-05-20 15:42                           ` Herbert Xu
2015-05-20 15:42                           ` Herbert Xu

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.