Blob Blame History Raw
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Date: Fri, 21 Feb 2014 17:24:04 +0100
Subject: crypto: Reduce preempt disabled regions, more algos
Git-repo: git://git.kernel.org/pub/scm/linux/kernel/git/rt/linux-rt-devel.git
Git-commit: da77ceac3d20f27310a07a7c346a4ee6b40d6c28
Patch-mainline: Queued in subsystem maintainer repository
References: SLE Realtime Extension

Don Estabrook reported
| kernel: WARNING: CPU: 2 PID: 858 at kernel/sched/core.c:2428 migrate_disable+0xed/0x100()
| kernel: WARNING: CPU: 2 PID: 858 at kernel/sched/core.c:2462 migrate_enable+0x17b/0x200()
| kernel: WARNING: CPU: 3 PID: 865 at kernel/sched/core.c:2428 migrate_disable+0xed/0x100()

and his backtrace showed some crypto functions which looked fine.

The problem is the following sequence:

glue_xts_crypt_128bit()
{
	blkcipher_walk_virt(); /* normal migrate_disable() */

	glue_fpu_begin(); /* get atomic */

	while (nbytes) {
		__glue_xts_crypt_128bit();
		blkcipher_walk_done(); /* with nbytes = 0, migrate_enable()
					* while we are atomic */
	};
	glue_fpu_end() /* no longer atomic */
}

and this is why the counter get out of sync and the warning is printed.
The other problem is that we are non-preemptible between
glue_fpu_begin() and glue_fpu_end() and the latency grows. To fix this,
I shorten the FPU off region and ensure blkcipher_walk_done() is called
with preemption enabled. This might hurt the performance because we now
enable/disable the FPU state more often but we gain lower latency and
the bug is gone.


Reported-by: Don Estabrook <don.estabrook@gmail.com>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Daniel Wagner <dwagner@suse.de>
---
 arch/x86/crypto/cast5_avx_glue.c |   21 +++++++++------------
 arch/x86/crypto/glue_helper.c    |   26 +++++++++++++++-----------
 2 files changed, 24 insertions(+), 23 deletions(-)

--- a/arch/x86/crypto/cast5_avx_glue.c
+++ b/arch/x86/crypto/cast5_avx_glue.c
@@ -46,7 +46,7 @@ static inline void cast5_fpu_end(bool fp
 
 static int ecb_crypt(struct skcipher_request *req, bool enc)
 {
-	bool fpu_enabled = false;
+	bool fpu_enabled;
 	struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
 	struct cast5_ctx *ctx = crypto_skcipher_ctx(tfm);
 	struct skcipher_walk walk;
@@ -61,7 +61,7 @@ static int ecb_crypt(struct skcipher_req
 		u8 *wsrc = walk.src.virt.addr;
 		u8 *wdst = walk.dst.virt.addr;
 
-		fpu_enabled = cast5_fpu_begin(fpu_enabled, &walk, nbytes);
+		fpu_enabled = cast5_fpu_begin(false, &walk, nbytes);
 
 		/* Process multi-block batch */
 		if (nbytes >= bsize * CAST5_PARALLEL_BLOCKS) {
@@ -90,10 +90,9 @@ static int ecb_crypt(struct skcipher_req
 		} while (nbytes >= bsize);
 
 done:
+		cast5_fpu_end(fpu_enabled);
 		err = skcipher_walk_done(&walk, nbytes);
 	}
-
-	cast5_fpu_end(fpu_enabled);
 	return err;
 }
 
@@ -197,7 +196,7 @@ static int cbc_decrypt(struct skcipher_r
 {
 	struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
 	struct cast5_ctx *ctx = crypto_skcipher_ctx(tfm);
-	bool fpu_enabled = false;
+	bool fpu_enabled;
 	struct skcipher_walk walk;
 	unsigned int nbytes;
 	int err;
@@ -205,12 +204,11 @@ static int cbc_decrypt(struct skcipher_r
 	err = skcipher_walk_virt(&walk, req, false);
 
 	while ((nbytes = walk.nbytes)) {
-		fpu_enabled = cast5_fpu_begin(fpu_enabled, &walk, nbytes);
+		fpu_enabled = cast5_fpu_begin(false, &walk, nbytes);
 		nbytes = __cbc_decrypt(ctx, &walk);
+		cast5_fpu_end(fpu_enabled);
 		err = skcipher_walk_done(&walk, nbytes);
 	}
-
-	cast5_fpu_end(fpu_enabled);
 	return err;
 }
 
@@ -277,7 +275,7 @@ static int ctr_crypt(struct skcipher_req
 {
 	struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
 	struct cast5_ctx *ctx = crypto_skcipher_ctx(tfm);
-	bool fpu_enabled = false;
+	bool fpu_enabled;
 	struct skcipher_walk walk;
 	unsigned int nbytes;
 	int err;
@@ -285,13 +283,12 @@ static int ctr_crypt(struct skcipher_req
 	err = skcipher_walk_virt(&walk, req, false);
 
 	while ((nbytes = walk.nbytes) >= CAST5_BLOCK_SIZE) {
-		fpu_enabled = cast5_fpu_begin(fpu_enabled, &walk, nbytes);
+		fpu_enabled = cast5_fpu_begin(false, &walk, nbytes);
 		nbytes = __ctr_crypt(&walk, ctx);
+		cast5_fpu_end(fpu_enabled);
 		err = skcipher_walk_done(&walk, nbytes);
 	}
 
-	cast5_fpu_end(fpu_enabled);
-
 	if (walk.nbytes) {
 		ctr_crypt_final(&walk, ctx);
 		err = skcipher_walk_done(&walk, 0);
--- a/arch/x86/crypto/glue_helper.c
+++ b/arch/x86/crypto/glue_helper.c
@@ -24,7 +24,7 @@ int glue_ecb_req_128bit(const struct com
 	void *ctx = crypto_skcipher_ctx(crypto_skcipher_reqtfm(req));
 	const unsigned int bsize = 128 / 8;
 	struct skcipher_walk walk;
-	bool fpu_enabled = false;
+	bool fpu_enabled;
 	unsigned int nbytes;
 	int err;
 
@@ -37,7 +37,7 @@ int glue_ecb_req_128bit(const struct com
 		unsigned int i;
 
 		fpu_enabled = glue_fpu_begin(bsize, gctx->fpu_blocks_limit,
-					     &walk, fpu_enabled, nbytes);
+					     &walk, false, nbytes);
 		for (i = 0; i < gctx->num_funcs; i++) {
 			func_bytes = bsize * gctx->funcs[i].num_blocks;
 
@@ -55,10 +55,9 @@ int glue_ecb_req_128bit(const struct com
 			if (nbytes < bsize)
 				break;
 		}
+		glue_fpu_end(fpu_enabled);
 		err = skcipher_walk_done(&walk, nbytes);
 	}
-
-	glue_fpu_end(fpu_enabled);
 	return err;
 }
 EXPORT_SYMBOL_GPL(glue_ecb_req_128bit);
@@ -101,7 +100,7 @@ int glue_cbc_decrypt_req_128bit(const st
 	void *ctx = crypto_skcipher_ctx(crypto_skcipher_reqtfm(req));
 	const unsigned int bsize = 128 / 8;
 	struct skcipher_walk walk;
-	bool fpu_enabled = false;
+	bool fpu_enabled;
 	unsigned int nbytes;
 	int err;
 
@@ -115,7 +114,7 @@ int glue_cbc_decrypt_req_128bit(const st
 		u128 last_iv;
 
 		fpu_enabled = glue_fpu_begin(bsize, gctx->fpu_blocks_limit,
-					     &walk, fpu_enabled, nbytes);
+					     &walk, false, nbytes);
 		/* Start of the last block. */
 		src += nbytes / bsize - 1;
 		dst += nbytes / bsize - 1;
@@ -147,10 +146,10 @@ int glue_cbc_decrypt_req_128bit(const st
 done:
 		u128_xor(dst, dst, (u128 *)walk.iv);
 		*(u128 *)walk.iv = last_iv;
+		glue_fpu_end(fpu_enabled);
 		err = skcipher_walk_done(&walk, nbytes);
 	}
 
-	glue_fpu_end(fpu_enabled);
 	return err;
 }
 EXPORT_SYMBOL_GPL(glue_cbc_decrypt_req_128bit);
@@ -161,7 +160,7 @@ int glue_ctr_req_128bit(const struct com
 	void *ctx = crypto_skcipher_ctx(crypto_skcipher_reqtfm(req));
 	const unsigned int bsize = 128 / 8;
 	struct skcipher_walk walk;
-	bool fpu_enabled = false;
+	bool fpu_enabled;
 	unsigned int nbytes;
 	int err;
 
@@ -175,7 +174,7 @@ int glue_ctr_req_128bit(const struct com
 		le128 ctrblk;
 
 		fpu_enabled = glue_fpu_begin(bsize, gctx->fpu_blocks_limit,
-					     &walk, fpu_enabled, nbytes);
+					     &walk, false, nbytes);
 
 		be128_to_le128(&ctrblk, (be128 *)walk.iv);
 
@@ -199,11 +198,10 @@ int glue_ctr_req_128bit(const struct com
 		}
 
 		le128_to_be128((be128 *)walk.iv, &ctrblk);
+		glue_fpu_end(fpu_enabled);
 		err = skcipher_walk_done(&walk, nbytes);
 	}
 
-	glue_fpu_end(fpu_enabled);
-
 	if (nbytes) {
 		le128 ctrblk;
 		u128 tmp;
@@ -301,8 +299,14 @@ int glue_xts_req_128bit(const struct com
 	tweak_fn(tweak_ctx, walk.iv, walk.iv);
 
 	while (nbytes) {
+		fpu_enabled = glue_fpu_begin(bsize, gctx->fpu_blocks_limit,
+					     &walk, fpu_enabled,
+					     nbytes < bsize ? bsize : nbytes);
 		nbytes = __glue_xts_req_128bit(gctx, crypt_ctx, &walk);
 
+		glue_fpu_end(fpu_enabled);
+		fpu_enabled = false;
+
 		err = skcipher_walk_done(&walk, nbytes);
 		nbytes = walk.nbytes;
 	}