From: Christophe JAILLET christophe.jaillet@wanadoo.fr

8 is not the maximum size of the suffix used when creating debugfs files.

Let the compiler compute the correct size, and only give a hint about the longest possible string that is used.

When building with W=1, this fixes the following warnings:

fs/dlm/debug_fs.c: In function ‘dlm_create_debug_file’: fs/dlm/debug_fs.c:1020:58: error: ‘snprintf’ output may be truncated before the last format character [-Werror=format-truncation=] 1020 | snprintf(name, DLM_LOCKSPACE_LEN + 8, "%s_waiters", ls->ls_name); | ^ fs/dlm/debug_fs.c:1020:9: note: ‘snprintf’ output between 9 and 73 bytes into a destination of size 72 1020 | snprintf(name, DLM_LOCKSPACE_LEN + 8, "%s_waiters", ls->ls_name); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ fs/dlm/debug_fs.c:1031:50: error: ‘_queued_asts’ directive output may be truncated writing 12 bytes into a region of size between 8 and 72 [-Werror=format-truncation=] 1031 | snprintf(name, DLM_LOCKSPACE_LEN + 8, "%s_queued_asts", ls->ls_name); | ^~~~~~~~~~~~ fs/dlm/debug_fs.c:1031:9: note: ‘snprintf’ output between 13 and 77 bytes into a destination of size 72 1031 | snprintf(name, DLM_LOCKSPACE_LEN + 8, "%s_queued_asts", ls->ls_name); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Fixes: 541adb0d4d10b ("fs: dlm: debugfs for queued callbacks") Signed-off-by: Christophe JAILLET christophe.jaillet@wanadoo.fr Signed-off-by: Alexander Aring aahringo@redhat.com --- fs/dlm/debug_fs.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/dlm/debug_fs.c b/fs/dlm/debug_fs.c index e9726c6cbdf2..c93359ceaae6 100644 --- a/fs/dlm/debug_fs.c +++ b/fs/dlm/debug_fs.c @@ -973,7 +973,8 @@ void dlm_delete_debug_comms_file(void *ctx)

void dlm_create_debug_file(struct dlm_ls *ls) { - char name[DLM_LOCKSPACE_LEN + 8]; + /* Reserve enough space for the longest file name */ + char name[DLM_LOCKSPACE_LEN + sizeof("_queued_asts")];

/* format 1 */

From: Christophe JAILLET christophe.jaillet@wanadoo.fr

There is no need to clear the buffer used to build the file name.

snprintf() already guarantees that it is NULL terminated and such a (useless) precaution was not done for the first string (i.e ls_debug_rsb_dentry)

So, save a few LoC.

Signed-off-by: Christophe JAILLET christophe.jaillet@wanadoo.fr Signed-off-by: Alexander Aring aahringo@redhat.com --- fs/dlm/debug_fs.c | 5 ----- 1 file changed, 5 deletions(-)

diff --git a/fs/dlm/debug_fs.c b/fs/dlm/debug_fs.c index c93359ceaae6..42f332f46359 100644 --- a/fs/dlm/debug_fs.c +++ b/fs/dlm/debug_fs.c @@ -986,7 +986,6 @@ void dlm_create_debug_file(struct dlm_ls *ls)

/* format 2 */

- memset(name, 0, sizeof(name)); snprintf(name, sizeof(name), "%s_locks", ls->ls_name);

ls->ls_debug_locks_dentry = debugfs_create_file(name, @@ -997,7 +996,6 @@ void dlm_create_debug_file(struct dlm_ls *ls)

/* format 3 */

- memset(name, 0, sizeof(name)); snprintf(name, sizeof(name), "%s_all", ls->ls_name);

ls->ls_debug_all_dentry = debugfs_create_file(name, @@ -1008,7 +1006,6 @@ void dlm_create_debug_file(struct dlm_ls *ls)

/* format 4 */

- memset(name, 0, sizeof(name)); snprintf(name, sizeof(name), "%s_toss", ls->ls_name);

ls->ls_debug_toss_dentry = debugfs_create_file(name, @@ -1017,7 +1014,6 @@ void dlm_create_debug_file(struct dlm_ls *ls) ls, &format4_fops);

- memset(name, 0, sizeof(name)); snprintf(name, sizeof(name), "%s_waiters", ls->ls_name);

ls->ls_debug_waiters_dentry = debugfs_create_file(name, @@ -1028,7 +1024,6 @@ void dlm_create_debug_file(struct dlm_ls *ls)

/* format 5 */

- memset(name, 0, sizeof(name)); snprintf(name, sizeof(name), "%s_queued_asts", ls->ls_name);

ls->ls_debug_queued_asts_dentry = debugfs_create_file(name,

This patch will lookup existing nodes instead of always creating them when dlm_midcomms_addr() is called. The idea is here to create midcomms nodes when user space getting informed that nodes joins the cluster. This is the case when dlm_midcomms_addr() is called, however it can be called multiple times by user space to add several address configurations to one node e.g. when using SCTP. Those multiple times need to be filtered out and we doing that by looking up if the node exists before. Due configfs entry it is safe that this function gets only called once at a time.

Cc: stable@vger.kernel.org Fixes: 63e711b08160 ("fs: dlm: create midcomms nodes when configure") Signed-off-by: Alexander Aring aahringo@redhat.com --- fs/dlm/midcomms.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/fs/dlm/midcomms.c b/fs/dlm/midcomms.c index f641b36a36db..455265c6ba53 100644 --- a/fs/dlm/midcomms.c +++ b/fs/dlm/midcomms.c @@ -337,13 +337,21 @@ static struct midcomms_node *nodeid2node(int nodeid)

int dlm_midcomms_addr(int nodeid, struct sockaddr_storage *addr, int len) { - int ret, r = nodeid_hash(nodeid); + int ret, idx, r = nodeid_hash(nodeid); struct midcomms_node *node;

ret = dlm_lowcomms_addr(nodeid, addr, len); if (ret) return ret;

+ idx = srcu_read_lock(&nodes_srcu); + node = __find_node(nodeid, r); + if (node) { + srcu_read_unlock(&nodes_srcu, idx); + return 0; + } + srcu_read_unlock(&nodes_srcu, idx); + node = kmalloc(sizeof(*node), GFP_NOFS); if (!node) return -ENOMEM;

The idea of commit 63e711b08160 ("fs: dlm: create midcomms nodes when configure") is to set the midcomms node lifetime when a node joins or leaves the cluster. Currently we can hit the following warning:

[10844.611495] ------------[ cut here ]------------ [10844.615913] WARNING: CPU: 4 PID: 84304 at fs/dlm/midcomms.c:1263 dlm_midcomms_remove_member+0x13f/0x180 [dlm]

or running in a state where we hit a midcomms node usage count in a negative value:

[ 260.830782] node 2 users dec count -1

The first warning happens when the a specific node does not exists and it was probably removed but dlm_midcomms_close() which is called when a node leaves the cluster. The second kernel log message is probably in a case when dlm_midcomms_addr() is called when a joined the cluster but due fencing a node leaved the cluster without getting removed from the lockspace. If the node joins the cluster and it was removed from the cluster due fencing the first call is to remove the node from lockspaces triggered by the user space. In both cases if the node wasn't found or the user count is zero, we should ignore any additional midcomms handling of dlm_midcomms_remove_member().

Cc: stable@vger.kernel.org Fixes: 63e711b08160 ("fs: dlm: create midcomms nodes when configure") Signed-off-by: Alexander Aring aahringo@redhat.com --- fs/dlm/midcomms.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/fs/dlm/midcomms.c b/fs/dlm/midcomms.c index 455265c6ba53..4ad71e97cec2 100644 --- a/fs/dlm/midcomms.c +++ b/fs/dlm/midcomms.c @@ -1268,12 +1268,23 @@ void dlm_midcomms_remove_member(int nodeid)

idx = srcu_read_lock(&nodes_srcu); node = nodeid2node(nodeid); - if (WARN_ON_ONCE(!node)) { + /* in case of dlm_midcomms_close() removes node */ + if (!node) { srcu_read_unlock(&nodes_srcu, idx); return; }

spin_lock(&node->state_lock); + /* case of dlm_midcomms_addr() created node but + * was not added before because dlm_midcomms_close() + * removed the node + */ + if (!node->users) { + spin_unlock(&node->state_lock); + srcu_read_unlock(&nodes_srcu, idx); + return; + } + node->users--; pr_debug("node %d users dec count %d\n", nodeid, node->users);

In case of an final DLM message we can't should not send an ack out after the final message. This patch moves the ack message before the messages will be transmitted. If it's the final message and the receiving node turns into DLM_CLOSED state another ack messages will being received and turning the receiving node into DLM_ESTABLISHED again.

Cc: stable@vger.kernel.org Fixes: 1696c75f1864 ("fs: dlm: add send ack threshold and append acks to msgs") Signed-off-by: Alexander Aring aahringo@redhat.com --- fs/dlm/midcomms.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/dlm/midcomms.c b/fs/dlm/midcomms.c index 6bc8d7f89b2c..2247ebb61be1 100644 --- a/fs/dlm/midcomms.c +++ b/fs/dlm/midcomms.c @@ -1038,15 +1038,15 @@ struct dlm_mhandle *dlm_midcomms_get_mhandle(int nodeid, int len,

break; case DLM_VERSION_3_2: + /* send ack back if necessary */ + dlm_send_ack_threshold(node, DLM_SEND_ACK_BACK_MSG_THRESHOLD); + msg = dlm_midcomms_get_msg_3_2(mh, nodeid, len, allocation, ppc); if (!msg) { dlm_free_mhandle(mh); goto err; } - - /* send ack back if necessary */ - dlm_send_ack_threshold(node, DLM_SEND_ACK_BACK_MSG_THRESHOLD); break; default: dlm_free_mhandle(mh);