Removed
Link Here
|
1 |
From 3c53a1601b6f861f8b7f0cd0984b18e78291fe85 Mon Sep 17 00:00:00 2001 |
2 |
From: Victor Julien <victor@inliniac.net> |
3 |
Date: Wed, 18 Aug 2021 20:14:48 +0200 |
4 |
Subject: [PATCH] threading: don't pass locked flow between threads |
5 |
|
6 |
Previously the flow manager would share evicted flows with the workers |
7 |
while keeping the flows mutex locked. This reduced the number of unlock/ |
8 |
lock cycles while there was guaranteed to be no contention. |
9 |
|
10 |
This turns out to be undefined behavior. A lock is supposed to be locked |
11 |
and unlocked from the same thread. It appears that FreeBSD is stricter on |
12 |
this than Linux. |
13 |
|
14 |
This patch addresses the issue by unlocking before handing a flow off |
15 |
to another thread, and locking again from the new thread. |
16 |
|
17 |
Issue was reported and largely analyzed by Bill Meeks. |
18 |
|
19 |
Bug: #4478 |
20 |
(cherry picked from commit 9551cd05357925e8bec8e0030d5f98fd07f17839) |
21 |
--- |
22 |
src/flow-hash.c | 1 + |
23 |
src/flow-manager.c | 2 +- |
24 |
src/flow-timeout.c | 1 + |
25 |
src/flow-worker.c | 1 + |
26 |
4 files changed, 4 insertions(+), 1 deletion(-) |
27 |
|
28 |
diff --git a/src/flow-hash.c b/src/flow-hash.c |
29 |
index ebbd836e81a..760bc53e0a8 100644 |
30 |
--- src/flow-hash.c |
31 |
+++ src/flow-hash.c |
32 |
@@ -669,6 +669,7 @@ static inline void MoveToWorkQueue(ThreadVars *tv, FlowLookupStruct *fls, |
33 |
f->fb = NULL; |
34 |
f->next = NULL; |
35 |
FlowQueuePrivateAppendFlow(&fls->work_queue, f); |
36 |
+ FLOWLOCK_UNLOCK(f); |
37 |
} else { |
38 |
/* implied: TCP but our thread does not own it. So set it |
39 |
* aside for the Flow Manager to pick it up. */ |
40 |
diff --git a/src/flow-manager.c b/src/flow-manager.c |
41 |
index d58a49637d6..9228c88490c 100644 |
42 |
--- src/flow-manager.c |
43 |
+++ src/flow-manager.c |
44 |
@@ -333,9 +333,9 @@ static uint32_t ProcessAsideQueue(FlowManagerTimeoutThread *td, FlowTimeoutCount |
45 |
FlowForceReassemblyNeedReassembly(f) == 1) |
46 |
{ |
47 |
FlowForceReassemblyForFlow(f); |
48 |
+ FLOWLOCK_UNLOCK(f); |
49 |
/* flow ownership is passed to the worker thread */ |
50 |
|
51 |
- /* flow remains locked */ |
52 |
counters->flows_aside_needs_work++; |
53 |
continue; |
54 |
} |
55 |
diff --git a/src/flow-timeout.c b/src/flow-timeout.c |
56 |
index 972b35076bd..d6cca490087 100644 |
57 |
--- src/flow-timeout.c |
58 |
+++ src/flow-timeout.c |
59 |
@@ -401,6 +401,7 @@ static inline void FlowForceReassemblyForHash(void) |
60 |
RemoveFromHash(f, prev_f); |
61 |
f->flow_end_flags |= FLOW_END_FLAG_SHUTDOWN; |
62 |
FlowForceReassemblyForFlow(f); |
63 |
+ FLOWLOCK_UNLOCK(f); |
64 |
f = next_f; |
65 |
continue; |
66 |
} |
67 |
diff --git a/src/flow-worker.c b/src/flow-worker.c |
68 |
index 69dbb6ac575..dccf3581dd5 100644 |
69 |
--- src/flow-worker.c |
70 |
+++ src/flow-worker.c |
71 |
@@ -168,6 +168,7 @@ static void CheckWorkQueue(ThreadVars *tv, FlowWorkerThreadData *fw, |
72 |
{ |
73 |
Flow *f; |
74 |
while ((f = FlowQueuePrivateGetFromTop(fq)) != NULL) { |
75 |
+ FLOWLOCK_WRLOCK(f); |
76 |
f->flow_end_flags |= FLOW_END_FLAG_TIMEOUT; //TODO emerg |
77 |
|
78 |
const FlowStateType state = f->flow_state; |