Commit f9a2c80b authored by Ying Xue's avatar Ying Xue Committed by David S. Miller

tipc: introduce new spinlock to protect struct link_req

Currently, only 'bearer_lock' is used to protect struct link_req in
the function disc_timeout(). This is unsafe, since the member fields
'num_nodes' and 'timer_intv' might be accessed by below three different
threads simultaneously, none of them grabbing bearer_lock in the
critical region:

link_activate()
  tipc_bearer_add_dest()
    tipc_disc_add_dest()
      req->num_nodes++;

tipc_link_reset()
  tipc_bearer_remove_dest()
    tipc_disc_remove_dest()
      req->num_nodes--
      disc_update()
        read req->num_nodes
	write req->timer_intv

disc_timeout()
  read req->num_nodes
  read/write req->timer_intv

Without lock protection, the only symptom of a race is that discovery
messages occasionally may not be sent out. This is not fatal, since such
messages are best-effort anyway. On the other hand, since discovery
messages are not time critical, adding a protecting lock brings no
serious overhead either. So we add a new, dedicated spinlock in
order to guarantee absolute data consistency in link_req objects.
This also helps reduce the overall role of the bearer_lock, which
we want to remove completely in a later commit series.
Signed-off-by: default avatarYing Xue <ying.xue@windriver.com>
Reviewed-by: default avatarPaul Gortmaker <paul.gortmaker@windriver.com>
Signed-off-by: default avatarJon Maloy <jon.maloy@ericsson.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent b9d4c339
...@@ -50,6 +50,7 @@ ...@@ -50,6 +50,7 @@
* @dest: destination address for request messages * @dest: destination address for request messages
* @domain: network domain to which links can be established * @domain: network domain to which links can be established
* @num_nodes: number of nodes currently discovered (i.e. with an active link) * @num_nodes: number of nodes currently discovered (i.e. with an active link)
* @lock: spinlock for controlling access to requests
* @buf: request message to be (repeatedly) sent * @buf: request message to be (repeatedly) sent
* @timer: timer governing period between requests * @timer: timer governing period between requests
* @timer_intv: current interval between requests (in ms) * @timer_intv: current interval between requests (in ms)
...@@ -59,6 +60,7 @@ struct tipc_link_req { ...@@ -59,6 +60,7 @@ struct tipc_link_req {
struct tipc_media_addr dest; struct tipc_media_addr dest;
u32 domain; u32 domain;
int num_nodes; int num_nodes;
spinlock_t lock;
struct sk_buff *buf; struct sk_buff *buf;
struct timer_list timer; struct timer_list timer;
unsigned int timer_intv; unsigned int timer_intv;
...@@ -274,7 +276,9 @@ static void disc_update(struct tipc_link_req *req) ...@@ -274,7 +276,9 @@ static void disc_update(struct tipc_link_req *req)
*/ */
void tipc_disc_add_dest(struct tipc_link_req *req) void tipc_disc_add_dest(struct tipc_link_req *req)
{ {
spin_lock_bh(&req->lock);
req->num_nodes++; req->num_nodes++;
spin_unlock_bh(&req->lock);
} }
/** /**
...@@ -283,8 +287,10 @@ void tipc_disc_add_dest(struct tipc_link_req *req) ...@@ -283,8 +287,10 @@ void tipc_disc_add_dest(struct tipc_link_req *req)
*/ */
void tipc_disc_remove_dest(struct tipc_link_req *req) void tipc_disc_remove_dest(struct tipc_link_req *req)
{ {
spin_lock_bh(&req->lock);
req->num_nodes--; req->num_nodes--;
disc_update(req); disc_update(req);
spin_unlock_bh(&req->lock);
} }
/** /**
...@@ -297,7 +303,7 @@ static void disc_timeout(struct tipc_link_req *req) ...@@ -297,7 +303,7 @@ static void disc_timeout(struct tipc_link_req *req)
{ {
int max_delay; int max_delay;
spin_lock_bh(&req->bearer->lock); spin_lock_bh(&req->lock);
/* Stop searching if only desired node has been found */ /* Stop searching if only desired node has been found */
if (tipc_node(req->domain) && req->num_nodes) { if (tipc_node(req->domain) && req->num_nodes) {
...@@ -325,7 +331,7 @@ static void disc_timeout(struct tipc_link_req *req) ...@@ -325,7 +331,7 @@ static void disc_timeout(struct tipc_link_req *req)
k_start_timer(&req->timer, req->timer_intv); k_start_timer(&req->timer, req->timer_intv);
exit: exit:
spin_unlock_bh(&req->bearer->lock); spin_unlock_bh(&req->lock);
} }
/** /**
...@@ -356,6 +362,7 @@ int tipc_disc_create(struct tipc_bearer *b_ptr, struct tipc_media_addr *dest, ...@@ -356,6 +362,7 @@ int tipc_disc_create(struct tipc_bearer *b_ptr, struct tipc_media_addr *dest,
req->domain = dest_domain; req->domain = dest_domain;
req->num_nodes = 0; req->num_nodes = 0;
req->timer_intv = TIPC_LINK_REQ_INIT; req->timer_intv = TIPC_LINK_REQ_INIT;
spin_lock_init(&req->lock);
k_init_timer(&req->timer, (Handler)disc_timeout, (unsigned long)req); k_init_timer(&req->timer, (Handler)disc_timeout, (unsigned long)req);
k_start_timer(&req->timer, req->timer_intv); k_start_timer(&req->timer, req->timer_intv);
b_ptr->link_req = req; b_ptr->link_req = req;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment