diff options
author | Ying Xue <ying.xue@windriver.com> | 2014-04-28 06:00:10 -0400 |
---|---|---|
committer | David S. Miller <davem@davemloft.net> | 2014-04-28 14:49:54 -0400 |
commit | eab8c045732635e3833a5d58b17c6da08ff71f9e (patch) | |
tree | 8bd9c3e5aaee575abef950d992e92511bdc5f2b8 /net/tipc | |
parent | d7bb74c38cb3de40600dcbba50a4f84df290dc91 (diff) |
tipc: move the delivery of named messages out of nametbl lock
Commit a89778d8baf19cd7e728d81121a294a06cedaad1 ("tipc: add support
for link state subscriptions") introduced below possible deadlock
scenario:
CPU0 CPU1
T0: tipc_publish() link_timeout()
T1: tipc_nametbl_publish() [grab node lock]*
T2: [grab nametbl write lock]* link_state_event()
T3: named_cluster_distribute() link_activate()
T4: [grab node lock]* tipc_node_link_up()
T5: tipc_nametbl_publish()
T6: [grab nametble write lock]*
The opposite order of holding nametbl write lock and node lock on
above two different paths may result in a deadlock. If we move the
the delivery of named messages via link out of name nametbl lock,
the reverse order of holding locks will be eliminated, as a result,
the deadlock will be killed as well.
Signed-off-by: Ying Xue <ying.xue@windriver.com>
Reviewed-by: Erik Hugne <erik.hugne@ericsson.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'net/tipc')
-rw-r--r-- | net/tipc/name_distr.c | 18 | ||||
-rw-r--r-- | net/tipc/name_distr.h | 5 | ||||
-rw-r--r-- | net/tipc/name_table.c | 12 |
3 files changed, 22 insertions, 13 deletions
diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c index 36a72822601c..974a73f3d876 100644 --- a/net/tipc/name_distr.c +++ b/net/tipc/name_distr.c | |||
@@ -127,7 +127,7 @@ static struct sk_buff *named_prepare_buf(u32 type, u32 size, u32 dest) | |||
127 | return buf; | 127 | return buf; |
128 | } | 128 | } |
129 | 129 | ||
130 | static void named_cluster_distribute(struct sk_buff *buf) | 130 | void named_cluster_distribute(struct sk_buff *buf) |
131 | { | 131 | { |
132 | struct sk_buff *buf_copy; | 132 | struct sk_buff *buf_copy; |
133 | struct tipc_node *n_ptr; | 133 | struct tipc_node *n_ptr; |
@@ -156,7 +156,7 @@ static void named_cluster_distribute(struct sk_buff *buf) | |||
156 | /** | 156 | /** |
157 | * tipc_named_publish - tell other nodes about a new publication by this node | 157 | * tipc_named_publish - tell other nodes about a new publication by this node |
158 | */ | 158 | */ |
159 | void tipc_named_publish(struct publication *publ) | 159 | struct sk_buff *tipc_named_publish(struct publication *publ) |
160 | { | 160 | { |
161 | struct sk_buff *buf; | 161 | struct sk_buff *buf; |
162 | struct distr_item *item; | 162 | struct distr_item *item; |
@@ -165,23 +165,23 @@ void tipc_named_publish(struct publication *publ) | |||
165 | publ_lists[publ->scope]->size++; | 165 | publ_lists[publ->scope]->size++; |
166 | 166 | ||
167 | if (publ->scope == TIPC_NODE_SCOPE) | 167 | if (publ->scope == TIPC_NODE_SCOPE) |
168 | return; | 168 | return NULL; |
169 | 169 | ||
170 | buf = named_prepare_buf(PUBLICATION, ITEM_SIZE, 0); | 170 | buf = named_prepare_buf(PUBLICATION, ITEM_SIZE, 0); |
171 | if (!buf) { | 171 | if (!buf) { |
172 | pr_warn("Publication distribution failure\n"); | 172 | pr_warn("Publication distribution failure\n"); |
173 | return; | 173 | return NULL; |
174 | } | 174 | } |
175 | 175 | ||
176 | item = (struct distr_item *)msg_data(buf_msg(buf)); | 176 | item = (struct distr_item *)msg_data(buf_msg(buf)); |
177 | publ_to_item(item, publ); | 177 | publ_to_item(item, publ); |
178 | named_cluster_distribute(buf); | 178 | return buf; |
179 | } | 179 | } |
180 | 180 | ||
181 | /** | 181 | /** |
182 | * tipc_named_withdraw - tell other nodes about a withdrawn publication by this node | 182 | * tipc_named_withdraw - tell other nodes about a withdrawn publication by this node |
183 | */ | 183 | */ |
184 | void tipc_named_withdraw(struct publication *publ) | 184 | struct sk_buff *tipc_named_withdraw(struct publication *publ) |
185 | { | 185 | { |
186 | struct sk_buff *buf; | 186 | struct sk_buff *buf; |
187 | struct distr_item *item; | 187 | struct distr_item *item; |
@@ -190,17 +190,17 @@ void tipc_named_withdraw(struct publication *publ) | |||
190 | publ_lists[publ->scope]->size--; | 190 | publ_lists[publ->scope]->size--; |
191 | 191 | ||
192 | if (publ->scope == TIPC_NODE_SCOPE) | 192 | if (publ->scope == TIPC_NODE_SCOPE) |
193 | return; | 193 | return NULL; |
194 | 194 | ||
195 | buf = named_prepare_buf(WITHDRAWAL, ITEM_SIZE, 0); | 195 | buf = named_prepare_buf(WITHDRAWAL, ITEM_SIZE, 0); |
196 | if (!buf) { | 196 | if (!buf) { |
197 | pr_warn("Withdrawal distribution failure\n"); | 197 | pr_warn("Withdrawal distribution failure\n"); |
198 | return; | 198 | return NULL; |
199 | } | 199 | } |
200 | 200 | ||
201 | item = (struct distr_item *)msg_data(buf_msg(buf)); | 201 | item = (struct distr_item *)msg_data(buf_msg(buf)); |
202 | publ_to_item(item, publ); | 202 | publ_to_item(item, publ); |
203 | named_cluster_distribute(buf); | 203 | return buf; |
204 | } | 204 | } |
205 | 205 | ||
206 | /* | 206 | /* |
diff --git a/net/tipc/name_distr.h b/net/tipc/name_distr.h index 9b312ccfd43e..47ff829f9361 100644 --- a/net/tipc/name_distr.h +++ b/net/tipc/name_distr.h | |||
@@ -39,8 +39,9 @@ | |||
39 | 39 | ||
40 | #include "name_table.h" | 40 | #include "name_table.h" |
41 | 41 | ||
42 | void tipc_named_publish(struct publication *publ); | 42 | struct sk_buff *tipc_named_publish(struct publication *publ); |
43 | void tipc_named_withdraw(struct publication *publ); | 43 | struct sk_buff *tipc_named_withdraw(struct publication *publ); |
44 | void named_cluster_distribute(struct sk_buff *buf); | ||
44 | void tipc_named_node_up(unsigned long node); | 45 | void tipc_named_node_up(unsigned long node); |
45 | void tipc_named_rcv(struct sk_buff *buf); | 46 | void tipc_named_rcv(struct sk_buff *buf); |
46 | void tipc_named_reinit(void); | 47 | void tipc_named_reinit(void); |
diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c index 042e8e3cabc0..9bcf4b58853f 100644 --- a/net/tipc/name_table.c +++ b/net/tipc/name_table.c | |||
@@ -664,6 +664,7 @@ struct publication *tipc_nametbl_publish(u32 type, u32 lower, u32 upper, | |||
664 | u32 scope, u32 port_ref, u32 key) | 664 | u32 scope, u32 port_ref, u32 key) |
665 | { | 665 | { |
666 | struct publication *publ; | 666 | struct publication *publ; |
667 | struct sk_buff *buf = NULL; | ||
667 | 668 | ||
668 | if (table.local_publ_count >= TIPC_MAX_PUBLICATIONS) { | 669 | if (table.local_publ_count >= TIPC_MAX_PUBLICATIONS) { |
669 | pr_warn("Publication failed, local publication limit reached (%u)\n", | 670 | pr_warn("Publication failed, local publication limit reached (%u)\n", |
@@ -676,9 +677,12 @@ struct publication *tipc_nametbl_publish(u32 type, u32 lower, u32 upper, | |||
676 | tipc_own_addr, port_ref, key); | 677 | tipc_own_addr, port_ref, key); |
677 | if (likely(publ)) { | 678 | if (likely(publ)) { |
678 | table.local_publ_count++; | 679 | table.local_publ_count++; |
679 | tipc_named_publish(publ); | 680 | buf = tipc_named_publish(publ); |
680 | } | 681 | } |
681 | write_unlock_bh(&tipc_nametbl_lock); | 682 | write_unlock_bh(&tipc_nametbl_lock); |
683 | |||
684 | if (buf) | ||
685 | named_cluster_distribute(buf); | ||
682 | return publ; | 686 | return publ; |
683 | } | 687 | } |
684 | 688 | ||
@@ -688,15 +692,19 @@ struct publication *tipc_nametbl_publish(u32 type, u32 lower, u32 upper, | |||
688 | int tipc_nametbl_withdraw(u32 type, u32 lower, u32 ref, u32 key) | 692 | int tipc_nametbl_withdraw(u32 type, u32 lower, u32 ref, u32 key) |
689 | { | 693 | { |
690 | struct publication *publ; | 694 | struct publication *publ; |
695 | struct sk_buff *buf; | ||
691 | 696 | ||
692 | write_lock_bh(&tipc_nametbl_lock); | 697 | write_lock_bh(&tipc_nametbl_lock); |
693 | publ = tipc_nametbl_remove_publ(type, lower, tipc_own_addr, ref, key); | 698 | publ = tipc_nametbl_remove_publ(type, lower, tipc_own_addr, ref, key); |
694 | if (likely(publ)) { | 699 | if (likely(publ)) { |
695 | table.local_publ_count--; | 700 | table.local_publ_count--; |
696 | tipc_named_withdraw(publ); | 701 | buf = tipc_named_withdraw(publ); |
697 | write_unlock_bh(&tipc_nametbl_lock); | 702 | write_unlock_bh(&tipc_nametbl_lock); |
698 | list_del_init(&publ->pport_list); | 703 | list_del_init(&publ->pport_list); |
699 | kfree(publ); | 704 | kfree(publ); |
705 | |||
706 | if (buf) | ||
707 | named_cluster_distribute(buf); | ||
700 | return 1; | 708 | return 1; |
701 | } | 709 | } |
702 | write_unlock_bh(&tipc_nametbl_lock); | 710 | write_unlock_bh(&tipc_nametbl_lock); |