Skip to content

Commit

Permalink
fs: dlm: fix mark setting deadlock
Browse files Browse the repository at this point in the history 
This patch fixes an deadlock issue when dlm_lowcomms_close() is called.
When dlm_lowcomms_close() is called the clusters_root.subsys.su_mutex is
held to remove configfs items. At this time we flushing (e.g.
cancel_work_sync()) the workers of send and recv workqueue. Due the fact
that we accessing configfs items (mark values), these workers will lock
clusters_root.subsys.su_mutex as well which are already hold by
dlm_lowcomms_close() and ends in a deadlock situation.

[67170.703046] ======================================================
[67170.703965] WARNING: possible circular locking dependency detected
[67170.704758] 5.11.0-rc4+ torvalds#22 Tainted: G        W
[67170.705433] ------------------------------------------------------
[67170.706228] dlm_controld/280 is trying to acquire lock:
[67170.706915] ffff9f2f475a6948 ((wq_completion)dlm_recv){+.+.}-{0:0}, at: __flush_work+0x203/0x4c0
[67170.708026]
               but task is already holding lock:
[67170.708758] ffffffffa132f878 (&clusters_root.subsys.su_mutex){+.+.}-{3:3}, at: configfs_rmdir+0x29b/0x310
[67170.710016]
               which lock already depends on the new lock.

The new behaviour adds the mark value to the node address configuration
which doesn't require to held the clusters_root.subsys.su_mutex by
accessing mark values in a separate datastructure. However the mark
values can be set now only after a node address was set which is the
case when the user is using dlm_controld.

Signed-off-by: Alexander Aring <[email protected]>
Signed-off-by: David Teigland <[email protected]>
  • Loading branch information
@teigland
Alexander Aring authored and teigland committed Mar 9, 2021
1 parent 92c4895 commit e125fbe
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 35 deletions.
29 changes: 10 additions & 19 deletions fs/dlm/config.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -688,14 +688,23 @@ static ssize_t comm_mark_show(struct config_item *item, char *buf)
static ssize_t comm_mark_store(struct config_item *item, const char *buf,
size_t len)
{
struct dlm_comm *comm;
unsigned int mark;
int rc;

rc = kstrtouint(buf, 0, &mark);
if (rc)
return rc;

config_item_to_comm(item)->mark = mark;
if (mark == 0)
mark = dlm_config.ci_mark;

comm = config_item_to_comm(item);
rc = dlm_lowcomms_nodes_set_mark(comm->nodeid, mark);
if (rc)
return rc;

comm->mark = mark;
return len;
}

Expand Down Expand Up @@ -870,24 +879,6 @@ int dlm_comm_seq(int nodeid, uint32_t *seq)
return 0;
}

void dlm_comm_mark(int nodeid, unsigned int *mark)
{
struct dlm_comm *cm;

cm = get_comm(nodeid);
if (!cm) {
*mark = dlm_config.ci_mark;
return;
}

if (cm->mark)
*mark = cm->mark;
else
*mark = dlm_config.ci_mark;

put_comm(cm);
}

int dlm_our_nodeid(void)
{
return local_comm ? local_comm->nodeid : 0;
Expand Down
1 change: 0 additions & 1 deletion fs/dlm/config.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ void dlm_config_exit(void);
int dlm_config_nodes(char *lsname, struct dlm_config_node **nodes_out,
int *count_out);
int dlm_comm_seq(int nodeid, uint32_t *seq);
void dlm_comm_mark(int nodeid, unsigned int *mark);
int dlm_our_nodeid(void);
int dlm_our_addr(struct sockaddr_storage *addr, int num);

Expand Down
49 changes: 34 additions & 15 deletions fs/dlm/lowcomms.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ struct writequeue_entry {
struct dlm_node_addr {
struct list_head list;
int nodeid;
int mark;
int addr_count;
int curr_addr_index;
struct sockaddr_storage *addr[DLM_MAX_ADDR_COUNT];
Expand Down Expand Up @@ -303,7 +304,8 @@ static int addr_compare(const struct sockaddr_storage *x,
}

static int nodeid_to_addr(int nodeid, struct sockaddr_storage *sas_out,
struct sockaddr *sa_out, bool try_new_addr)
struct sockaddr *sa_out, bool try_new_addr,
unsigned int *mark)
{
struct sockaddr_storage sas;
struct dlm_node_addr *na;
Expand Down Expand Up @@ -331,6 +333,8 @@ static int nodeid_to_addr(int nodeid, struct sockaddr_storage *sas_out,
if (!na->addr_count)
return -ENOENT;

*mark = na->mark;

if (sas_out)
memcpy(sas_out, &sas, sizeof(struct sockaddr_storage));

Expand All @@ -350,7 +354,8 @@ static int nodeid_to_addr(int nodeid, struct sockaddr_storage *sas_out,
return 0;
}

static int addr_to_nodeid(struct sockaddr_storage *addr, int *nodeid)
static int addr_to_nodeid(struct sockaddr_storage *addr, int *nodeid,
unsigned int *mark)
{
struct dlm_node_addr *na;
int rv = -EEXIST;
Expand All @@ -364,6 +369,7 @@ static int addr_to_nodeid(struct sockaddr_storage *addr, int *nodeid)
for (addr_i = 0; addr_i < na->addr_count; addr_i++) {
if (addr_compare(na->addr[addr_i], addr)) {
*nodeid = na->nodeid;
*mark = na->mark;
rv = 0;
goto unlock;
}
Expand Down Expand Up @@ -412,6 +418,7 @@ int dlm_lowcomms_addr(int nodeid, struct sockaddr_storage *addr, int len)
new_node->nodeid = nodeid;
new_node->addr[0] = new_addr;
new_node->addr_count = 1;
new_node->mark = dlm_config.ci_mark;
list_add(&new_node->list, &dlm_node_addrs);
spin_unlock(&dlm_node_addrs_spin);
return 0;
Expand Down Expand Up @@ -519,6 +526,23 @@ int dlm_lowcomms_connect_node(int nodeid)
return 0;
}

int dlm_lowcomms_nodes_set_mark(int nodeid, unsigned int mark)
{
struct dlm_node_addr *na;

spin_lock(&dlm_node_addrs_spin);
na = find_node_addr(nodeid);
if (!na) {
spin_unlock(&dlm_node_addrs_spin);
return -ENOENT;
}

na->mark = mark;
spin_unlock(&dlm_node_addrs_spin);

return 0;
}

static void lowcomms_error_report(struct sock *sk)
{
struct connection *con;
Expand Down Expand Up @@ -867,7 +891,7 @@ static int accept_from_sock(struct listen_connection *con)

/* Get the new node's NODEID */
make_sockaddr(&peeraddr, 0, &len);
if (addr_to_nodeid(&peeraddr, &nodeid)) {
if (addr_to_nodeid(&peeraddr, &nodeid, &mark)) {
unsigned char *b=(unsigned char *)&peeraddr;
log_print("connect from non cluster node");
print_hex_dump_bytes("ss: ", DUMP_PREFIX_NONE,
Expand All @@ -876,9 +900,6 @@ static int accept_from_sock(struct listen_connection *con)
return -1;
}

dlm_comm_mark(nodeid, &mark);
sock_set_mark(newsock->sk, mark);

log_print("got connection from %d", nodeid);

/* Check to see if we already have a connection to this node. This
Expand All @@ -892,6 +913,8 @@ static int accept_from_sock(struct listen_connection *con)
goto accept_err;
}

sock_set_mark(newsock->sk, mark);

mutex_lock(&newcon->sock_mutex);
if (newcon->sock) {
struct connection *othercon = newcon->othercon;
Expand Down Expand Up @@ -1015,8 +1038,6 @@ static void sctp_connect_to_sock(struct connection *con)
struct socket *sock;
unsigned int mark;

dlm_comm_mark(con->nodeid, &mark);

mutex_lock(&con->sock_mutex);

/* Some odd races can cause double-connects, ignore them */
Expand All @@ -1029,7 +1050,7 @@ static void sctp_connect_to_sock(struct connection *con)
}

memset(&daddr, 0, sizeof(daddr));
result = nodeid_to_addr(con->nodeid, &daddr, NULL, true);
result = nodeid_to_addr(con->nodeid, &daddr, NULL, true, &mark);
if (result < 0) {
log_print("no address for nodeid %d", con->nodeid);
goto out;
Expand Down Expand Up @@ -1104,13 +1125,11 @@ static void sctp_connect_to_sock(struct connection *con)
static void tcp_connect_to_sock(struct connection *con)
{
struct sockaddr_storage saddr, src_addr;
unsigned int mark;
int addr_len;
struct socket *sock = NULL;
unsigned int mark;
int result;

dlm_comm_mark(con->nodeid, &mark);

mutex_lock(&con->sock_mutex);
if (con->retries++ > MAX_CONNECT_RETRIES)
goto out;
Expand All @@ -1125,15 +1144,15 @@ static void tcp_connect_to_sock(struct connection *con)
if (result < 0)
goto out_err;

sock_set_mark(sock->sk, mark);

memset(&saddr, 0, sizeof(saddr));
result = nodeid_to_addr(con->nodeid, &saddr, NULL, false);
result = nodeid_to_addr(con->nodeid, &saddr, NULL, false, &mark);
if (result < 0) {
log_print("no address for nodeid %d", con->nodeid);
goto out_err;
}

sock_set_mark(sock->sk, mark);

add_sock(sock, con);

/* Bind to our cluster-known address connecting to avoid
Expand Down
1 change: 1 addition & 0 deletions fs/dlm/lowcomms.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ int dlm_lowcomms_close(int nodeid);
void *dlm_lowcomms_get_buffer(int nodeid, int len, gfp_t allocation, char **ppc);
void dlm_lowcomms_commit_buffer(void *mh);
int dlm_lowcomms_connect_node(int nodeid);
int dlm_lowcomms_nodes_set_mark(int nodeid, unsigned int mark);
int dlm_lowcomms_addr(int nodeid, struct sockaddr_storage *addr, int len);

#endif /* __LOWCOMMS_DOT_H__ */
Expand Down

0 comments on commit e125fbe

Please sign in to comment.