crypto: ccp - Use a single queue for proper ordering of tfm requests

Move to a single queue to serialize requests within a tfm. When testing using IPSec with a large number of network connections the per cpu tfm queuing logic was not working properly. Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

crypto: ccp - Use a single queue for proper ordering of tfm requests
Move to a single queue to serialize requests within a tfm. When testing using IPSec with a large number of network connections the per cpu tfm queuing logic was not working properly. Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Tom Lendacky · Herbert Xu
1 parent c11baa02c5
Showing 1 changed file with 48 additions and 116 deletions Side-by-side Diff
drivers/crypto/ccp/ccp-crypto-main.c
@@ -38,23 +38,20 @@
 static LIST_HEAD(hash_algs);
 static LIST_HEAD(cipher_algs);
  
-/* For any tfm, requests for that tfm on the same CPU must be returned
- * in the order received.  With multiple queues available, the CCP can
- * process more than one cmd at a time.  Therefore we must maintain
- * a cmd list to insure the proper ordering of requests on a given tfm/cpu
- * combination.
+/* For any tfm, requests for that tfm must be returned on the order
+ * received.  With multiple queues available, the CCP can process more
+ * than one cmd at a time.  Therefore we must maintain a cmd list to insure
+ * the proper ordering of requests on a given tfm.
  */
-struct ccp_crypto_cpu_queue {
+struct ccp_crypto_queue {
 	struct list_head cmds;
 	struct list_head *backlog;
 	unsigned int cmd_count;
 };
-#define CCP_CRYPTO_MAX_QLEN	50
+#define CCP_CRYPTO_MAX_QLEN	100
  
-struct ccp_crypto_percpu_queue {
-	struct ccp_crypto_cpu_queue __percpu *cpu_queue;
-};
-static struct ccp_crypto_percpu_queue req_queue;
+static struct ccp_crypto_queue req_queue;
+static spinlock_t req_queue_lock;
  
 struct ccp_crypto_cmd {
 	struct list_head entry;
@@ -71,8 +68,6 @@
  
 	/* Used for held command processing to determine state */
 	int ret;
-
-	int cpu;
 };
  
 struct ccp_crypto_cpu {
  
  
  
  
@@ -91,25 +86,21 @@
 	return true;
 }
  
-/*
- * ccp_crypto_cmd_complete must be called while running on the appropriate
- * cpu and the caller must have done a get_cpu to disable preemption
- */
 static struct ccp_crypto_cmd *ccp_crypto_cmd_complete(
 	struct ccp_crypto_cmd *crypto_cmd, struct ccp_crypto_cmd **backlog)
 {
-	struct ccp_crypto_cpu_queue *cpu_queue;
 	struct ccp_crypto_cmd *held = NULL, *tmp;
+	unsigned long flags;
  
 	*backlog = NULL;
  
-	cpu_queue = this_cpu_ptr(req_queue.cpu_queue);
+	spin_lock_irqsave(&req_queue_lock, flags);
  
 	/* Held cmds will be after the current cmd in the queue so start
 	 * searching for a cmd with a matching tfm for submission.
 	 */
 	tmp = crypto_cmd;
-	list_for_each_entry_continue(tmp, &cpu_queue->cmds, entry) {
+	list_for_each_entry_continue(tmp, &req_queue.cmds, entry) {
 		if (crypto_cmd->tfm != tmp->tfm)
 			continue;
 		held = tmp;
  
  
  
  
  
  
  
  
  
  
  
@@ -120,47 +111,45 @@
 	 *   Because cmds can be executed from any point in the cmd list
 	 *   special precautions have to be taken when handling the backlog.
 	 */
-	if (cpu_queue->backlog != &cpu_queue->cmds) {
+	if (req_queue.backlog != &req_queue.cmds) {
 		/* Skip over this cmd if it is the next backlog cmd */
-		if (cpu_queue->backlog == &crypto_cmd->entry)
-			cpu_queue->backlog = crypto_cmd->entry.next;
+		if (req_queue.backlog == &crypto_cmd->entry)
+			req_queue.backlog = crypto_cmd->entry.next;
  
-		*backlog = container_of(cpu_queue->backlog,
+		*backlog = container_of(req_queue.backlog,
 					struct ccp_crypto_cmd, entry);
-		cpu_queue->backlog = cpu_queue->backlog->next;
+		req_queue.backlog = req_queue.backlog->next;
  
 		/* Skip over this cmd if it is now the next backlog cmd */
-		if (cpu_queue->backlog == &crypto_cmd->entry)
-			cpu_queue->backlog = crypto_cmd->entry.next;
+		if (req_queue.backlog == &crypto_cmd->entry)
+			req_queue.backlog = crypto_cmd->entry.next;
 	}
  
 	/* Remove the cmd entry from the list of cmds */
-	cpu_queue->cmd_count--;
+	req_queue.cmd_count--;
 	list_del(&crypto_cmd->entry);
  
+	spin_unlock_irqrestore(&req_queue_lock, flags);
+
 	return held;
 }
  
-static void ccp_crypto_complete_on_cpu(struct work_struct *work)
+static void ccp_crypto_complete(void *data, int err)
 {
-	struct ccp_crypto_cpu *cpu_work =
-		container_of(work, struct ccp_crypto_cpu, work);
-	struct ccp_crypto_cmd *crypto_cmd = cpu_work->crypto_cmd;
+	struct ccp_crypto_cmd *crypto_cmd = data;
 	struct ccp_crypto_cmd *held, *next, *backlog;
 	struct crypto_async_request *req = crypto_cmd->req;
 	struct ccp_ctx *ctx = crypto_tfm_ctx(req->tfm);
-	int cpu, ret;
+	int ret;
  
-	cpu = get_cpu();
-
-	if (cpu_work->err == -EINPROGRESS) {
+	if (err == -EINPROGRESS) {
 		/* Only propogate the -EINPROGRESS if necessary */
 		if (crypto_cmd->ret == -EBUSY) {
 			crypto_cmd->ret = -EINPROGRESS;
 			req->complete(req, -EINPROGRESS);
 		}
  
-		goto e_cpu;
+		return;
 	}
  
 	/* Operation has completed - update the queue before invoking
@@ -178,7 +167,7 @@
 		req->complete(req, -EINPROGRESS);
  
 	/* Completion callbacks */
-	ret = cpu_work->err;
+	ret = err;
 	if (ctx->complete)
 		ret = ctx->complete(req, ret);
 	req->complete(req, ret);
  
  
  
  
  
  
  
  
  
@@ -203,52 +192,28 @@
 	}
  
 	kfree(crypto_cmd);
-
-e_cpu:
-	put_cpu();
-
-	complete(&cpu_work->completion);
 }
  
-static void ccp_crypto_complete(void *data, int err)
-{
-	struct ccp_crypto_cmd *crypto_cmd = data;
-	struct ccp_crypto_cpu cpu_work;
-
-	INIT_WORK(&cpu_work.work, ccp_crypto_complete_on_cpu);
-	init_completion(&cpu_work.completion);
-	cpu_work.crypto_cmd = crypto_cmd;
-	cpu_work.err = err;
-
-	schedule_work_on(crypto_cmd->cpu, &cpu_work.work);
-
-	/* Keep the completion call synchronous */
-	wait_for_completion(&cpu_work.completion);
-}
-
 static int ccp_crypto_enqueue_cmd(struct ccp_crypto_cmd *crypto_cmd)
 {
-	struct ccp_crypto_cpu_queue *cpu_queue;
 	struct ccp_crypto_cmd *active = NULL, *tmp;
-	int cpu, ret;
+	unsigned long flags;
+	int ret;
  
-	cpu = get_cpu();
-	crypto_cmd->cpu = cpu;
+	spin_lock_irqsave(&req_queue_lock, flags);
  
-	cpu_queue = this_cpu_ptr(req_queue.cpu_queue);
-
 	/* Check if the cmd can/should be queued */
-	if (cpu_queue->cmd_count >= CCP_CRYPTO_MAX_QLEN) {
+	if (req_queue.cmd_count >= CCP_CRYPTO_MAX_QLEN) {
 		ret = -EBUSY;
 		if (!(crypto_cmd->cmd->flags & CCP_CMD_MAY_BACKLOG))
-			goto e_cpu;
+			goto e_lock;
 	}
  
 	/* Look for an entry with the same tfm.  If there is a cmd
-	 * with the same tfm in the list for this cpu then the current
-	 * cmd cannot be submitted to the CCP yet.
+	 * with the same tfm in the list then the current cmd cannot
+	 * be submitted to the CCP yet.
 	 */
-	list_for_each_entry(tmp, &cpu_queue->cmds, entry) {
+	list_for_each_entry(tmp, &req_queue.cmds, entry) {
 		if (crypto_cmd->tfm != tmp->tfm)
 			continue;
 		active = tmp;
  
  
  
  
@@ -259,21 +224,21 @@
 	if (!active) {
 		ret = ccp_enqueue_cmd(crypto_cmd->cmd);
 		if (!ccp_crypto_success(ret))
-			goto e_cpu;
+			goto e_lock;
 	}
  
-	if (cpu_queue->cmd_count >= CCP_CRYPTO_MAX_QLEN) {
+	if (req_queue.cmd_count >= CCP_CRYPTO_MAX_QLEN) {
 		ret = -EBUSY;
-		if (cpu_queue->backlog == &cpu_queue->cmds)
-			cpu_queue->backlog = &crypto_cmd->entry;
+		if (req_queue.backlog == &req_queue.cmds)
+			req_queue.backlog = &crypto_cmd->entry;
 	}
 	crypto_cmd->ret = ret;
  
-	cpu_queue->cmd_count++;
-	list_add_tail(&crypto_cmd->entry, &cpu_queue->cmds);
+	req_queue.cmd_count++;
+	list_add_tail(&crypto_cmd->entry, &req_queue.cmds);
  
-e_cpu:
-	put_cpu();
+e_lock:
+	spin_unlock_irqrestore(&req_queue_lock, flags);
  
 	return ret;
 }
  
  
  
@@ -387,50 +352,18 @@
 	}
 }
  
-static int ccp_init_queues(void)
-{
-	struct ccp_crypto_cpu_queue *cpu_queue;
-	int cpu;
-
-	req_queue.cpu_queue = alloc_percpu(struct ccp_crypto_cpu_queue);
-	if (!req_queue.cpu_queue)
-		return -ENOMEM;
-
-	for_each_possible_cpu(cpu) {
-		cpu_queue = per_cpu_ptr(req_queue.cpu_queue, cpu);
-		INIT_LIST_HEAD(&cpu_queue->cmds);
-		cpu_queue->backlog = &cpu_queue->cmds;
-		cpu_queue->cmd_count = 0;
-	}
-
-	return 0;
-}
-
-static void ccp_fini_queue(void)
-{
-	struct ccp_crypto_cpu_queue *cpu_queue;
-	int cpu;
-
-	for_each_possible_cpu(cpu) {
-		cpu_queue = per_cpu_ptr(req_queue.cpu_queue, cpu);
-		BUG_ON(!list_empty(&cpu_queue->cmds));
-	}
-	free_percpu(req_queue.cpu_queue);
-}
-
 static int ccp_crypto_init(void)
 {
 	int ret;
  
-	ret = ccp_init_queues();
-	if (ret)
-		return ret;
+	spin_lock_init(&req_queue_lock);
+	INIT_LIST_HEAD(&req_queue.cmds);
+	req_queue.backlog = &req_queue.cmds;
+	req_queue.cmd_count = 0;
  
 	ret = ccp_register_algs();
-	if (ret) {
+	if (ret)
 		ccp_unregister_algs();
-		ccp_fini_queue();
-	}
  
 	return ret;
 }
@@ -438,7 +371,6 @@
 static void ccp_crypto_exit(void)
 {
 	ccp_unregister_algs();
-	ccp_fini_queue();
 }
  
 module_init(ccp_crypto_init);
...	...	@@ -38,23 +38,20 @@
38	38	static LIST_HEAD(hash_algs);
39	39	static LIST_HEAD(cipher_algs);
40	40
41		-/* For any tfm, requests for that tfm on the same CPU must be returned
42		- * in the order received. With multiple queues available, the CCP can
43		- * process more than one cmd at a time. Therefore we must maintain
44		- * a cmd list to insure the proper ordering of requests on a given tfm/cpu
45		- * combination.
	41	+/* For any tfm, requests for that tfm must be returned on the order
	42	+ * received. With multiple queues available, the CCP can process more
	43	+ * than one cmd at a time. Therefore we must maintain a cmd list to insure
	44	+ * the proper ordering of requests on a given tfm.
46	45	*/
47		-struct ccp_crypto_cpu_queue {
	46	+struct ccp_crypto_queue {
48	47	struct list_head cmds;
49	48	struct list_head *backlog;
50	49	unsigned int cmd_count;
51	50	};
52		-#define CCP_CRYPTO_MAX_QLEN 50
	51	+#define CCP_CRYPTO_MAX_QLEN 100
53	52
54		-struct ccp_crypto_percpu_queue {
55		- struct ccp_crypto_cpu_queue __percpu *cpu_queue;
56		-};
57		-static struct ccp_crypto_percpu_queue req_queue;
	53	+static struct ccp_crypto_queue req_queue;
	54	+static spinlock_t req_queue_lock;
58	55
59	56	struct ccp_crypto_cmd {
60	57	struct list_head entry;
...	...	@@ -71,8 +68,6 @@
71	68
72	69	/* Used for held command processing to determine state */
73	70	int ret;
74		-
75		- int cpu;
76	71	};
77	72
78	73	struct ccp_crypto_cpu {
79	74
80	75
81	76
82	77
...	...	@@ -91,25 +86,21 @@
91	86	return true;
92	87	}
93	88
94		-/*
95		- * ccp_crypto_cmd_complete must be called while running on the appropriate
96		- * cpu and the caller must have done a get_cpu to disable preemption
97		- */
98	89	static struct ccp_crypto_cmd *ccp_crypto_cmd_complete(
99	90	struct ccp_crypto_cmd crypto_cmd, struct ccp_crypto_cmd *backlog)
100	91	{
101		- struct ccp_crypto_cpu_queue *cpu_queue;
102	92	struct ccp_crypto_cmd held = NULL, tmp;
	93	+ unsigned long flags;
103	94
104	95	*backlog = NULL;
105	96
106		- cpu_queue = this_cpu_ptr(req_queue.cpu_queue);
	97	+ spin_lock_irqsave(&req_queue_lock, flags);
107	98
108	99	/* Held cmds will be after the current cmd in the queue so start
109	100	* searching for a cmd with a matching tfm for submission.
110	101	*/
111	102	tmp = crypto_cmd;
112		- list_for_each_entry_continue(tmp, &cpu_queue->cmds, entry) {
	103	+ list_for_each_entry_continue(tmp, &req_queue.cmds, entry) {
113	104	if (crypto_cmd->tfm != tmp->tfm)
114	105	continue;
115	106	held = tmp;
116	107
117	108
118	109
119	110
120	111
121	112
122	113
123	114
124	115
125	116
126	117
...	...	@@ -120,47 +111,45 @@
120	111	* Because cmds can be executed from any point in the cmd list
121	112	* special precautions have to be taken when handling the backlog.
122	113	*/
123		- if (cpu_queue->backlog != &cpu_queue->cmds) {
	114	+ if (req_queue.backlog != &req_queue.cmds) {
124	115	/* Skip over this cmd if it is the next backlog cmd */
125		- if (cpu_queue->backlog == &crypto_cmd->entry)
126		- cpu_queue->backlog = crypto_cmd->entry.next;
	116	+ if (req_queue.backlog == &crypto_cmd->entry)
	117	+ req_queue.backlog = crypto_cmd->entry.next;
127	118
128		- *backlog = container_of(cpu_queue->backlog,
	119	+ *backlog = container_of(req_queue.backlog,
129	120	struct ccp_crypto_cmd, entry);
130		- cpu_queue->backlog = cpu_queue->backlog->next;
	121	+ req_queue.backlog = req_queue.backlog->next;
131	122
132	123	/* Skip over this cmd if it is now the next backlog cmd */
133		- if (cpu_queue->backlog == &crypto_cmd->entry)
134		- cpu_queue->backlog = crypto_cmd->entry.next;
	124	+ if (req_queue.backlog == &crypto_cmd->entry)
	125	+ req_queue.backlog = crypto_cmd->entry.next;
135	126	}
136	127
137	128	/* Remove the cmd entry from the list of cmds */
138		- cpu_queue->cmd_count--;
	129	+ req_queue.cmd_count--;
139	130	list_del(&crypto_cmd->entry);
140	131
	132	+ spin_unlock_irqrestore(&req_queue_lock, flags);
	133	+
141	134	return held;
142	135	}
143	136
144		-static void ccp_crypto_complete_on_cpu(struct work_struct *work)
	137	+static void ccp_crypto_complete(void *data, int err)
145	138	{
146		- struct ccp_crypto_cpu *cpu_work =
147		- container_of(work, struct ccp_crypto_cpu, work);
148		- struct ccp_crypto_cmd *crypto_cmd = cpu_work->crypto_cmd;
	139	+ struct ccp_crypto_cmd *crypto_cmd = data;
149	140	struct ccp_crypto_cmd held, next, *backlog;
150	141	struct crypto_async_request *req = crypto_cmd->req;
151	142	struct ccp_ctx *ctx = crypto_tfm_ctx(req->tfm);
152		- int cpu, ret;
	143	+ int ret;
153	144
154		- cpu = get_cpu();
155		-
156		- if (cpu_work->err == -EINPROGRESS) {
	145	+ if (err == -EINPROGRESS) {
157	146	/* Only propogate the -EINPROGRESS if necessary */
158	147	if (crypto_cmd->ret == -EBUSY) {
159	148	crypto_cmd->ret = -EINPROGRESS;
160	149	req->complete(req, -EINPROGRESS);
161	150	}
162	151
163		- goto e_cpu;
	152	+ return;
164	153	}
165	154
166	155	/* Operation has completed - update the queue before invoking
...	...	@@ -178,7 +167,7 @@
178	167	req->complete(req, -EINPROGRESS);
179	168
180	169	/* Completion callbacks */
181		- ret = cpu_work->err;
	170	+ ret = err;
182	171	if (ctx->complete)
183	172	ret = ctx->complete(req, ret);
184	173	req->complete(req, ret);
185	174
186	175
187	176
188	177
189	178
190	179
191	180
192	181
193	182
...	...	@@ -203,52 +192,28 @@
203	192	}
204	193
205	194	kfree(crypto_cmd);
206		-
207		-e_cpu:
208		- put_cpu();
209		-
210		- complete(&cpu_work->completion);
211	195	}
212	196
213		-static void ccp_crypto_complete(void *data, int err)
214		-{
215		- struct ccp_crypto_cmd *crypto_cmd = data;
216		- struct ccp_crypto_cpu cpu_work;
217		-
218		- INIT_WORK(&cpu_work.work, ccp_crypto_complete_on_cpu);
219		- init_completion(&cpu_work.completion);
220		- cpu_work.crypto_cmd = crypto_cmd;
221		- cpu_work.err = err;
222		-
223		- schedule_work_on(crypto_cmd->cpu, &cpu_work.work);
224		-
225		- /* Keep the completion call synchronous */
226		- wait_for_completion(&cpu_work.completion);
227		-}
228		-
229	197	static int ccp_crypto_enqueue_cmd(struct ccp_crypto_cmd *crypto_cmd)
230	198	{
231		- struct ccp_crypto_cpu_queue *cpu_queue;
232	199	struct ccp_crypto_cmd active = NULL, tmp;
233		- int cpu, ret;
	200	+ unsigned long flags;
	201	+ int ret;
234	202
235		- cpu = get_cpu();
236		- crypto_cmd->cpu = cpu;
	203	+ spin_lock_irqsave(&req_queue_lock, flags);
237	204
238		- cpu_queue = this_cpu_ptr(req_queue.cpu_queue);
239		-
240	205	/* Check if the cmd can/should be queued */
241		- if (cpu_queue->cmd_count >= CCP_CRYPTO_MAX_QLEN) {
	206	+ if (req_queue.cmd_count >= CCP_CRYPTO_MAX_QLEN) {
242	207	ret = -EBUSY;
243	208	if (!(crypto_cmd->cmd->flags & CCP_CMD_MAY_BACKLOG))
244		- goto e_cpu;
	209	+ goto e_lock;
245	210	}
246	211
247	212	/* Look for an entry with the same tfm. If there is a cmd
248		- * with the same tfm in the list for this cpu then the current
249		- * cmd cannot be submitted to the CCP yet.
	213	+ * with the same tfm in the list then the current cmd cannot
	214	+ * be submitted to the CCP yet.
250	215	*/
251		- list_for_each_entry(tmp, &cpu_queue->cmds, entry) {
	216	+ list_for_each_entry(tmp, &req_queue.cmds, entry) {
252	217	if (crypto_cmd->tfm != tmp->tfm)
253	218	continue;
254	219	active = tmp;
255	220
256	221
257	222
258	223
...	...	@@ -259,21 +224,21 @@
259	224	if (!active) {
260	225	ret = ccp_enqueue_cmd(crypto_cmd->cmd);
261	226	if (!ccp_crypto_success(ret))
262		- goto e_cpu;
	227	+ goto e_lock;
263	228	}
264	229
265		- if (cpu_queue->cmd_count >= CCP_CRYPTO_MAX_QLEN) {
	230	+ if (req_queue.cmd_count >= CCP_CRYPTO_MAX_QLEN) {
266	231	ret = -EBUSY;
267		- if (cpu_queue->backlog == &cpu_queue->cmds)
268		- cpu_queue->backlog = &crypto_cmd->entry;
	232	+ if (req_queue.backlog == &req_queue.cmds)
	233	+ req_queue.backlog = &crypto_cmd->entry;
269	234	}
270	235	crypto_cmd->ret = ret;
271	236
272		- cpu_queue->cmd_count++;
273		- list_add_tail(&crypto_cmd->entry, &cpu_queue->cmds);
	237	+ req_queue.cmd_count++;
	238	+ list_add_tail(&crypto_cmd->entry, &req_queue.cmds);
274	239
275		-e_cpu:
276		- put_cpu();
	240	+e_lock:
	241	+ spin_unlock_irqrestore(&req_queue_lock, flags);
277	242
278	243	return ret;
279	244	}
280	245
281	246
282	247
...	...	@@ -387,50 +352,18 @@
387	352	}
388	353	}
389	354
390		-static int ccp_init_queues(void)
391		-{
392		- struct ccp_crypto_cpu_queue *cpu_queue;
393		- int cpu;
394		-
395		- req_queue.cpu_queue = alloc_percpu(struct ccp_crypto_cpu_queue);
396		- if (!req_queue.cpu_queue)
397		- return -ENOMEM;
398		-
399		- for_each_possible_cpu(cpu) {
400		- cpu_queue = per_cpu_ptr(req_queue.cpu_queue, cpu);
401		- INIT_LIST_HEAD(&cpu_queue->cmds);
402		- cpu_queue->backlog = &cpu_queue->cmds;
403		- cpu_queue->cmd_count = 0;
404		- }
405		-
406		- return 0;
407		-}
408		-
409		-static void ccp_fini_queue(void)
410		-{
411		- struct ccp_crypto_cpu_queue *cpu_queue;
412		- int cpu;
413		-
414		- for_each_possible_cpu(cpu) {
415		- cpu_queue = per_cpu_ptr(req_queue.cpu_queue, cpu);
416		- BUG_ON(!list_empty(&cpu_queue->cmds));
417		- }
418		- free_percpu(req_queue.cpu_queue);
419		-}
420		-
421	355	static int ccp_crypto_init(void)
422	356	{
423	357	int ret;
424	358
425		- ret = ccp_init_queues();
426		- if (ret)
427		- return ret;
	359	+ spin_lock_init(&req_queue_lock);
	360	+ INIT_LIST_HEAD(&req_queue.cmds);
	361	+ req_queue.backlog = &req_queue.cmds;
	362	+ req_queue.cmd_count = 0;
428	363
429	364	ret = ccp_register_algs();
430		- if (ret) {
	365	+ if (ret)
431	366	ccp_unregister_algs();
432		- ccp_fini_queue();
433		- }
434	367
435	368	return ret;
436	369	}
...	...	@@ -438,7 +371,6 @@
438	371	static void ccp_crypto_exit(void)
439	372	{
440	373	ccp_unregister_algs();
441		- ccp_fini_queue();
442	374	}
443	375
444	376	module_init(ccp_crypto_init);