| Message ID | 1432014416-39326-1-git-send-email-jaegeuk@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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,
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/
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,
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
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.
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
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,
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
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,
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
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
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,
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
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.
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
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?
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,
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)
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(-)