Skip to content

Commit

Permalink
tipc: purge tipc_net_lock lock
Browse files Browse the repository at this point in the history
Now tipc routing hierarchy comprises the structures 'node', 'link'and
'bearer'. The whole hierarchy is protected by a big read/write lock,
tipc_net_lock, to ensure that nothing is added or removed while code
is accessing any of these structures. Obviously the locking policy
makes node, link and bearer components closely bound together so that
their relationship becomes unnecessarily complex. In the worst case,
such locking policy not only has a negative influence on performance,
but also it's prone to lead to deadlock occasionally.

In order o decouple the complex relationship between bearer and node
as well as link, the locking policy is adjusted as follows:

- Bearer level
  RTNL lock is used on update side, and RCU is used on read side.
  Meanwhile, all bearer instances including broadcast bearer are
  saved into bearer_list array.

- Node and link level
  All node instances are saved into two tipc_node_list and node_htable
  lists. The two lists are protected by node_list_lock on write side,
  and they are guarded with RCU lock on read side. All members in node
  structure including link instances are protected by node spin lock.

- The relationship between bearer and node
  When link accesses bearer, it first needs to find the bearer with
  its bearer identity from the bearer_list array. When bearer accesses
  node, it can iterate the node_htable hash list with the node
  address to find the corresponding node.

In the new locking policy, every component has its private locking
solution and the relationship between bearer and node is very simple,
that is, they can find each other with node address or bearer identity
from node_htable hash list or bearer_list array.

Until now above all changes have been done, so tipc_net_lock can be
removed safely.

Signed-off-by: Ying Xue <ying.xue@windriver.com>
Reviewed-by: Jon Maloy <jon.maloy@ericsson.com>
Reviewed-by: Erik Hugne <erik.hugne@ericsson.com>
Tested-by: Erik Hugne <erik.hugne@ericsson.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
Ying Xue authored and David S. Miller committed Apr 23, 2014
1 parent 2231c5a commit 7216cd9
Show file tree
Hide file tree
Showing 7 changed files with 42 additions and 100 deletions.
6 changes: 2 additions & 4 deletions net/tipc/bcast.c
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ void tipc_bclink_acknowledge(struct tipc_node *n_ptr, u32 acked)
/**
* tipc_bclink_update_link_state - update broadcast link state
*
* tipc_net_lock and node lock set
* RCU and node lock set
*/
void tipc_bclink_update_link_state(struct tipc_node *n_ptr, u32 last_sent)
{
Expand Down Expand Up @@ -335,8 +335,6 @@ void tipc_bclink_update_link_state(struct tipc_node *n_ptr, u32 last_sent)
*
* Delay any upcoming NACK by this node if another node has already
* requested the first message this node is going to ask for.
*
* Only tipc_net_lock set.
*/
static void bclink_peek_nack(struct tipc_msg *msg)
{
Expand Down Expand Up @@ -408,7 +406,7 @@ static void bclink_accept_pkt(struct tipc_node *node, u32 seqno)
/**
* tipc_bclink_rcv - receive a broadcast packet, and deliver upwards
*
* tipc_net_lock is read_locked, no other locks set
* RCU is locked, no other locks set
*/
void tipc_bclink_rcv(struct sk_buff *buf)
{
Expand Down
31 changes: 10 additions & 21 deletions net/tipc/bearer.c
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,6 @@ struct sk_buff *tipc_bearer_get_names(void)
if (!buf)
return NULL;

read_lock_bh(&tipc_net_lock);
for (i = 0; media_info_array[i] != NULL; i++) {
for (j = 0; j < MAX_BEARERS; j++) {
b = rtnl_dereference(bearer_list[j]);
Expand All @@ -211,7 +210,6 @@ struct sk_buff *tipc_bearer_get_names(void)
}
}
}
read_unlock_bh(&tipc_net_lock);
return buf;
}

Expand Down Expand Up @@ -285,13 +283,11 @@ int tipc_enable_bearer(const char *name, u32 disc_domain, u32 priority)
return -EINVAL;
}

write_lock_bh(&tipc_net_lock);

m_ptr = tipc_media_find(b_names.media_name);
if (!m_ptr) {
pr_warn("Bearer <%s> rejected, media <%s> not registered\n",
name, b_names.media_name);
goto exit;
return -EINVAL;
}

if (priority == TIPC_MEDIA_LINK_PRI)
Expand All @@ -309,14 +305,14 @@ int tipc_enable_bearer(const char *name, u32 disc_domain, u32 priority)
if (!strcmp(name, b_ptr->name)) {
pr_warn("Bearer <%s> rejected, already enabled\n",
name);
goto exit;
return -EINVAL;
}
if ((b_ptr->priority == priority) &&
(++with_this_prio > 2)) {
if (priority-- == 0) {
pr_warn("Bearer <%s> rejected, duplicate priority\n",
name);
goto exit;
return -EINVAL;
}
pr_warn("Bearer <%s> priority adjustment required %u->%u\n",
name, priority + 1, priority);
Expand All @@ -326,21 +322,20 @@ int tipc_enable_bearer(const char *name, u32 disc_domain, u32 priority)
if (bearer_id >= MAX_BEARERS) {
pr_warn("Bearer <%s> rejected, bearer limit reached (%u)\n",
name, MAX_BEARERS);
goto exit;
return -EINVAL;
}

b_ptr = kzalloc(sizeof(*b_ptr), GFP_ATOMIC);
if (!b_ptr) {
res = -ENOMEM;
goto exit;
}
if (!b_ptr)
return -ENOMEM;

strcpy(b_ptr->name, name);
b_ptr->media = m_ptr;
res = m_ptr->enable_media(b_ptr);
if (res) {
pr_warn("Bearer <%s> rejected, enable failure (%d)\n",
name, -res);
goto exit;
return -EINVAL;
}

b_ptr->identity = bearer_id;
Expand All @@ -355,16 +350,14 @@ int tipc_enable_bearer(const char *name, u32 disc_domain, u32 priority)
bearer_disable(b_ptr, false);
pr_warn("Bearer <%s> rejected, discovery object creation failed\n",
name);
goto exit;
return -EINVAL;
}

rcu_assign_pointer(bearer_list[bearer_id], b_ptr);

pr_info("Enabled bearer <%s>, discovery domain %s, priority %u\n",
name,
tipc_addr_string_fill(addr_string, disc_domain), priority);
exit:
write_unlock_bh(&tipc_net_lock);
return res;
}

Expand All @@ -373,19 +366,17 @@ int tipc_enable_bearer(const char *name, u32 disc_domain, u32 priority)
*/
static int tipc_reset_bearer(struct tipc_bearer *b_ptr)
{
read_lock_bh(&tipc_net_lock);
pr_info("Resetting bearer <%s>\n", b_ptr->name);
tipc_disc_delete(b_ptr->link_req);
tipc_link_reset_list(b_ptr->identity);
tipc_disc_create(b_ptr, &b_ptr->bcast_addr);
read_unlock_bh(&tipc_net_lock);
return 0;
}

/**
* bearer_disable
*
* Note: This routine assumes caller holds tipc_net_lock.
* Note: This routine assumes caller holds RTNL lock.
*/
static void bearer_disable(struct tipc_bearer *b_ptr, bool shutting_down)
{
Expand All @@ -412,7 +403,6 @@ int tipc_disable_bearer(const char *name)
struct tipc_bearer *b_ptr;
int res;

write_lock_bh(&tipc_net_lock);
b_ptr = tipc_bearer_find(name);
if (b_ptr == NULL) {
pr_warn("Attempt to disable unknown bearer <%s>\n", name);
Expand All @@ -421,7 +411,6 @@ int tipc_disable_bearer(const char *name)
bearer_disable(b_ptr, false);
res = 0;
}
write_unlock_bh(&tipc_net_lock);
return res;
}

Expand Down
40 changes: 6 additions & 34 deletions net/tipc/link.c
Original file line number Diff line number Diff line change
Expand Up @@ -835,7 +835,6 @@ int tipc_link_xmit(struct sk_buff *buf, u32 dest, u32 selector)
struct tipc_node *n_ptr;
int res = -ELINKCONG;

read_lock_bh(&tipc_net_lock);
n_ptr = tipc_node_find(dest);
if (n_ptr) {
tipc_node_lock(n_ptr);
Expand All @@ -848,7 +847,6 @@ int tipc_link_xmit(struct sk_buff *buf, u32 dest, u32 selector)
} else {
kfree_skb(buf);
}
read_unlock_bh(&tipc_net_lock);
return res;
}

Expand Down Expand Up @@ -912,7 +910,6 @@ void tipc_link_names_xmit(struct list_head *message_list, u32 dest)
if (list_empty(message_list))
return;

read_lock_bh(&tipc_net_lock);
n_ptr = tipc_node_find(dest);
if (n_ptr) {
tipc_node_lock(n_ptr);
Expand All @@ -927,7 +924,6 @@ void tipc_link_names_xmit(struct list_head *message_list, u32 dest)
}
tipc_node_unlock(n_ptr);
}
read_unlock_bh(&tipc_net_lock);

/* discard the messages if they couldn't be sent */
list_for_each_safe(buf, temp_buf, ((struct sk_buff *)message_list)) {
Expand Down Expand Up @@ -989,7 +985,6 @@ int tipc_link_iovec_xmit_fast(struct tipc_port *sender,
if (unlikely(res < 0))
return res;

read_lock_bh(&tipc_net_lock);
node = tipc_node_find(destaddr);
if (likely(node)) {
tipc_node_lock(node);
Expand All @@ -1000,7 +995,6 @@ int tipc_link_iovec_xmit_fast(struct tipc_port *sender,
&sender->max_pkt);
exit:
tipc_node_unlock(node);
read_unlock_bh(&tipc_net_lock);
return res;
}

Expand All @@ -1017,7 +1011,6 @@ int tipc_link_iovec_xmit_fast(struct tipc_port *sender,
*/
sender->max_pkt = l_ptr->max_pkt;
tipc_node_unlock(node);
read_unlock_bh(&tipc_net_lock);


if ((msg_hdr_sz(hdr) + res) <= sender->max_pkt)
Expand All @@ -1028,7 +1021,6 @@ int tipc_link_iovec_xmit_fast(struct tipc_port *sender,
}
tipc_node_unlock(node);
}
read_unlock_bh(&tipc_net_lock);

/* Couldn't find a link to the destination node */
kfree_skb(buf);
Expand Down Expand Up @@ -1273,12 +1265,9 @@ static void link_reset_all(unsigned long addr)
char addr_string[16];
u32 i;

read_lock_bh(&tipc_net_lock);
n_ptr = tipc_node_find((u32)addr);
if (!n_ptr) {
read_unlock_bh(&tipc_net_lock);
if (!n_ptr)
return; /* node no longer exists */
}

tipc_node_lock(n_ptr);

Expand All @@ -1293,7 +1282,6 @@ static void link_reset_all(unsigned long addr)
}

tipc_node_unlock(n_ptr);
read_unlock_bh(&tipc_net_lock);
}

static void link_retransmit_failure(struct tipc_link *l_ptr,
Expand Down Expand Up @@ -1458,7 +1446,6 @@ static int link_recv_buf_validate(struct sk_buff *buf)
*/
void tipc_rcv(struct sk_buff *head, struct tipc_bearer *b_ptr)
{
read_lock_bh(&tipc_net_lock);
while (head) {
struct tipc_node *n_ptr;
struct tipc_link *l_ptr;
Expand Down Expand Up @@ -1646,7 +1633,6 @@ void tipc_rcv(struct sk_buff *head, struct tipc_bearer *b_ptr)
discard:
kfree_skb(buf);
}
read_unlock_bh(&tipc_net_lock);
}

/**
Expand Down Expand Up @@ -2408,8 +2394,6 @@ void tipc_link_set_queue_limits(struct tipc_link *l_ptr, u32 window)
/* tipc_link_find_owner - locate owner node of link by link's name
* @name: pointer to link name string
* @bearer_id: pointer to index in 'node->links' array where the link was found.
* Caller must hold 'tipc_net_lock' to ensure node and bearer are not deleted;
* this also prevents link deletion.
*
* Returns pointer to node owning the link, or 0 if no matching link is found.
*/
Expand Down Expand Up @@ -2471,7 +2455,7 @@ static int link_value_is_valid(u16 cmd, u32 new_value)
* @new_value: new value of link, bearer, or media setting
* @cmd: which link, bearer, or media attribute to set (TIPC_CMD_SET_LINK_*)
*
* Caller must hold 'tipc_net_lock' to ensure link/bearer/media is not deleted.
* Caller must hold RTNL lock to ensure link/bearer/media is not deleted.
*
* Returns 0 if value updated and negative value on error.
*/
Expand Down Expand Up @@ -2577,9 +2561,7 @@ struct sk_buff *tipc_link_cmd_config(const void *req_tlv_area, int req_tlv_space
" (cannot change setting on broadcast link)");
}

read_lock_bh(&tipc_net_lock);
res = link_cmd_set_value(args->name, new_value, cmd);
read_unlock_bh(&tipc_net_lock);
if (res)
return tipc_cfg_reply_error_string("cannot change link setting");

Expand Down Expand Up @@ -2613,22 +2595,18 @@ struct sk_buff *tipc_link_cmd_reset_stats(const void *req_tlv_area, int req_tlv_
return tipc_cfg_reply_error_string("link not found");
return tipc_cfg_reply_none();
}
read_lock_bh(&tipc_net_lock);
node = tipc_link_find_owner(link_name, &bearer_id);
if (!node) {
read_unlock_bh(&tipc_net_lock);
if (!node)
return tipc_cfg_reply_error_string("link not found");
}

tipc_node_lock(node);
l_ptr = node->links[bearer_id];
if (!l_ptr) {
tipc_node_unlock(node);
read_unlock_bh(&tipc_net_lock);
return tipc_cfg_reply_error_string("link not found");
}
link_reset_statistics(l_ptr);
tipc_node_unlock(node);
read_unlock_bh(&tipc_net_lock);
return tipc_cfg_reply_none();
}

Expand Down Expand Up @@ -2661,18 +2639,15 @@ static int tipc_link_stats(const char *name, char *buf, const u32 buf_size)
if (!strcmp(name, tipc_bclink_name))
return tipc_bclink_stats(buf, buf_size);

read_lock_bh(&tipc_net_lock);
node = tipc_link_find_owner(name, &bearer_id);
if (!node) {
read_unlock_bh(&tipc_net_lock);
if (!node)
return 0;
}

tipc_node_lock(node);

l = node->links[bearer_id];
if (!l) {
tipc_node_unlock(node);
read_unlock_bh(&tipc_net_lock);
return 0;
}

Expand Down Expand Up @@ -2738,7 +2713,6 @@ static int tipc_link_stats(const char *name, char *buf, const u32 buf_size)
(s->accu_queue_sz / s->queue_sz_counts) : 0);

tipc_node_unlock(node);
read_unlock_bh(&tipc_net_lock);
return ret;
}

Expand Down Expand Up @@ -2789,7 +2763,6 @@ u32 tipc_link_get_max_pkt(u32 dest, u32 selector)
if (dest == tipc_own_addr)
return MAX_MSG_SIZE;

read_lock_bh(&tipc_net_lock);
n_ptr = tipc_node_find(dest);
if (n_ptr) {
tipc_node_lock(n_ptr);
Expand All @@ -2798,7 +2771,6 @@ u32 tipc_link_get_max_pkt(u32 dest, u32 selector)
res = l_ptr->max_pkt;
tipc_node_unlock(n_ptr);
}
read_unlock_bh(&tipc_net_lock);
return res;
}

Expand Down
2 changes: 0 additions & 2 deletions net/tipc/name_distr.c
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,6 @@ void tipc_named_node_up(unsigned long nodearg)
u32 max_item_buf = 0;

/* compute maximum amount of publication data to send per message */
read_lock_bh(&tipc_net_lock);
n_ptr = tipc_node_find(node);
if (n_ptr) {
tipc_node_lock(n_ptr);
Expand All @@ -258,7 +257,6 @@ void tipc_named_node_up(unsigned long nodearg)
ITEM_SIZE) * ITEM_SIZE;
tipc_node_unlock(n_ptr);
}
read_unlock_bh(&tipc_net_lock);
if (!max_item_buf)
return;

Expand Down
Loading

0 comments on commit 7216cd9

Please sign in to comment.