From d96d0f9617793b6cd95b9b9a8fef66f69f8f6b1b Mon Sep 17 00:00:00 2001 From: Paulo Miguel Almeida Date: Wed, 12 Oct 2022 09:23:14 +1300 Subject: [PATCH 01/37] dlm: replace one-element array with fixed size array One-element arrays are deprecated. So, replace one-element array with fixed size array member in struct dlm_ls, and refactor the rest of the code, accordingly. Link: https://github.com/KSPP/linux/issues/79 Link: https://github.com/KSPP/linux/issues/228 Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836 Link: https://lore.kernel.org/lkml/Y0W5jkiXUkpNl4ap@mail.google.com/ Signed-off-by: Paulo Miguel Almeida Reviewed-by: Gustavo A. R. Silva Reviewed-by: Kees Cook Signed-off-by: David Teigland --- fs/dlm/dlm_internal.h | 2 +- fs/dlm/lockspace.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/dlm/dlm_internal.h b/fs/dlm/dlm_internal.h index e34c3d2639a5a..94fadb619ba09 100644 --- a/fs/dlm/dlm_internal.h +++ b/fs/dlm/dlm_internal.h @@ -670,7 +670,7 @@ struct dlm_ls { void *ls_ops_arg; int ls_namelen; - char ls_name[1]; + char ls_name[DLM_LOCKSPACE_LEN + 1]; }; /* diff --git a/fs/dlm/lockspace.c b/fs/dlm/lockspace.c index bae050df7abff..9479c81109796 100644 --- a/fs/dlm/lockspace.c +++ b/fs/dlm/lockspace.c @@ -473,7 +473,7 @@ static int new_lockspace(const char *name, const char *cluster, error = -ENOMEM; - ls = kzalloc(sizeof(struct dlm_ls) + namelen, GFP_NOFS); + ls = kzalloc(sizeof(*ls), GFP_NOFS); if (!ls) goto out; memcpy(ls->ls_name, name, namelen); From 08ae0547e75ec3d062b6b6b9cf4830c730df68df Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Thu, 27 Oct 2022 16:45:11 -0400 Subject: [PATCH 02/37] fs: dlm: fix sock release if listen fails This patch fixes a double sock_release() call when the listen() is called for the dlm lowcomms listen socket. The caller of dlm_listen_for_all should never care about releasing the socket if dlm_listen_for_all() fails, it's done now only once if listen() fails. Cc: stable@vger.kernel.org Fixes: 2dc6b1158c28 ("fs: dlm: introduce generic listen") Signed-off-by: Alexander Aring Signed-off-by: David Teigland --- fs/dlm/lowcomms.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c index 59f64c596233b..2cb9f3b49e05c 100644 --- a/fs/dlm/lowcomms.c +++ b/fs/dlm/lowcomms.c @@ -1820,7 +1820,7 @@ static int dlm_listen_for_all(void) result = sock->ops->listen(sock, 5); if (result < 0) { dlm_close_sock(&listen_con.sock); - goto out; + return result; } return 0; @@ -2023,7 +2023,6 @@ int dlm_lowcomms_start(void) dlm_proto_ops = NULL; fail_proto_ops: dlm_allow_conn = 0; - dlm_close_sock(&listen_con.sock); work_stop(); fail_local: deinit_local(); From f0f4bb431bd543ed7bebbaea3ce326cfcd5388bc Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Thu, 27 Oct 2022 16:45:12 -0400 Subject: [PATCH 03/37] fs: dlm: retry accept() until -EAGAIN or error returns This patch fixes a race if we get two times an socket data ready event while the listen connection worker is queued. Currently it will be served only once but we need to do it (in this case twice) until we hit -EAGAIN which tells us there is no pending accept going on. This patch wraps an do while loop until we receive a return value which is different than 0 as it was done before commit d11ccd451b65 ("fs: dlm: listen socket out of connection hash"). Cc: stable@vger.kernel.org Fixes: d11ccd451b65 ("fs: dlm: listen socket out of connection hash") Signed-off-by: Alexander Aring Signed-off-by: David Teigland --- fs/dlm/lowcomms.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c index 2cb9f3b49e05c..871d4e9f49fb6 100644 --- a/fs/dlm/lowcomms.c +++ b/fs/dlm/lowcomms.c @@ -1543,7 +1543,11 @@ static void process_recv_sockets(struct work_struct *work) static void process_listen_recv_socket(struct work_struct *work) { - accept_from_sock(&listen_con); + int ret; + + do { + ret = accept_from_sock(&listen_con); + } while (!ret); } static void dlm_connect(struct connection *con) From 57a5724ef0b332eb6e78250157910a006b01bf6e Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Thu, 27 Oct 2022 16:45:13 -0400 Subject: [PATCH 04/37] fs: dlm: remove send repeat remove handling This patch removes the send repeat remove handling. This handling is there to repeatingly DLM_MSG_REMOVE messages in cases the dlm stack thinks it was not received at the first time. In cases of message drops this functionality is necessary, but since the DLM midcomms layer guarantees there are no messages drops between cluster nodes this feature became not strict necessary anymore. Due message delays/processing it could be that two send_repeat_remove() are sent out while the other should be still on it's way. We remove the repeat remove handling because we are sure that the message cannot be dropped due communication errors. Signed-off-by: Alexander Aring Signed-off-by: David Teigland --- fs/dlm/lock.c | 74 --------------------------------------------------- 1 file changed, 74 deletions(-) diff --git a/fs/dlm/lock.c b/fs/dlm/lock.c index 94a72ede57646..b246d71b5e17a 100644 --- a/fs/dlm/lock.c +++ b/fs/dlm/lock.c @@ -4044,66 +4044,6 @@ static int validate_message(struct dlm_lkb *lkb, struct dlm_message *ms) return error; } -static void send_repeat_remove(struct dlm_ls *ls, char *ms_name, int len) -{ - char name[DLM_RESNAME_MAXLEN + 1]; - struct dlm_message *ms; - struct dlm_mhandle *mh; - struct dlm_rsb *r; - uint32_t hash, b; - int rv, dir_nodeid; - - memset(name, 0, sizeof(name)); - memcpy(name, ms_name, len); - - hash = jhash(name, len, 0); - b = hash & (ls->ls_rsbtbl_size - 1); - - dir_nodeid = dlm_hash2nodeid(ls, hash); - - log_error(ls, "send_repeat_remove dir %d %s", dir_nodeid, name); - - spin_lock(&ls->ls_rsbtbl[b].lock); - rv = dlm_search_rsb_tree(&ls->ls_rsbtbl[b].keep, name, len, &r); - if (!rv) { - spin_unlock(&ls->ls_rsbtbl[b].lock); - log_error(ls, "repeat_remove on keep %s", name); - return; - } - - rv = dlm_search_rsb_tree(&ls->ls_rsbtbl[b].toss, name, len, &r); - if (!rv) { - spin_unlock(&ls->ls_rsbtbl[b].lock); - log_error(ls, "repeat_remove on toss %s", name); - return; - } - - /* use ls->remove_name2 to avoid conflict with shrink? */ - - spin_lock(&ls->ls_remove_spin); - ls->ls_remove_len = len; - memcpy(ls->ls_remove_name, name, DLM_RESNAME_MAXLEN); - spin_unlock(&ls->ls_remove_spin); - spin_unlock(&ls->ls_rsbtbl[b].lock); - - rv = _create_message(ls, sizeof(struct dlm_message) + len, - dir_nodeid, DLM_MSG_REMOVE, &ms, &mh); - if (rv) - goto out; - - memcpy(ms->m_extra, name, len); - ms->m_hash = cpu_to_le32(hash); - - send_message(mh, ms); - -out: - spin_lock(&ls->ls_remove_spin); - ls->ls_remove_len = 0; - memset(ls->ls_remove_name, 0, DLM_RESNAME_MAXLEN); - spin_unlock(&ls->ls_remove_spin); - wake_up(&ls->ls_remove_wait); -} - static int receive_request(struct dlm_ls *ls, struct dlm_message *ms) { struct dlm_lkb *lkb; @@ -4173,25 +4113,11 @@ static int receive_request(struct dlm_ls *ls, struct dlm_message *ms) ENOTBLK request failures when the lookup reply designating us as master is delayed. */ - /* We could repeatedly return -EBADR here if our send_remove() is - delayed in being sent/arriving/being processed on the dir node. - Another node would repeatedly lookup up the master, and the dir - node would continue returning our nodeid until our send_remove - took effect. - - We send another remove message in case our previous send_remove - was lost/ignored/missed somehow. */ - if (error != -ENOTBLK) { log_limit(ls, "receive_request %x from %d %d", le32_to_cpu(ms->m_lkid), from_nodeid, error); } - if (namelen && error == -EBADR) { - send_repeat_remove(ls, ms->m_extra, namelen); - msleep(1000); - } - setup_stub_lkb(ls, ms); send_request_reply(&ls->ls_stub_rsb, &ls->ls_stub_lkb, error); return error; From 5b787667e87a373a2f8f70e6be2b5d99c408462f Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Thu, 27 Oct 2022 16:45:14 -0400 Subject: [PATCH 05/37] fs: dlm: use packet in dlm_mhandle To allow more than just dereferencing the inner header we directly point to the inner dlm packet which allows us to dereference the header, rcom or message structure. Signed-off-by: Alexander Aring Signed-off-by: David Teigland --- 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 6489bc22ad617..4eed40d66da10 100644 --- a/fs/dlm/midcomms.c +++ b/fs/dlm/midcomms.c @@ -194,7 +194,7 @@ struct midcomms_node { }; struct dlm_mhandle { - const struct dlm_header *inner_hd; + const union dlm_packet *inner_p; struct midcomms_node *node; struct dlm_opts *opts; struct dlm_msg *msg; @@ -1055,7 +1055,7 @@ static struct dlm_msg *dlm_midcomms_get_msg_3_2(struct dlm_mhandle *mh, int node dlm_fill_opts_header(opts, len, mh->seq); *ppc += sizeof(*opts); - mh->inner_hd = (const struct dlm_header *)*ppc; + mh->inner_p = (const union dlm_packet *)*ppc; return msg; } @@ -1133,7 +1133,7 @@ struct dlm_mhandle *dlm_midcomms_get_mhandle(int nodeid, int len, static void dlm_midcomms_commit_msg_3_2(struct dlm_mhandle *mh) { /* nexthdr chain for fast lookup */ - mh->opts->o_nextcmd = mh->inner_hd->h_cmd; + mh->opts->o_nextcmd = mh->inner_p->header.h_cmd; mh->committed = true; dlm_lowcomms_commit_msg(mh->msg); } From e01c4b7bd41522ae0299c07e2ee8c721fee02595 Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Thu, 27 Oct 2022 16:45:15 -0400 Subject: [PATCH 06/37] fd: dlm: trace send/recv of dlm message and rcom This patch adds tracepoints for send and recv cases of dlm messages and dlm rcom messages. In case of send and dlm message we add the dlm rsb resource name this dlm messages belongs to. This has the advantage to follow dlm messages on a per lock basis. In case of recv message the resource name can be extracted by follow the send message sequence number. The dlm message DLM_MSG_PURGE doesn't belong to a lock request and will not set the resource name in a dlm_message trace. The same for all rcom messages. There is additional handling required for this debugging functionality which is tried to be small as possible. Also the midcomms layer gets aware of lock resource names, for now this is required to make a connection between sequence number and lock resource names. It is for debugging purpose only. Signed-off-by: Alexander Aring Signed-off-by: David Teigland --- fs/dlm/lock.c | 21 +-- fs/dlm/midcomms.c | 45 +++++- fs/dlm/midcomms.h | 3 +- fs/dlm/rcom.c | 4 +- include/trace/events/dlm.h | 297 +++++++++++++++++++++++++++++++++++++ 5 files changed, 353 insertions(+), 17 deletions(-) diff --git a/fs/dlm/lock.c b/fs/dlm/lock.c index b246d71b5e17a..0b1bc24536ceb 100644 --- a/fs/dlm/lock.c +++ b/fs/dlm/lock.c @@ -3611,9 +3611,10 @@ static int create_message(struct dlm_rsb *r, struct dlm_lkb *lkb, /* further lowcomms enhancements or alternate implementations may make the return value from this function useful at some point */ -static int send_message(struct dlm_mhandle *mh, struct dlm_message *ms) +static int send_message(struct dlm_mhandle *mh, struct dlm_message *ms, + const void *name, int namelen) { - dlm_midcomms_commit_mhandle(mh); + dlm_midcomms_commit_mhandle(mh, name, namelen); return 0; } @@ -3679,7 +3680,7 @@ static int send_common(struct dlm_rsb *r, struct dlm_lkb *lkb, int mstype) send_args(r, lkb, ms); - error = send_message(mh, ms); + error = send_message(mh, ms, r->res_name, r->res_length); if (error) goto fail; return 0; @@ -3742,7 +3743,7 @@ static int send_grant(struct dlm_rsb *r, struct dlm_lkb *lkb) ms->m_result = 0; - error = send_message(mh, ms); + error = send_message(mh, ms, r->res_name, r->res_length); out: return error; } @@ -3763,7 +3764,7 @@ static int send_bast(struct dlm_rsb *r, struct dlm_lkb *lkb, int mode) ms->m_bastmode = cpu_to_le32(mode); - error = send_message(mh, ms); + error = send_message(mh, ms, r->res_name, r->res_length); out: return error; } @@ -3786,7 +3787,7 @@ static int send_lookup(struct dlm_rsb *r, struct dlm_lkb *lkb) send_args(r, lkb, ms); - error = send_message(mh, ms); + error = send_message(mh, ms, r->res_name, r->res_length); if (error) goto fail; return 0; @@ -3811,7 +3812,7 @@ static int send_remove(struct dlm_rsb *r) memcpy(ms->m_extra, r->res_name, r->res_length); ms->m_hash = cpu_to_le32(r->res_hash); - error = send_message(mh, ms); + error = send_message(mh, ms, r->res_name, r->res_length); out: return error; } @@ -3833,7 +3834,7 @@ static int send_common_reply(struct dlm_rsb *r, struct dlm_lkb *lkb, ms->m_result = cpu_to_le32(to_dlm_errno(rv)); - error = send_message(mh, ms); + error = send_message(mh, ms, r->res_name, r->res_length); out: return error; } @@ -3874,7 +3875,7 @@ static int send_lookup_reply(struct dlm_ls *ls, struct dlm_message *ms_in, ms->m_result = cpu_to_le32(to_dlm_errno(rv)); ms->m_nodeid = cpu_to_le32(ret_nodeid); - error = send_message(mh, ms); + error = send_message(mh, ms, ms_in->m_extra, receive_extralen(ms_in)); out: return error; } @@ -6300,7 +6301,7 @@ static int send_purge(struct dlm_ls *ls, int nodeid, int pid) ms->m_nodeid = cpu_to_le32(nodeid); ms->m_pid = cpu_to_le32(pid); - return send_message(mh, ms); + return send_message(mh, ms, NULL, 0); } int dlm_user_purge(struct dlm_ls *ls, struct dlm_user_proc *proc, diff --git a/fs/dlm/midcomms.c b/fs/dlm/midcomms.c index 4eed40d66da10..e10a6e97df443 100644 --- a/fs/dlm/midcomms.c +++ b/fs/dlm/midcomms.c @@ -132,6 +132,7 @@ */ #define DLM_DEBUG_FENCE_TERMINATION 0 +#include #include #include "dlm_internal.h" @@ -415,7 +416,7 @@ static int dlm_send_fin(struct midcomms_node *node, m_header->h_cmd = DLM_FIN; pr_debug("sending fin msg to node %d\n", node->nodeid); - dlm_midcomms_commit_mhandle(mh); + dlm_midcomms_commit_mhandle(mh, NULL, 0); set_bit(DLM_NODE_FLAG_STOP_TX, &node->flags); return 0; @@ -474,6 +475,20 @@ static void dlm_pas_fin_ack_rcv(struct midcomms_node *node) spin_unlock(&node->state_lock); } +static void dlm_receive_buffer_3_2_trace(uint32_t seq, union dlm_packet *p) +{ + switch (p->header.h_cmd) { + case DLM_MSG: + trace_dlm_recv_message(seq, &p->message); + break; + case DLM_RCOM: + trace_dlm_recv_rcom(seq, &p->rcom); + break; + default: + break; + } +} + static void dlm_midcomms_receive_buffer(union dlm_packet *p, struct midcomms_node *node, uint32_t seq) @@ -534,6 +549,7 @@ static void dlm_midcomms_receive_buffer(union dlm_packet *p, break; default: WARN_ON(test_bit(DLM_NODE_FLAG_STOP_RX, &node->flags)); + dlm_receive_buffer_3_2_trace(seq, p); dlm_receive_buffer(p, node->nodeid); set_bit(DLM_NODE_ULP_DELIVERED, &node->flags); break; @@ -1130,11 +1146,30 @@ struct dlm_mhandle *dlm_midcomms_get_mhandle(int nodeid, int len, } #endif -static void dlm_midcomms_commit_msg_3_2(struct dlm_mhandle *mh) +static void dlm_midcomms_commit_msg_3_2_trace(const struct dlm_mhandle *mh, + const void *name, int namelen) +{ + switch (mh->inner_p->header.h_cmd) { + case DLM_MSG: + trace_dlm_send_message(mh->seq, &mh->inner_p->message, + name, namelen); + break; + case DLM_RCOM: + trace_dlm_send_rcom(mh->seq, &mh->inner_p->rcom); + break; + default: + /* nothing to trace */ + break; + } +} + +static void dlm_midcomms_commit_msg_3_2(struct dlm_mhandle *mh, + const void *name, int namelen) { /* nexthdr chain for fast lookup */ mh->opts->o_nextcmd = mh->inner_p->header.h_cmd; mh->committed = true; + dlm_midcomms_commit_msg_3_2_trace(mh, name, namelen); dlm_lowcomms_commit_msg(mh->msg); } @@ -1142,8 +1177,10 @@ static void dlm_midcomms_commit_msg_3_2(struct dlm_mhandle *mh) * dlm_midcomms_get_mhandle */ #ifndef __CHECKER__ -void dlm_midcomms_commit_mhandle(struct dlm_mhandle *mh) +void dlm_midcomms_commit_mhandle(struct dlm_mhandle *mh, + const void *name, int namelen) { + switch (mh->node->version) { case DLM_VERSION_3_1: srcu_read_unlock(&nodes_srcu, mh->idx); @@ -1154,7 +1191,7 @@ void dlm_midcomms_commit_mhandle(struct dlm_mhandle *mh) dlm_free_mhandle(mh); break; case DLM_VERSION_3_2: - dlm_midcomms_commit_msg_3_2(mh); + dlm_midcomms_commit_msg_3_2(mh, name, namelen); srcu_read_unlock(&nodes_srcu, mh->idx); break; default: diff --git a/fs/dlm/midcomms.h b/fs/dlm/midcomms.h index 82bcd96619228..d6286e80b077a 100644 --- a/fs/dlm/midcomms.h +++ b/fs/dlm/midcomms.h @@ -17,7 +17,8 @@ struct midcomms_node; int dlm_process_incoming_buffer(int nodeid, unsigned char *buf, int buflen); struct dlm_mhandle *dlm_midcomms_get_mhandle(int nodeid, int len, gfp_t allocation, char **ppc); -void dlm_midcomms_commit_mhandle(struct dlm_mhandle *mh); +void dlm_midcomms_commit_mhandle(struct dlm_mhandle *mh, const void *name, + int namelen); int dlm_midcomms_close(int nodeid); int dlm_midcomms_start(void); void dlm_midcomms_shutdown(void); diff --git a/fs/dlm/rcom.c b/fs/dlm/rcom.c index f19860315043a..b76d52e2f6bdd 100644 --- a/fs/dlm/rcom.c +++ b/fs/dlm/rcom.c @@ -91,7 +91,7 @@ static int create_rcom_stateless(struct dlm_ls *ls, int to_nodeid, int type, static void send_rcom(struct dlm_mhandle *mh, struct dlm_rcom *rc) { - dlm_midcomms_commit_mhandle(mh); + dlm_midcomms_commit_mhandle(mh, NULL, 0); } static void send_rcom_stateless(struct dlm_msg *msg, struct dlm_rcom *rc) @@ -516,7 +516,7 @@ int dlm_send_ls_not_ready(int nodeid, struct dlm_rcom *rc_in) rf = (struct rcom_config *) rc->rc_buf; rf->rf_lvblen = cpu_to_le32(~0U); - dlm_midcomms_commit_mhandle(mh); + dlm_midcomms_commit_mhandle(mh, NULL, 0); return 0; } diff --git a/include/trace/events/dlm.h b/include/trace/events/dlm.h index da0eaae98fa34..4ec47828d55ed 100644 --- a/include/trace/events/dlm.h +++ b/include/trace/events/dlm.h @@ -46,6 +46,56 @@ { DLM_SBF_VALNOTVALID, "VALNOTVALID" }, \ { DLM_SBF_ALTMODE, "ALTMODE" }) +#define show_lkb_flags(flags) __print_flags(flags, "|", \ + { DLM_IFL_MSTCPY, "MSTCPY" }, \ + { DLM_IFL_RESEND, "RESEND" }, \ + { DLM_IFL_DEAD, "DEAD" }, \ + { DLM_IFL_OVERLAP_UNLOCK, "OVERLAP_UNLOCK" }, \ + { DLM_IFL_OVERLAP_CANCEL, "OVERLAP_CANCEL" }, \ + { DLM_IFL_ENDOFLIFE, "ENDOFLIFE" }, \ + { DLM_IFL_DEADLOCK_CANCEL, "DEADLOCK_CANCEL" }, \ + { DLM_IFL_STUB_MS, "STUB_MS" }, \ + { DLM_IFL_USER, "USER" }, \ + { DLM_IFL_ORPHAN, "ORPHAN" }) + +#define show_header_cmd(cmd) __print_symbolic(cmd, \ + { DLM_MSG, "MSG"}, \ + { DLM_RCOM, "RCOM"}, \ + { DLM_OPTS, "OPTS"}, \ + { DLM_ACK, "ACK"}, \ + { DLM_FIN, "FIN"}) + +#define show_message_version(version) __print_symbolic(version, \ + { DLM_VERSION_3_1, "3.1"}, \ + { DLM_VERSION_3_2, "3.2"}) + +#define show_message_type(type) __print_symbolic(type, \ + { DLM_MSG_REQUEST, "REQUEST"}, \ + { DLM_MSG_CONVERT, "CONVERT"}, \ + { DLM_MSG_UNLOCK, "UNLOCK"}, \ + { DLM_MSG_CANCEL, "CANCEL"}, \ + { DLM_MSG_REQUEST_REPLY, "REQUEST_REPLY"}, \ + { DLM_MSG_CONVERT_REPLY, "CONVERT_REPLY"}, \ + { DLM_MSG_UNLOCK_REPLY, "UNLOCK_REPLY"}, \ + { DLM_MSG_CANCEL_REPLY, "CANCEL_REPLY"}, \ + { DLM_MSG_GRANT, "GRANT"}, \ + { DLM_MSG_BAST, "BAST"}, \ + { DLM_MSG_LOOKUP, "LOOKUP"}, \ + { DLM_MSG_REMOVE, "REMOVE"}, \ + { DLM_MSG_LOOKUP_REPLY, "LOOKUP_REPLY"}, \ + { DLM_MSG_PURGE, "PURGE"}) + +#define show_rcom_type(type) __print_symbolic(type, \ + { DLM_RCOM_STATUS, "STATUS"}, \ + { DLM_RCOM_NAMES, "NAMES"}, \ + { DLM_RCOM_LOOKUP, "LOOKUP"}, \ + { DLM_RCOM_LOCK, "LOCK"}, \ + { DLM_RCOM_STATUS_REPLY, "STATUS_REPLY"}, \ + { DLM_RCOM_NAMES_REPLY, "NAMES_REPLY"}, \ + { DLM_RCOM_LOOKUP_REPLY, "LOOKUP_REPLY"}, \ + { DLM_RCOM_LOCK_REPLY, "LOCK_REPLY"}) + + /* note: we begin tracing dlm_lock_start() only if ls and lkb are found */ TRACE_EVENT(dlm_lock_start, @@ -290,6 +340,253 @@ TRACE_EVENT(dlm_unlock_end, ); +DECLARE_EVENT_CLASS(dlm_rcom_template, + + TP_PROTO(uint32_t seq, const struct dlm_rcom *rc), + + TP_ARGS(seq, rc), + + TP_STRUCT__entry( + __field(uint32_t, seq) + __field(uint32_t, h_version) + __field(uint32_t, h_lockspace) + __field(uint32_t, h_nodeid) + __field(uint16_t, h_length) + __field(uint8_t, h_cmd) + __field(uint32_t, rc_type) + __field(int32_t, rc_result) + __field(uint64_t, rc_id) + __field(uint64_t, rc_seq) + __field(uint64_t, rc_seq_reply) + __dynamic_array(unsigned char, rc_buf, + le16_to_cpu(rc->rc_header.h_length) - sizeof(*rc)) + ), + + TP_fast_assign( + __entry->seq = seq; + __entry->h_version = le32_to_cpu(rc->rc_header.h_version); + __entry->h_lockspace = le32_to_cpu(rc->rc_header.u.h_lockspace); + __entry->h_nodeid = le32_to_cpu(rc->rc_header.h_nodeid); + __entry->h_length = le16_to_cpu(rc->rc_header.h_length); + __entry->h_cmd = rc->rc_header.h_cmd; + __entry->rc_type = le32_to_cpu(rc->rc_type); + __entry->rc_result = le32_to_cpu(rc->rc_result); + __entry->rc_id = le64_to_cpu(rc->rc_id); + __entry->rc_seq = le64_to_cpu(rc->rc_seq); + __entry->rc_seq_reply = le64_to_cpu(rc->rc_seq_reply); + memcpy(__get_dynamic_array(rc_buf), rc->rc_buf, + __get_dynamic_array_len(rc_buf)); + ), + + TP_printk("seq=%u, h_version=%s h_lockspace=%u h_nodeid=%u " + "h_length=%u h_cmd=%s rc_type=%s rc_result=%d " + "rc_id=%llu rc_seq=%llu rc_seq_reply=%llu " + "rc_buf=0x%s", __entry->seq, + show_message_version(__entry->h_version), + __entry->h_lockspace, __entry->h_nodeid, __entry->h_length, + show_header_cmd(__entry->h_cmd), + show_rcom_type(__entry->rc_type), + __entry->rc_result, __entry->rc_id, __entry->rc_seq, + __entry->rc_seq_reply, + __print_hex_str(__get_dynamic_array(rc_buf), + __get_dynamic_array_len(rc_buf))) + +); + +DEFINE_EVENT(dlm_rcom_template, dlm_send_rcom, + TP_PROTO(uint32_t seq, const struct dlm_rcom *rc), + TP_ARGS(seq, rc)); + +DEFINE_EVENT(dlm_rcom_template, dlm_recv_rcom, + TP_PROTO(uint32_t seq, const struct dlm_rcom *rc), + TP_ARGS(seq, rc)); + +TRACE_EVENT(dlm_send_message, + + TP_PROTO(uint32_t seq, const struct dlm_message *ms, + const void *name, int namelen), + + TP_ARGS(seq, ms, name, namelen), + + TP_STRUCT__entry( + __field(uint32_t, seq) + __field(uint32_t, h_version) + __field(uint32_t, h_lockspace) + __field(uint32_t, h_nodeid) + __field(uint16_t, h_length) + __field(uint8_t, h_cmd) + __field(uint32_t, m_type) + __field(uint32_t, m_nodeid) + __field(uint32_t, m_pid) + __field(uint32_t, m_lkid) + __field(uint32_t, m_remid) + __field(uint32_t, m_parent_lkid) + __field(uint32_t, m_parent_remid) + __field(uint32_t, m_exflags) + __field(uint32_t, m_sbflags) + __field(uint32_t, m_flags) + __field(uint32_t, m_lvbseq) + __field(uint32_t, m_hash) + __field(int32_t, m_status) + __field(int32_t, m_grmode) + __field(int32_t, m_rqmode) + __field(int32_t, m_bastmode) + __field(int32_t, m_asts) + __field(int32_t, m_result) + __dynamic_array(unsigned char, m_extra, + le16_to_cpu(ms->m_header.h_length) - sizeof(*ms)) + __dynamic_array(unsigned char, res_name, namelen) + ), + + TP_fast_assign( + __entry->seq = seq; + __entry->h_version = le32_to_cpu(ms->m_header.h_version); + __entry->h_lockspace = le32_to_cpu(ms->m_header.u.h_lockspace); + __entry->h_nodeid = le32_to_cpu(ms->m_header.h_nodeid); + __entry->h_length = le16_to_cpu(ms->m_header.h_length); + __entry->h_cmd = ms->m_header.h_cmd; + __entry->m_type = le32_to_cpu(ms->m_type); + __entry->m_nodeid = le32_to_cpu(ms->m_nodeid); + __entry->m_pid = le32_to_cpu(ms->m_pid); + __entry->m_lkid = le32_to_cpu(ms->m_lkid); + __entry->m_remid = le32_to_cpu(ms->m_remid); + __entry->m_parent_lkid = le32_to_cpu(ms->m_parent_lkid); + __entry->m_parent_remid = le32_to_cpu(ms->m_parent_remid); + __entry->m_exflags = le32_to_cpu(ms->m_exflags); + __entry->m_sbflags = le32_to_cpu(ms->m_sbflags); + __entry->m_flags = le32_to_cpu(ms->m_flags); + __entry->m_lvbseq = le32_to_cpu(ms->m_lvbseq); + __entry->m_hash = le32_to_cpu(ms->m_hash); + __entry->m_status = le32_to_cpu(ms->m_status); + __entry->m_grmode = le32_to_cpu(ms->m_grmode); + __entry->m_rqmode = le32_to_cpu(ms->m_rqmode); + __entry->m_bastmode = le32_to_cpu(ms->m_bastmode); + __entry->m_asts = le32_to_cpu(ms->m_asts); + __entry->m_result = le32_to_cpu(ms->m_result); + memcpy(__get_dynamic_array(m_extra), ms->m_extra, + __get_dynamic_array_len(m_extra)); + memcpy(__get_dynamic_array(res_name), name, + __get_dynamic_array_len(res_name)); + ), + + TP_printk("seq=%u h_version=%s h_lockspace=%u h_nodeid=%u " + "h_length=%u h_cmd=%s m_type=%s m_nodeid=%u " + "m_pid=%u m_lkid=%u m_remid=%u m_parent_lkid=%u " + "m_parent_remid=%u m_exflags=%s m_sbflags=%s m_flags=%s " + "m_lvbseq=%u m_hash=%u m_status=%d m_grmode=%s " + "m_rqmode=%s m_bastmode=%s m_asts=%d m_result=%d " + "m_extra=0x%s res_name=0x%s", + __entry->seq, show_message_version(__entry->h_version), + __entry->h_lockspace, __entry->h_nodeid, __entry->h_length, + show_header_cmd(__entry->h_cmd), + show_message_type(__entry->m_type), + __entry->m_nodeid, __entry->m_pid, __entry->m_lkid, + __entry->m_remid, __entry->m_parent_lkid, + __entry->m_parent_remid, show_lock_flags(__entry->m_exflags), + show_dlm_sb_flags(__entry->m_sbflags), + show_lkb_flags(__entry->m_flags), __entry->m_lvbseq, + __entry->m_hash, __entry->m_status, + show_lock_mode(__entry->m_grmode), + show_lock_mode(__entry->m_rqmode), + show_lock_mode(__entry->m_bastmode), + __entry->m_asts, __entry->m_result, + __print_hex_str(__get_dynamic_array(m_extra), + __get_dynamic_array_len(m_extra)), + __print_hex_str(__get_dynamic_array(res_name), + __get_dynamic_array_len(res_name))) + +); + +TRACE_EVENT(dlm_recv_message, + + TP_PROTO(uint32_t seq, const struct dlm_message *ms), + + TP_ARGS(seq, ms), + + TP_STRUCT__entry( + __field(uint32_t, seq) + __field(uint32_t, h_version) + __field(uint32_t, h_lockspace) + __field(uint32_t, h_nodeid) + __field(uint16_t, h_length) + __field(uint8_t, h_cmd) + __field(uint32_t, m_type) + __field(uint32_t, m_nodeid) + __field(uint32_t, m_pid) + __field(uint32_t, m_lkid) + __field(uint32_t, m_remid) + __field(uint32_t, m_parent_lkid) + __field(uint32_t, m_parent_remid) + __field(uint32_t, m_exflags) + __field(uint32_t, m_sbflags) + __field(uint32_t, m_flags) + __field(uint32_t, m_lvbseq) + __field(uint32_t, m_hash) + __field(int32_t, m_status) + __field(int32_t, m_grmode) + __field(int32_t, m_rqmode) + __field(int32_t, m_bastmode) + __field(int32_t, m_asts) + __field(int32_t, m_result) + __dynamic_array(unsigned char, m_extra, + le16_to_cpu(ms->m_header.h_length) - sizeof(*ms)) + ), + + TP_fast_assign( + __entry->seq = seq; + __entry->h_version = le32_to_cpu(ms->m_header.h_version); + __entry->h_lockspace = le32_to_cpu(ms->m_header.u.h_lockspace); + __entry->h_nodeid = le32_to_cpu(ms->m_header.h_nodeid); + __entry->h_length = le16_to_cpu(ms->m_header.h_length); + __entry->h_cmd = ms->m_header.h_cmd; + __entry->m_type = le32_to_cpu(ms->m_type); + __entry->m_nodeid = le32_to_cpu(ms->m_nodeid); + __entry->m_pid = le32_to_cpu(ms->m_pid); + __entry->m_lkid = le32_to_cpu(ms->m_lkid); + __entry->m_remid = le32_to_cpu(ms->m_remid); + __entry->m_parent_lkid = le32_to_cpu(ms->m_parent_lkid); + __entry->m_parent_remid = le32_to_cpu(ms->m_parent_remid); + __entry->m_exflags = le32_to_cpu(ms->m_exflags); + __entry->m_sbflags = le32_to_cpu(ms->m_sbflags); + __entry->m_flags = le32_to_cpu(ms->m_flags); + __entry->m_lvbseq = le32_to_cpu(ms->m_lvbseq); + __entry->m_hash = le32_to_cpu(ms->m_hash); + __entry->m_status = le32_to_cpu(ms->m_status); + __entry->m_grmode = le32_to_cpu(ms->m_grmode); + __entry->m_rqmode = le32_to_cpu(ms->m_rqmode); + __entry->m_bastmode = le32_to_cpu(ms->m_bastmode); + __entry->m_asts = le32_to_cpu(ms->m_asts); + __entry->m_result = le32_to_cpu(ms->m_result); + memcpy(__get_dynamic_array(m_extra), ms->m_extra, + __get_dynamic_array_len(m_extra)); + ), + + TP_printk("seq=%u h_version=%s h_lockspace=%u h_nodeid=%u " + "h_length=%u h_cmd=%s m_type=%s m_nodeid=%u " + "m_pid=%u m_lkid=%u m_remid=%u m_parent_lkid=%u " + "m_parent_remid=%u m_exflags=%s m_sbflags=%s m_flags=%s " + "m_lvbseq=%u m_hash=%u m_status=%d m_grmode=%s " + "m_rqmode=%s m_bastmode=%s m_asts=%d m_result=%d " + "m_extra=0x%s", + __entry->seq, show_message_version(__entry->h_version), + __entry->h_lockspace, __entry->h_nodeid, __entry->h_length, + show_header_cmd(__entry->h_cmd), + show_message_type(__entry->m_type), + __entry->m_nodeid, __entry->m_pid, __entry->m_lkid, + __entry->m_remid, __entry->m_parent_lkid, + __entry->m_parent_remid, show_lock_flags(__entry->m_exflags), + show_dlm_sb_flags(__entry->m_sbflags), + show_lkb_flags(__entry->m_flags), __entry->m_lvbseq, + __entry->m_hash, __entry->m_status, + show_lock_mode(__entry->m_grmode), + show_lock_mode(__entry->m_rqmode), + show_lock_mode(__entry->m_bastmode), + __entry->m_asts, __entry->m_result, + __print_hex_str(__get_dynamic_array(m_extra), + __get_dynamic_array_len(m_extra))) + +); + TRACE_EVENT(dlm_send, TP_PROTO(int nodeid, int ret), From 85839f27b17ddebe21d36a174050420783824ed3 Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Thu, 27 Oct 2022 16:45:16 -0400 Subject: [PATCH 07/37] fs: dlm: let dlm_add_cb queue work after resume only We should allow dlm_add_cb() to call queue_work() only after the recovery queued pending for delayed lkbs. This patch will move the switch LSFL_CB_DELAY after the delayed lkb work was processed. Signed-off-by: Alexander Aring Signed-off-by: David Teigland --- fs/dlm/ast.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/dlm/ast.c b/fs/dlm/ast.c index d60a8d8f109df..6e07c151ad288 100644 --- a/fs/dlm/ast.c +++ b/fs/dlm/ast.c @@ -308,8 +308,6 @@ void dlm_callback_resume(struct dlm_ls *ls) if (!ls->ls_callback_wq) return; - clear_bit(LSFL_CB_DELAY, &ls->ls_flags); - more: mutex_lock(&ls->ls_cb_mutex); list_for_each_entry_safe(lkb, safe, &ls->ls_cb_delay, lkb_cb_list) { @@ -320,6 +318,8 @@ void dlm_callback_resume(struct dlm_ls *ls) break; } empty = list_empty(&ls->ls_cb_delay); + if (empty) + clear_bit(LSFL_CB_DELAY, &ls->ls_flags); mutex_unlock(&ls->ls_cb_mutex); sum += count; From d3e4dc5d68c8fef719291cc9f3dc907aac494c55 Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Thu, 27 Oct 2022 16:45:17 -0400 Subject: [PATCH 08/37] fs: dlm: use list_first_entry marco Instead of using list_entry() this patch moves to using the list_first_entry() macro. Signed-off-by: Alexander Aring Signed-off-by: David Teigland --- fs/dlm/user.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/dlm/user.c b/fs/dlm/user.c index c5d27bccc3dc4..6a5de0918a962 100644 --- a/fs/dlm/user.c +++ b/fs/dlm/user.c @@ -857,7 +857,7 @@ static ssize_t device_read(struct file *file, char __user *buf, size_t count, without removing lkb_cb_list; so empty lkb_cb_list is always consistent with empty lkb_callbacks */ - lkb = list_entry(proc->asts.next, struct dlm_lkb, lkb_cb_list); + lkb = list_first_entry(&proc->asts, struct dlm_lkb, lkb_cb_list); /* rem_lkb_callback sets a new lkb_last_cast */ old_mode = lkb->lkb_last_cast.mode; From a4c0352bb1094cbe242f4458e267de845790737a Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Thu, 27 Oct 2022 16:45:18 -0400 Subject: [PATCH 09/37] fs: dlm: convert ls_cb_mutex mutex to spinlock This patch converts the ls_cb_mutex mutex to a spinlock, there is no sleepable context when this lock is held. Signed-off-by: Alexander Aring Signed-off-by: David Teigland --- fs/dlm/ast.c | 12 ++++++------ fs/dlm/dlm_internal.h | 2 +- fs/dlm/lockspace.c | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/fs/dlm/ast.c b/fs/dlm/ast.c index 6e07c151ad288..daaa0dff6ef4e 100644 --- a/fs/dlm/ast.c +++ b/fs/dlm/ast.c @@ -200,13 +200,13 @@ void dlm_add_cb(struct dlm_lkb *lkb, uint32_t flags, int mode, int status, if (!prev_seq) { kref_get(&lkb->lkb_ref); - mutex_lock(&ls->ls_cb_mutex); + spin_lock(&ls->ls_cb_lock); if (test_bit(LSFL_CB_DELAY, &ls->ls_flags)) { list_add(&lkb->lkb_cb_list, &ls->ls_cb_delay); } else { queue_work(ls->ls_callback_wq, &lkb->lkb_cb_work); } - mutex_unlock(&ls->ls_cb_mutex); + spin_unlock(&ls->ls_cb_lock); } out: mutex_unlock(&lkb->lkb_cb_mutex); @@ -289,9 +289,9 @@ void dlm_callback_stop(struct dlm_ls *ls) void dlm_callback_suspend(struct dlm_ls *ls) { if (ls->ls_callback_wq) { - mutex_lock(&ls->ls_cb_mutex); + spin_lock(&ls->ls_cb_lock); set_bit(LSFL_CB_DELAY, &ls->ls_flags); - mutex_unlock(&ls->ls_cb_mutex); + spin_unlock(&ls->ls_cb_lock); flush_workqueue(ls->ls_callback_wq); } @@ -309,7 +309,7 @@ void dlm_callback_resume(struct dlm_ls *ls) return; more: - mutex_lock(&ls->ls_cb_mutex); + spin_lock(&ls->ls_cb_lock); list_for_each_entry_safe(lkb, safe, &ls->ls_cb_delay, lkb_cb_list) { list_del_init(&lkb->lkb_cb_list); queue_work(ls->ls_callback_wq, &lkb->lkb_cb_work); @@ -320,7 +320,7 @@ void dlm_callback_resume(struct dlm_ls *ls) empty = list_empty(&ls->ls_cb_delay); if (empty) clear_bit(LSFL_CB_DELAY, &ls->ls_flags); - mutex_unlock(&ls->ls_cb_mutex); + spin_unlock(&ls->ls_cb_lock); sum += count; if (!empty) { diff --git a/fs/dlm/dlm_internal.h b/fs/dlm/dlm_internal.h index 94fadb619ba09..fc4be8c357036 100644 --- a/fs/dlm/dlm_internal.h +++ b/fs/dlm/dlm_internal.h @@ -631,7 +631,7 @@ struct dlm_ls { /* recovery related */ - struct mutex ls_cb_mutex; + spinlock_t ls_cb_lock; struct list_head ls_cb_delay; /* save for queue_work later */ struct timer_list ls_timer; struct task_struct *ls_recoverd_task; diff --git a/fs/dlm/lockspace.c b/fs/dlm/lockspace.c index 9479c81109796..4965737705b72 100644 --- a/fs/dlm/lockspace.c +++ b/fs/dlm/lockspace.c @@ -567,7 +567,7 @@ static int new_lockspace(const char *name, const char *cluster, init_completion(&ls->ls_recovery_done); ls->ls_recovery_result = -1; - mutex_init(&ls->ls_cb_mutex); + spin_lock_init(&ls->ls_cb_lock); INIT_LIST_HEAD(&ls->ls_cb_delay); ls->ls_recoverd_task = NULL; From 92e95733307e7b6dd352c12fa174089ed51e7208 Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Thu, 27 Oct 2022 16:45:19 -0400 Subject: [PATCH 10/37] fs: dlm: use spin lock instead of mutex There is no need to use a mutex in those hot path sections. We change it to spin lock to serve callbacks more faster by not allowing schedule. The locked sections will not be locked for a long time. Signed-off-by: Alexander Aring Signed-off-by: David Teigland --- fs/dlm/ast.c | 8 ++++---- fs/dlm/dlm_internal.h | 2 +- fs/dlm/lock.c | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/fs/dlm/ast.c b/fs/dlm/ast.c index daaa0dff6ef4e..3e76ec75bc555 100644 --- a/fs/dlm/ast.c +++ b/fs/dlm/ast.c @@ -190,7 +190,7 @@ void dlm_add_cb(struct dlm_lkb *lkb, uint32_t flags, int mode, int status, return; } - mutex_lock(&lkb->lkb_cb_mutex); + spin_lock(&lkb->lkb_cb_lock); prev_seq = lkb->lkb_callbacks[0].seq; rv = dlm_add_lkb_callback(lkb, flags, mode, status, sbflags, new_seq); @@ -209,7 +209,7 @@ void dlm_add_cb(struct dlm_lkb *lkb, uint32_t flags, int mode, int status, spin_unlock(&ls->ls_cb_lock); } out: - mutex_unlock(&lkb->lkb_cb_mutex); + spin_unlock(&lkb->lkb_cb_lock); } void dlm_callback_work(struct work_struct *work) @@ -223,7 +223,7 @@ void dlm_callback_work(struct work_struct *work) memset(&callbacks, 0, sizeof(callbacks)); - mutex_lock(&lkb->lkb_cb_mutex); + spin_lock(&lkb->lkb_cb_lock); if (!lkb->lkb_callbacks[0].seq) { /* no callback work exists, shouldn't happen */ log_error(ls, "dlm_callback_work %x no work", lkb->lkb_id); @@ -244,7 +244,7 @@ void dlm_callback_work(struct work_struct *work) dlm_print_lkb(lkb); dlm_dump_lkb_callbacks(lkb); } - mutex_unlock(&lkb->lkb_cb_mutex); + spin_unlock(&lkb->lkb_cb_lock); castfn = lkb->lkb_astfn; bastfn = lkb->lkb_bastfn; diff --git a/fs/dlm/dlm_internal.h b/fs/dlm/dlm_internal.h index fc4be8c357036..730808289a424 100644 --- a/fs/dlm/dlm_internal.h +++ b/fs/dlm/dlm_internal.h @@ -268,7 +268,7 @@ struct dlm_lkb { unsigned long lkb_timeout_cs; #endif - struct mutex lkb_cb_mutex; + spinlock_t lkb_cb_lock; struct work_struct lkb_cb_work; struct list_head lkb_cb_list; /* for ls_cb_delay or proc->asts */ struct dlm_callback lkb_callbacks[DLM_CALLBACKS_SIZE]; diff --git a/fs/dlm/lock.c b/fs/dlm/lock.c index 0b1bc24536ceb..40e4e4a1c5823 100644 --- a/fs/dlm/lock.c +++ b/fs/dlm/lock.c @@ -1218,7 +1218,7 @@ static int _create_lkb(struct dlm_ls *ls, struct dlm_lkb **lkb_ret, INIT_LIST_HEAD(&lkb->lkb_time_list); #endif INIT_LIST_HEAD(&lkb->lkb_cb_list); - mutex_init(&lkb->lkb_cb_mutex); + spin_lock_init(&lkb->lkb_cb_lock); INIT_WORK(&lkb->lkb_cb_work, dlm_callback_work); idr_preload(GFP_NOFS); From 27d3994ebb5cea9c26f52064a3da8b0e606a8d11 Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Thu, 27 Oct 2022 16:45:20 -0400 Subject: [PATCH 11/37] fs: dlm: move last cast bast time to function call This patch moves the debugging information of the last cast and bast time when calling the last and bast function call. Signed-off-by: Alexander Aring Signed-off-by: David Teigland --- fs/dlm/ast.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/fs/dlm/ast.c b/fs/dlm/ast.c index 3e76ec75bc555..8393d2090c1cc 100644 --- a/fs/dlm/ast.c +++ b/fs/dlm/ast.c @@ -158,15 +158,11 @@ int dlm_rem_lkb_callback(struct dlm_ls *ls, struct dlm_lkb *lkb, } } - if (cb->flags & DLM_CB_CAST) { + if (cb->flags & DLM_CB_CAST) memcpy(&lkb->lkb_last_cast, cb, sizeof(struct dlm_callback)); - lkb->lkb_last_cast_time = ktime_get(); - } - if (cb->flags & DLM_CB_BAST) { + if (cb->flags & DLM_CB_BAST) memcpy(&lkb->lkb_last_bast, cb, sizeof(struct dlm_callback)); - lkb->lkb_last_bast_time = ktime_get(); - } rv = 0; out: return rv; @@ -256,11 +252,13 @@ void dlm_callback_work(struct work_struct *work) continue; } else if (callbacks[i].flags & DLM_CB_BAST) { trace_dlm_bast(ls, lkb, callbacks[i].mode); + lkb->lkb_last_bast_time = ktime_get(); bastfn(lkb->lkb_astparam, callbacks[i].mode); } else if (callbacks[i].flags & DLM_CB_CAST) { lkb->lkb_lksb->sb_status = callbacks[i].sb_status; lkb->lkb_lksb->sb_flags = callbacks[i].sb_flags; trace_dlm_ast(ls, lkb); + lkb->lkb_last_cast_time = ktime_get(); castfn(lkb->lkb_astparam); } } From 61bed0baa4dba17dd06cdfe20481a580718d6c7c Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Thu, 27 Oct 2022 16:45:21 -0400 Subject: [PATCH 12/37] fs: dlm: use a non-static queue for callbacks This patch will introducde a queue implementation for callbacks by using the Linux lists. The current callback queue handling is implemented by a static limit of 6 entries, see DLM_CALLBACKS_SIZE. The sequence number inside the callback structure was used to see if the entries inside the static entry is valid or not. We don't need any sequence numbers anymore with a dynamic datastructure with grows and shrinks during runtime to offer such functionality. We assume that every callback will be delivered to the DLM user if once queued. Therefore the callback flag DLM_CB_SKIP was dropped and the check for skipping bast was moved before worker handling and not skip while the callback worker executes. This will reduce unnecessary queues of the callback worker. All last callback saves are pointers now and don't need to copied over. There is a reference counter for callback structures which will care about to free the callback structures at the right time if they are not referenced anymore. Signed-off-by: Alexander Aring Signed-off-by: David Teigland --- fs/dlm/ast.c | 294 ++++++++++++++++++------------------------ fs/dlm/ast.h | 17 ++- fs/dlm/debug_fs.c | 2 +- fs/dlm/dlm_internal.h | 15 ++- fs/dlm/lock.c | 8 +- fs/dlm/memory.c | 26 ++++ fs/dlm/memory.h | 2 + fs/dlm/user.c | 73 ++++++----- fs/dlm/user.h | 2 +- 9 files changed, 222 insertions(+), 217 deletions(-) diff --git a/fs/dlm/ast.c b/fs/dlm/ast.c index 8393d2090c1cc..078bbbd43a537 100644 --- a/fs/dlm/ast.c +++ b/fs/dlm/ast.c @@ -12,55 +12,68 @@ #include #include "dlm_internal.h" +#include "memory.h" #include "lock.h" #include "user.h" #include "ast.h" -static uint64_t dlm_cb_seq; -static DEFINE_SPINLOCK(dlm_cb_seq_spin); +void dlm_release_callback(struct kref *ref) +{ + struct dlm_callback *cb = container_of(ref, struct dlm_callback, ref); + + dlm_free_cb(cb); +} + +void dlm_callback_set_last_ptr(struct dlm_callback **from, + struct dlm_callback *to) +{ + if (*from) + kref_put(&(*from)->ref, dlm_release_callback); + + if (to) + kref_get(&to->ref); + + *from = to; +} -static void dlm_dump_lkb_callbacks(struct dlm_lkb *lkb) +void dlm_purge_lkb_callbacks(struct dlm_lkb *lkb) { - int i; - - log_print("last_bast %x %llu flags %x mode %d sb %d %x", - lkb->lkb_id, - (unsigned long long)lkb->lkb_last_bast.seq, - lkb->lkb_last_bast.flags, - lkb->lkb_last_bast.mode, - lkb->lkb_last_bast.sb_status, - lkb->lkb_last_bast.sb_flags); - - log_print("last_cast %x %llu flags %x mode %d sb %d %x", - lkb->lkb_id, - (unsigned long long)lkb->lkb_last_cast.seq, - lkb->lkb_last_cast.flags, - lkb->lkb_last_cast.mode, - lkb->lkb_last_cast.sb_status, - lkb->lkb_last_cast.sb_flags); - - for (i = 0; i < DLM_CALLBACKS_SIZE; i++) { - log_print("cb %x %llu flags %x mode %d sb %d %x", - lkb->lkb_id, - (unsigned long long)lkb->lkb_callbacks[i].seq, - lkb->lkb_callbacks[i].flags, - lkb->lkb_callbacks[i].mode, - lkb->lkb_callbacks[i].sb_status, - lkb->lkb_callbacks[i].sb_flags); + struct dlm_callback *cb, *safe; + + list_for_each_entry_safe(cb, safe, &lkb->lkb_callbacks, list) { + list_del(&cb->list); + kref_put(&cb->ref, dlm_release_callback); } + + /* TODO */ + lkb->lkb_flags &= ~DLM_IFL_NEED_SCHED; + + /* invalidate */ + dlm_callback_set_last_ptr(&lkb->lkb_last_cast, NULL); + dlm_callback_set_last_ptr(&lkb->lkb_last_cb, NULL); + lkb->lkb_last_bast_mode = -1; } -int dlm_add_lkb_callback(struct dlm_lkb *lkb, uint32_t flags, int mode, - int status, uint32_t sbflags, uint64_t seq) +int dlm_enqueue_lkb_callback(struct dlm_lkb *lkb, uint32_t flags, int mode, + int status, uint32_t sbflags) { struct dlm_ls *ls = lkb->lkb_resource->res_ls; - uint64_t prev_seq; + int rv = DLM_ENQUEUE_CALLBACK_SUCCESS; + struct dlm_callback *cb; int prev_mode; - int i, rv; - for (i = 0; i < DLM_CALLBACKS_SIZE; i++) { - if (lkb->lkb_callbacks[i].seq) - continue; + if (flags & DLM_CB_BAST) { + /* if cb is a bast, it should be skipped if the blocking mode is + * compatible with the last granted mode + */ + if (lkb->lkb_last_cast) { + if (dlm_modes_compat(mode, lkb->lkb_last_cast->mode)) { + log_debug(ls, "skip %x bast mode %d for cast mode %d", + lkb->lkb_id, mode, + lkb->lkb_last_cast->mode); + goto out; + } + } /* * Suppress some redundant basts here, do more on removal. @@ -68,132 +81,75 @@ int dlm_add_lkb_callback(struct dlm_lkb *lkb, uint32_t flags, int mode, * is a bast for the same mode or a more restrictive mode. * (the addional > PR check is needed for PR/CW inversion) */ - - if ((i > 0) && (flags & DLM_CB_BAST) && - (lkb->lkb_callbacks[i-1].flags & DLM_CB_BAST)) { - - prev_seq = lkb->lkb_callbacks[i-1].seq; - prev_mode = lkb->lkb_callbacks[i-1].mode; + if (lkb->lkb_last_cb && lkb->lkb_last_cb->flags & DLM_CB_BAST) { + prev_mode = lkb->lkb_last_cb->mode; if ((prev_mode == mode) || (prev_mode > mode && prev_mode > DLM_LOCK_PR)) { - - log_debug(ls, "skip %x add bast %llu mode %d " - "for bast %llu mode %d", - lkb->lkb_id, - (unsigned long long)seq, - mode, - (unsigned long long)prev_seq, - prev_mode); - rv = 0; + log_debug(ls, "skip %x add bast mode %d for bast mode %d", + lkb->lkb_id, mode, prev_mode); goto out; } } - - lkb->lkb_callbacks[i].seq = seq; - lkb->lkb_callbacks[i].flags = flags; - lkb->lkb_callbacks[i].mode = mode; - lkb->lkb_callbacks[i].sb_status = status; - lkb->lkb_callbacks[i].sb_flags = (sbflags & 0x000000FF); - rv = 0; - break; } - if (i == DLM_CALLBACKS_SIZE) { - log_error(ls, "no callbacks %x %llu flags %x mode %d sb %d %x", - lkb->lkb_id, (unsigned long long)seq, - flags, mode, status, sbflags); - dlm_dump_lkb_callbacks(lkb); - rv = -1; + cb = dlm_allocate_cb(); + if (!cb) { + rv = DLM_ENQUEUE_CALLBACK_FAILURE; goto out; } - out: - return rv; -} - -int dlm_rem_lkb_callback(struct dlm_ls *ls, struct dlm_lkb *lkb, - struct dlm_callback *cb, int *resid) -{ - int i, rv; - - *resid = 0; - - if (!lkb->lkb_callbacks[0].seq) { - rv = -ENOENT; - goto out; - } - - /* oldest undelivered cb is callbacks[0] */ - - memcpy(cb, &lkb->lkb_callbacks[0], sizeof(struct dlm_callback)); - memset(&lkb->lkb_callbacks[0], 0, sizeof(struct dlm_callback)); - /* shift others down */ - - for (i = 1; i < DLM_CALLBACKS_SIZE; i++) { - if (!lkb->lkb_callbacks[i].seq) - break; - memcpy(&lkb->lkb_callbacks[i-1], &lkb->lkb_callbacks[i], - sizeof(struct dlm_callback)); - memset(&lkb->lkb_callbacks[i], 0, sizeof(struct dlm_callback)); - (*resid)++; + cb->flags = flags; + cb->mode = mode; + cb->sb_status = status; + cb->sb_flags = (sbflags & 0x000000FF); + kref_init(&cb->ref); + if (!(lkb->lkb_flags & DLM_IFL_NEED_SCHED)) { + lkb->lkb_flags |= DLM_IFL_NEED_SCHED; + rv = DLM_ENQUEUE_CALLBACK_NEED_SCHED; } + list_add_tail(&cb->list, &lkb->lkb_callbacks); - /* if cb is a bast, it should be skipped if the blocking mode is - compatible with the last granted mode */ - - if ((cb->flags & DLM_CB_BAST) && lkb->lkb_last_cast.seq) { - if (dlm_modes_compat(cb->mode, lkb->lkb_last_cast.mode)) { - cb->flags |= DLM_CB_SKIP; - - log_debug(ls, "skip %x bast %llu mode %d " - "for cast %llu mode %d", - lkb->lkb_id, - (unsigned long long)cb->seq, - cb->mode, - (unsigned long long)lkb->lkb_last_cast.seq, - lkb->lkb_last_cast.mode); - rv = 0; - goto out; - } - } + if (flags & DLM_CB_CAST) + dlm_callback_set_last_ptr(&lkb->lkb_last_cast, cb); - if (cb->flags & DLM_CB_CAST) - memcpy(&lkb->lkb_last_cast, cb, sizeof(struct dlm_callback)); + dlm_callback_set_last_ptr(&lkb->lkb_last_cb, cb); - if (cb->flags & DLM_CB_BAST) - memcpy(&lkb->lkb_last_bast, cb, sizeof(struct dlm_callback)); - rv = 0; out: return rv; } +int dlm_dequeue_lkb_callback(struct dlm_lkb *lkb, struct dlm_callback **cb) +{ + /* oldest undelivered cb is callbacks first entry */ + *cb = list_first_entry_or_null(&lkb->lkb_callbacks, + struct dlm_callback, list); + if (!*cb) + return DLM_DEQUEUE_CALLBACK_EMPTY; + + /* remove it from callbacks so shift others down */ + list_del(&(*cb)->list); + if (list_empty(&lkb->lkb_callbacks)) + return DLM_DEQUEUE_CALLBACK_LAST; + + return DLM_DEQUEUE_CALLBACK_SUCCESS; +} + void dlm_add_cb(struct dlm_lkb *lkb, uint32_t flags, int mode, int status, uint32_t sbflags) { struct dlm_ls *ls = lkb->lkb_resource->res_ls; - uint64_t new_seq, prev_seq; int rv; - spin_lock(&dlm_cb_seq_spin); - new_seq = ++dlm_cb_seq; - if (!dlm_cb_seq) - new_seq = ++dlm_cb_seq; - spin_unlock(&dlm_cb_seq_spin); - if (lkb->lkb_flags & DLM_IFL_USER) { - dlm_user_add_ast(lkb, flags, mode, status, sbflags, new_seq); + dlm_user_add_ast(lkb, flags, mode, status, sbflags); return; } spin_lock(&lkb->lkb_cb_lock); - prev_seq = lkb->lkb_callbacks[0].seq; - - rv = dlm_add_lkb_callback(lkb, flags, mode, status, sbflags, new_seq); - if (rv < 0) - goto out; - - if (!prev_seq) { + rv = dlm_enqueue_lkb_callback(lkb, flags, mode, status, sbflags); + switch (rv) { + case DLM_ENQUEUE_CALLBACK_NEED_SCHED: kref_get(&lkb->lkb_ref); spin_lock(&ls->ls_cb_lock); @@ -203,8 +159,16 @@ void dlm_add_cb(struct dlm_lkb *lkb, uint32_t flags, int mode, int status, queue_work(ls->ls_callback_wq, &lkb->lkb_cb_work); } spin_unlock(&ls->ls_cb_lock); + break; + case DLM_ENQUEUE_CALLBACK_FAILURE: + WARN_ON(1); + break; + case DLM_ENQUEUE_CALLBACK_SUCCESS: + break; + default: + WARN_ON(1); + break; } - out: spin_unlock(&lkb->lkb_cb_lock); } @@ -214,53 +178,43 @@ void dlm_callback_work(struct work_struct *work) struct dlm_ls *ls = lkb->lkb_resource->res_ls; void (*castfn) (void *astparam); void (*bastfn) (void *astparam, int mode); - struct dlm_callback callbacks[DLM_CALLBACKS_SIZE]; - int i, rv, resid; - - memset(&callbacks, 0, sizeof(callbacks)); + struct dlm_callback *cb; + int rv; spin_lock(&lkb->lkb_cb_lock); - if (!lkb->lkb_callbacks[0].seq) { - /* no callback work exists, shouldn't happen */ - log_error(ls, "dlm_callback_work %x no work", lkb->lkb_id); - dlm_print_lkb(lkb); - dlm_dump_lkb_callbacks(lkb); - } - - for (i = 0; i < DLM_CALLBACKS_SIZE; i++) { - rv = dlm_rem_lkb_callback(ls, lkb, &callbacks[i], &resid); - if (rv < 0) - break; - } - - if (resid) { - /* cbs remain, loop should have removed all, shouldn't happen */ - log_error(ls, "dlm_callback_work %x resid %d", lkb->lkb_id, - resid); - dlm_print_lkb(lkb); - dlm_dump_lkb_callbacks(lkb); - } + rv = dlm_dequeue_lkb_callback(lkb, &cb); spin_unlock(&lkb->lkb_cb_lock); - castfn = lkb->lkb_astfn; - bastfn = lkb->lkb_bastfn; + if (WARN_ON(rv == DLM_DEQUEUE_CALLBACK_EMPTY)) + return; - for (i = 0; i < DLM_CALLBACKS_SIZE; i++) { - if (!callbacks[i].seq) - break; - if (callbacks[i].flags & DLM_CB_SKIP) { - continue; - } else if (callbacks[i].flags & DLM_CB_BAST) { - trace_dlm_bast(ls, lkb, callbacks[i].mode); + for (;;) { + castfn = lkb->lkb_astfn; + bastfn = lkb->lkb_bastfn; + + if (cb->flags & DLM_CB_BAST) { + trace_dlm_bast(ls, lkb, cb->mode); lkb->lkb_last_bast_time = ktime_get(); - bastfn(lkb->lkb_astparam, callbacks[i].mode); - } else if (callbacks[i].flags & DLM_CB_CAST) { - lkb->lkb_lksb->sb_status = callbacks[i].sb_status; - lkb->lkb_lksb->sb_flags = callbacks[i].sb_flags; + lkb->lkb_last_bast_mode = cb->mode; + bastfn(lkb->lkb_astparam, cb->mode); + } else if (cb->flags & DLM_CB_CAST) { + lkb->lkb_lksb->sb_status = cb->sb_status; + lkb->lkb_lksb->sb_flags = cb->sb_flags; trace_dlm_ast(ls, lkb); lkb->lkb_last_cast_time = ktime_get(); castfn(lkb->lkb_astparam); } + + kref_put(&cb->ref, dlm_release_callback); + + spin_lock(&lkb->lkb_cb_lock); + rv = dlm_dequeue_lkb_callback(lkb, &cb); + if (rv == DLM_DEQUEUE_CALLBACK_EMPTY) { + lkb->lkb_flags &= ~DLM_IFL_NEED_SCHED; + spin_unlock(&lkb->lkb_cb_lock); + break; + } + spin_unlock(&lkb->lkb_cb_lock); } /* undo kref_get from dlm_add_callback, may cause lkb to be freed */ diff --git a/fs/dlm/ast.h b/fs/dlm/ast.h index e5e05fcc58138..880b118824953 100644 --- a/fs/dlm/ast.h +++ b/fs/dlm/ast.h @@ -11,13 +11,22 @@ #ifndef __ASTD_DOT_H__ #define __ASTD_DOT_H__ -int dlm_add_lkb_callback(struct dlm_lkb *lkb, uint32_t flags, int mode, - int status, uint32_t sbflags, uint64_t seq); -int dlm_rem_lkb_callback(struct dlm_ls *ls, struct dlm_lkb *lkb, - struct dlm_callback *cb, int *resid); +#define DLM_ENQUEUE_CALLBACK_NEED_SCHED 1 +#define DLM_ENQUEUE_CALLBACK_SUCCESS 0 +#define DLM_ENQUEUE_CALLBACK_FAILURE -1 +int dlm_enqueue_lkb_callback(struct dlm_lkb *lkb, uint32_t flags, int mode, + int status, uint32_t sbflags); +#define DLM_DEQUEUE_CALLBACK_EMPTY 2 +#define DLM_DEQUEUE_CALLBACK_LAST 1 +#define DLM_DEQUEUE_CALLBACK_SUCCESS 0 +int dlm_dequeue_lkb_callback(struct dlm_lkb *lkb, struct dlm_callback **cb); void dlm_add_cb(struct dlm_lkb *lkb, uint32_t flags, int mode, int status, uint32_t sbflags); +void dlm_callback_set_last_ptr(struct dlm_callback **from, + struct dlm_callback *to); +void dlm_release_callback(struct kref *ref); +void dlm_purge_lkb_callbacks(struct dlm_lkb *lkb); void dlm_callback_work(struct work_struct *work); int dlm_callback_start(struct dlm_ls *ls); void dlm_callback_stop(struct dlm_ls *ls); diff --git a/fs/dlm/debug_fs.c b/fs/dlm/debug_fs.c index 8fb04ebbafb5d..8a0e1b1f74ada 100644 --- a/fs/dlm/debug_fs.c +++ b/fs/dlm/debug_fs.c @@ -246,7 +246,7 @@ static void print_format3_lock(struct seq_file *s, struct dlm_lkb *lkb, lkb->lkb_status, lkb->lkb_grmode, lkb->lkb_rqmode, - lkb->lkb_last_bast.mode, + lkb->lkb_last_bast_mode, rsb_lookup, lkb->lkb_wait_type, lkb->lkb_lvbseq, diff --git a/fs/dlm/dlm_internal.h b/fs/dlm/dlm_internal.h index 730808289a424..69e3928c756b6 100644 --- a/fs/dlm/dlm_internal.h +++ b/fs/dlm/dlm_internal.h @@ -211,6 +211,7 @@ struct dlm_args { #endif #define DLM_IFL_DEADLOCK_CANCEL 0x01000000 #define DLM_IFL_STUB_MS 0x02000000 /* magic number for m_flags */ +#define DLM_IFL_NEED_SCHED 0x04000000 /* least significant 2 bytes are message changed, they are full transmitted * but at receive side only the 2 bytes LSB will be set. * @@ -222,18 +223,17 @@ struct dlm_args { #define DLM_IFL_USER 0x00000001 #define DLM_IFL_ORPHAN 0x00000002 -#define DLM_CALLBACKS_SIZE 6 - #define DLM_CB_CAST 0x00000001 #define DLM_CB_BAST 0x00000002 -#define DLM_CB_SKIP 0x00000004 struct dlm_callback { - uint64_t seq; uint32_t flags; /* DLM_CBF_ */ int sb_status; /* copy to lksb status */ uint8_t sb_flags; /* copy to lksb flags */ int8_t mode; /* rq mode of bast, gr mode of cast */ + + struct list_head list; + struct kref ref; }; struct dlm_lkb { @@ -271,9 +271,10 @@ struct dlm_lkb { spinlock_t lkb_cb_lock; struct work_struct lkb_cb_work; struct list_head lkb_cb_list; /* for ls_cb_delay or proc->asts */ - struct dlm_callback lkb_callbacks[DLM_CALLBACKS_SIZE]; - struct dlm_callback lkb_last_cast; - struct dlm_callback lkb_last_bast; + struct list_head lkb_callbacks; + struct dlm_callback *lkb_last_cast; + struct dlm_callback *lkb_last_cb; + int lkb_last_bast_mode; ktime_t lkb_last_cast_time; /* for debugging */ ktime_t lkb_last_bast_time; /* for debugging */ diff --git a/fs/dlm/lock.c b/fs/dlm/lock.c index 40e4e4a1c5823..f4b2fb17bbb14 100644 --- a/fs/dlm/lock.c +++ b/fs/dlm/lock.c @@ -1209,6 +1209,7 @@ static int _create_lkb(struct dlm_ls *ls, struct dlm_lkb **lkb_ret, if (!lkb) return -ENOMEM; + lkb->lkb_last_bast_mode = -1; lkb->lkb_nodeid = -1; lkb->lkb_grmode = DLM_LOCK_IV; kref_init(&lkb->lkb_ref); @@ -1218,6 +1219,7 @@ static int _create_lkb(struct dlm_ls *ls, struct dlm_lkb **lkb_ret, INIT_LIST_HEAD(&lkb->lkb_time_list); #endif INIT_LIST_HEAD(&lkb->lkb_cb_list); + INIT_LIST_HEAD(&lkb->lkb_callbacks); spin_lock_init(&lkb->lkb_cb_lock); INIT_WORK(&lkb->lkb_cb_work, dlm_callback_work); @@ -6221,8 +6223,7 @@ void dlm_clear_proc_locks(struct dlm_ls *ls, struct dlm_user_proc *proc) } list_for_each_entry_safe(lkb, safe, &proc->asts, lkb_cb_list) { - memset(&lkb->lkb_callbacks, 0, - sizeof(struct dlm_callback) * DLM_CALLBACKS_SIZE); + dlm_purge_lkb_callbacks(lkb); list_del_init(&lkb->lkb_cb_list); dlm_put_lkb(lkb); } @@ -6263,8 +6264,7 @@ static void purge_proc_locks(struct dlm_ls *ls, struct dlm_user_proc *proc) spin_lock(&proc->asts_spin); list_for_each_entry_safe(lkb, safe, &proc->asts, lkb_cb_list) { - memset(&lkb->lkb_callbacks, 0, - sizeof(struct dlm_callback) * DLM_CALLBACKS_SIZE); + dlm_purge_lkb_callbacks(lkb); list_del_init(&lkb->lkb_cb_list); dlm_put_lkb(lkb); } diff --git a/fs/dlm/memory.c b/fs/dlm/memory.c index ce35c3c19aeb5..61fe0d1f56467 100644 --- a/fs/dlm/memory.c +++ b/fs/dlm/memory.c @@ -14,12 +14,14 @@ #include "lowcomms.h" #include "config.h" #include "memory.h" +#include "ast.h" static struct kmem_cache *writequeue_cache; static struct kmem_cache *mhandle_cache; static struct kmem_cache *msg_cache; static struct kmem_cache *lkb_cache; static struct kmem_cache *rsb_cache; +static struct kmem_cache *cb_cache; int __init dlm_memory_init(void) @@ -46,8 +48,16 @@ int __init dlm_memory_init(void) if (!rsb_cache) goto rsb; + cb_cache = kmem_cache_create("dlm_cb", sizeof(struct dlm_callback), + __alignof__(struct dlm_callback), 0, + NULL); + if (!rsb_cache) + goto cb; + return 0; +cb: + kmem_cache_destroy(rsb_cache); rsb: kmem_cache_destroy(msg_cache); msg: @@ -67,6 +77,7 @@ void dlm_memory_exit(void) kmem_cache_destroy(msg_cache); kmem_cache_destroy(lkb_cache); kmem_cache_destroy(rsb_cache); + kmem_cache_destroy(cb_cache); } char *dlm_allocate_lvb(struct dlm_ls *ls) @@ -115,6 +126,11 @@ void dlm_free_lkb(struct dlm_lkb *lkb) kfree(ua); } } + + /* drop references if they are set */ + dlm_callback_set_last_ptr(&lkb->lkb_last_cast, NULL); + dlm_callback_set_last_ptr(&lkb->lkb_last_cb, NULL); + kmem_cache_free(lkb_cache, lkb); } @@ -147,3 +163,13 @@ void dlm_free_msg(struct dlm_msg *msg) { kmem_cache_free(msg_cache, msg); } + +struct dlm_callback *dlm_allocate_cb(void) +{ + return kmem_cache_alloc(cb_cache, GFP_ATOMIC); +} + +void dlm_free_cb(struct dlm_callback *cb) +{ + kmem_cache_free(cb_cache, cb); +} diff --git a/fs/dlm/memory.h b/fs/dlm/memory.h index 7bd3f1a391ca7..c1583ec8b2cfe 100644 --- a/fs/dlm/memory.h +++ b/fs/dlm/memory.h @@ -26,6 +26,8 @@ struct writequeue_entry *dlm_allocate_writequeue(void); void dlm_free_writequeue(struct writequeue_entry *writequeue); struct dlm_msg *dlm_allocate_msg(gfp_t allocation); void dlm_free_msg(struct dlm_msg *msg); +struct dlm_callback *dlm_allocate_cb(void); +void dlm_free_cb(struct dlm_callback *cb); #endif /* __MEMORY_DOT_H__ */ diff --git a/fs/dlm/user.c b/fs/dlm/user.c index 6a5de0918a962..6b530db4bc0b2 100644 --- a/fs/dlm/user.c +++ b/fs/dlm/user.c @@ -25,6 +25,7 @@ #include "user.h" #include "ast.h" #include "config.h" +#include "memory.h" static const char name_prefix[] = "dlm"; static const struct file_operations device_fops; @@ -175,7 +176,7 @@ static int lkb_is_endoflife(int mode, int status) being removed and then remove that lkb from the orphans list and free it */ void dlm_user_add_ast(struct dlm_lkb *lkb, uint32_t flags, int mode, - int status, uint32_t sbflags, uint64_t seq) + int status, uint32_t sbflags) { struct dlm_ls *ls; struct dlm_user_args *ua; @@ -209,16 +210,22 @@ void dlm_user_add_ast(struct dlm_lkb *lkb, uint32_t flags, int mode, spin_lock(&proc->asts_spin); - rv = dlm_add_lkb_callback(lkb, flags, mode, status, sbflags, seq); - if (rv < 0) { + rv = dlm_enqueue_lkb_callback(lkb, flags, mode, status, sbflags); + switch (rv) { + case DLM_ENQUEUE_CALLBACK_FAILURE: spin_unlock(&proc->asts_spin); + WARN_ON(1); goto out; - } - - if (list_empty(&lkb->lkb_cb_list)) { + case DLM_ENQUEUE_CALLBACK_NEED_SCHED: kref_get(&lkb->lkb_ref); list_add_tail(&lkb->lkb_cb_list, &proc->asts); wake_up_interruptible(&proc->wait); + break; + case DLM_ENQUEUE_CALLBACK_SUCCESS: + break; + default: + WARN_ON(1); + break; } spin_unlock(&proc->asts_spin); @@ -800,8 +807,8 @@ static ssize_t device_read(struct file *file, char __user *buf, size_t count, struct dlm_user_proc *proc = file->private_data; struct dlm_lkb *lkb; DECLARE_WAITQUEUE(wait, current); - struct dlm_callback cb; - int rv, resid, copy_lvb = 0; + struct dlm_callback *cb; + int rv, copy_lvb = 0; int old_mode, new_mode; if (count == sizeof(struct dlm_device_version)) { @@ -860,50 +867,56 @@ static ssize_t device_read(struct file *file, char __user *buf, size_t count, lkb = list_first_entry(&proc->asts, struct dlm_lkb, lkb_cb_list); /* rem_lkb_callback sets a new lkb_last_cast */ - old_mode = lkb->lkb_last_cast.mode; + old_mode = lkb->lkb_last_cast->mode; - rv = dlm_rem_lkb_callback(lkb->lkb_resource->res_ls, lkb, &cb, &resid); - if (rv < 0) { + rv = dlm_dequeue_lkb_callback(lkb, &cb); + switch (rv) { + case DLM_DEQUEUE_CALLBACK_EMPTY: /* this shouldn't happen; lkb should have been removed from - list when resid was zero */ + * list when last item was dequeued + */ log_print("dlm_rem_lkb_callback empty %x", lkb->lkb_id); list_del_init(&lkb->lkb_cb_list); spin_unlock(&proc->asts_spin); /* removes ref for proc->asts, may cause lkb to be freed */ dlm_put_lkb(lkb); + WARN_ON(1); goto try_another; - } - if (!resid) + case DLM_DEQUEUE_CALLBACK_LAST: list_del_init(&lkb->lkb_cb_list); - spin_unlock(&proc->asts_spin); - - if (cb.flags & DLM_CB_SKIP) { - /* removes ref for proc->asts, may cause lkb to be freed */ - if (!resid) - dlm_put_lkb(lkb); - goto try_another; + /* TODO */ + lkb->lkb_flags &= ~DLM_IFL_NEED_SCHED; + break; + case DLM_DEQUEUE_CALLBACK_SUCCESS: + break; + default: + WARN_ON(1); + break; } + spin_unlock(&proc->asts_spin); - if (cb.flags & DLM_CB_BAST) { - trace_dlm_bast(lkb->lkb_resource->res_ls, lkb, cb.mode); - } else if (cb.flags & DLM_CB_CAST) { - new_mode = cb.mode; + if (cb->flags & DLM_CB_BAST) { + trace_dlm_bast(lkb->lkb_resource->res_ls, lkb, cb->mode); + } else if (cb->flags & DLM_CB_CAST) { + new_mode = cb->mode; - if (!cb.sb_status && lkb->lkb_lksb->sb_lvbptr && + if (!cb->sb_status && lkb->lkb_lksb->sb_lvbptr && dlm_lvb_operations[old_mode + 1][new_mode + 1]) copy_lvb = 1; - lkb->lkb_lksb->sb_status = cb.sb_status; - lkb->lkb_lksb->sb_flags = cb.sb_flags; + lkb->lkb_lksb->sb_status = cb->sb_status; + lkb->lkb_lksb->sb_flags = cb->sb_flags; trace_dlm_ast(lkb->lkb_resource->res_ls, lkb); } rv = copy_result_to_user(lkb->lkb_ua, test_bit(DLM_PROC_FLAGS_COMPAT, &proc->flags), - cb.flags, cb.mode, copy_lvb, buf, count); + cb->flags, cb->mode, copy_lvb, buf, count); + + kref_put(&cb->ref, dlm_release_callback); /* removes ref for proc->asts, may cause lkb to be freed */ - if (!resid) + if (rv == DLM_DEQUEUE_CALLBACK_LAST) dlm_put_lkb(lkb); return rv; diff --git a/fs/dlm/user.h b/fs/dlm/user.h index 6b9bce6b96e08..33059452d79e0 100644 --- a/fs/dlm/user.h +++ b/fs/dlm/user.h @@ -7,7 +7,7 @@ #define __USER_DOT_H__ void dlm_user_add_ast(struct dlm_lkb *lkb, uint32_t flags, int mode, - int status, uint32_t sbflags, uint64_t seq); + int status, uint32_t sbflags); int dlm_user_init(void); void dlm_user_exit(void); int dlm_device_deregister(struct dlm_ls *ls); From e1711fe3fd59fa1e34e02add2e9c188441c12021 Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Thu, 27 Oct 2022 16:45:22 -0400 Subject: [PATCH 13/37] fs: dlm: allow different allocation context per _create_message This patch allows to give the use control about the allocation context based on a per message basis. Currently all messages forced to be created under GFP_NOFS context. Signed-off-by: Alexander Aring Signed-off-by: David Teigland --- fs/dlm/lock.c | 31 +++++++++++++++++++------------ fs/dlm/memory.c | 4 ++-- fs/dlm/memory.h | 2 +- fs/dlm/midcomms.c | 2 +- 4 files changed, 23 insertions(+), 16 deletions(-) diff --git a/fs/dlm/lock.c b/fs/dlm/lock.c index f4b2fb17bbb14..a2930e33c1347 100644 --- a/fs/dlm/lock.c +++ b/fs/dlm/lock.c @@ -3554,7 +3554,8 @@ int dlm_unlock(dlm_lockspace_t *lockspace, static int _create_message(struct dlm_ls *ls, int mb_len, int to_nodeid, int mstype, struct dlm_message **ms_ret, - struct dlm_mhandle **mh_ret) + struct dlm_mhandle **mh_ret, + gfp_t allocation) { struct dlm_message *ms; struct dlm_mhandle *mh; @@ -3564,7 +3565,7 @@ static int _create_message(struct dlm_ls *ls, int mb_len, pass into midcomms_commit and a message buffer (mb) that we write our data into */ - mh = dlm_midcomms_get_mhandle(to_nodeid, mb_len, GFP_NOFS, &mb); + mh = dlm_midcomms_get_mhandle(to_nodeid, mb_len, allocation, &mb); if (!mh) return -ENOBUFS; @@ -3586,7 +3587,8 @@ static int _create_message(struct dlm_ls *ls, int mb_len, static int create_message(struct dlm_rsb *r, struct dlm_lkb *lkb, int to_nodeid, int mstype, struct dlm_message **ms_ret, - struct dlm_mhandle **mh_ret) + struct dlm_mhandle **mh_ret, + gfp_t allocation) { int mb_len = sizeof(struct dlm_message); @@ -3607,7 +3609,7 @@ static int create_message(struct dlm_rsb *r, struct dlm_lkb *lkb, } return _create_message(r->res_ls, mb_len, to_nodeid, mstype, - ms_ret, mh_ret); + ms_ret, mh_ret, allocation); } /* further lowcomms enhancements or alternate implementations may make @@ -3676,7 +3678,7 @@ static int send_common(struct dlm_rsb *r, struct dlm_lkb *lkb, int mstype) if (error) return error; - error = create_message(r, lkb, to_nodeid, mstype, &ms, &mh); + error = create_message(r, lkb, to_nodeid, mstype, &ms, &mh, GFP_NOFS); if (error) goto fail; @@ -3737,7 +3739,8 @@ static int send_grant(struct dlm_rsb *r, struct dlm_lkb *lkb) to_nodeid = lkb->lkb_nodeid; - error = create_message(r, lkb, to_nodeid, DLM_MSG_GRANT, &ms, &mh); + error = create_message(r, lkb, to_nodeid, DLM_MSG_GRANT, &ms, &mh, + GFP_NOFS); if (error) goto out; @@ -3758,7 +3761,8 @@ static int send_bast(struct dlm_rsb *r, struct dlm_lkb *lkb, int mode) to_nodeid = lkb->lkb_nodeid; - error = create_message(r, NULL, to_nodeid, DLM_MSG_BAST, &ms, &mh); + error = create_message(r, NULL, to_nodeid, DLM_MSG_BAST, &ms, &mh, + GFP_NOFS); if (error) goto out; @@ -3783,7 +3787,8 @@ static int send_lookup(struct dlm_rsb *r, struct dlm_lkb *lkb) if (error) return error; - error = create_message(r, NULL, to_nodeid, DLM_MSG_LOOKUP, &ms, &mh); + error = create_message(r, NULL, to_nodeid, DLM_MSG_LOOKUP, &ms, &mh, + GFP_NOFS); if (error) goto fail; @@ -3807,7 +3812,8 @@ static int send_remove(struct dlm_rsb *r) to_nodeid = dlm_dir_nodeid(r); - error = create_message(r, NULL, to_nodeid, DLM_MSG_REMOVE, &ms, &mh); + error = create_message(r, NULL, to_nodeid, DLM_MSG_REMOVE, &ms, &mh, + GFP_NOFS); if (error) goto out; @@ -3828,7 +3834,7 @@ static int send_common_reply(struct dlm_rsb *r, struct dlm_lkb *lkb, to_nodeid = lkb->lkb_nodeid; - error = create_message(r, lkb, to_nodeid, mstype, &ms, &mh); + error = create_message(r, lkb, to_nodeid, mstype, &ms, &mh, GFP_NOFS); if (error) goto out; @@ -3869,7 +3875,8 @@ static int send_lookup_reply(struct dlm_ls *ls, struct dlm_message *ms_in, struct dlm_mhandle *mh; int error, nodeid = le32_to_cpu(ms_in->m_header.h_nodeid); - error = create_message(r, NULL, nodeid, DLM_MSG_LOOKUP_REPLY, &ms, &mh); + error = create_message(r, NULL, nodeid, DLM_MSG_LOOKUP_REPLY, &ms, &mh, + GFP_NOFS); if (error) goto out; @@ -6295,7 +6302,7 @@ static int send_purge(struct dlm_ls *ls, int nodeid, int pid) int error; error = _create_message(ls, sizeof(struct dlm_message), nodeid, - DLM_MSG_PURGE, &ms, &mh); + DLM_MSG_PURGE, &ms, &mh, GFP_NOFS); if (error) return error; ms->m_nodeid = cpu_to_le32(nodeid); diff --git a/fs/dlm/memory.c b/fs/dlm/memory.c index 61fe0d1f56467..eb7a08641fcf5 100644 --- a/fs/dlm/memory.c +++ b/fs/dlm/memory.c @@ -134,9 +134,9 @@ void dlm_free_lkb(struct dlm_lkb *lkb) kmem_cache_free(lkb_cache, lkb); } -struct dlm_mhandle *dlm_allocate_mhandle(void) +struct dlm_mhandle *dlm_allocate_mhandle(gfp_t allocation) { - return kmem_cache_alloc(mhandle_cache, GFP_NOFS); + return kmem_cache_alloc(mhandle_cache, allocation); } void dlm_free_mhandle(struct dlm_mhandle *mhandle) diff --git a/fs/dlm/memory.h b/fs/dlm/memory.h index c1583ec8b2cfe..6b29563d24f78 100644 --- a/fs/dlm/memory.h +++ b/fs/dlm/memory.h @@ -20,7 +20,7 @@ struct dlm_lkb *dlm_allocate_lkb(struct dlm_ls *ls); void dlm_free_lkb(struct dlm_lkb *l); char *dlm_allocate_lvb(struct dlm_ls *ls); void dlm_free_lvb(char *l); -struct dlm_mhandle *dlm_allocate_mhandle(void); +struct dlm_mhandle *dlm_allocate_mhandle(gfp_t allocation); void dlm_free_mhandle(struct dlm_mhandle *mhandle); struct writequeue_entry *dlm_allocate_writequeue(void); void dlm_free_writequeue(struct writequeue_entry *writequeue); diff --git a/fs/dlm/midcomms.c b/fs/dlm/midcomms.c index e10a6e97df443..3a4e20d6cd2dc 100644 --- a/fs/dlm/midcomms.c +++ b/fs/dlm/midcomms.c @@ -1097,7 +1097,7 @@ struct dlm_mhandle *dlm_midcomms_get_mhandle(int nodeid, int len, /* this is a bug, however we going on and hope it will be resolved */ WARN_ON(test_bit(DLM_NODE_FLAG_STOP_TX, &node->flags)); - mh = dlm_allocate_mhandle(); + mh = dlm_allocate_mhandle(allocation); if (!mh) goto err; From 3872f87b09e2f274ecf477895f9d1f9b9bdcf04b Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Thu, 27 Oct 2022 16:45:23 -0400 Subject: [PATCH 14/37] fs: dlm: remove ls_remove_wait waitqueue This patch removes the ls_remove_wait waitqueue handling. The current handling tries to wait before a lookup is send out for a identically resource name which is going to be removed. Hereby the remove message should be send out before the new lookup message. The reason is that after a lookup request and response will actually use the specific remote rsb. A followed remove message would delete the rsb on the remote side but it's still being used. To reach a similar behaviour we simple send the remove message out while the rsb lookup lock is held and the rsb is removed from the toss list. Other find_rsb() calls would never have the change to get a rsb back to live while a remove message will be send out (without holding the lock). This behaviour requires a non-sleepable context which should be provided now and might be the reason why it was not implemented so in the first place. Signed-off-by: Alexander Aring Signed-off-by: David Teigland --- fs/dlm/dlm_internal.h | 4 ---- fs/dlm/lock.c | 56 ++----------------------------------------- fs/dlm/lockspace.c | 3 --- 3 files changed, 2 insertions(+), 61 deletions(-) diff --git a/fs/dlm/dlm_internal.h b/fs/dlm/dlm_internal.h index 69e3928c756b6..6318e0f51bc90 100644 --- a/fs/dlm/dlm_internal.h +++ b/fs/dlm/dlm_internal.h @@ -592,11 +592,7 @@ struct dlm_ls { int ls_new_rsb_count; struct list_head ls_new_rsb; /* new rsb structs */ - spinlock_t ls_remove_spin; - wait_queue_head_t ls_remove_wait; - char ls_remove_name[DLM_RESNAME_MAXLEN+1]; char *ls_remove_names[DLM_REMOVE_NAMES_MAX]; - int ls_remove_len; int ls_remove_lens[DLM_REMOVE_NAMES_MAX]; struct list_head ls_nodes; /* current nodes in ls */ diff --git a/fs/dlm/lock.c b/fs/dlm/lock.c index a2930e33c1347..e1adfa5aed051 100644 --- a/fs/dlm/lock.c +++ b/fs/dlm/lock.c @@ -1589,37 +1589,6 @@ static int remove_from_waiters_ms(struct dlm_lkb *lkb, struct dlm_message *ms) return error; } -/* If there's an rsb for the same resource being removed, ensure - * that the remove message is sent before the new lookup message. - */ - -#define DLM_WAIT_PENDING_COND(ls, r) \ - (ls->ls_remove_len && \ - !rsb_cmp(r, ls->ls_remove_name, \ - ls->ls_remove_len)) - -static void wait_pending_remove(struct dlm_rsb *r) -{ - struct dlm_ls *ls = r->res_ls; - restart: - spin_lock(&ls->ls_remove_spin); - if (DLM_WAIT_PENDING_COND(ls, r)) { - log_debug(ls, "delay lookup for remove dir %d %s", - r->res_dir_nodeid, r->res_name); - spin_unlock(&ls->ls_remove_spin); - wait_event(ls->ls_remove_wait, !DLM_WAIT_PENDING_COND(ls, r)); - goto restart; - } - spin_unlock(&ls->ls_remove_spin); -} - -/* - * ls_remove_spin protects ls_remove_name and ls_remove_len which are - * read by other threads in wait_pending_remove. ls_remove_names - * and ls_remove_lens are only used by the scan thread, so they do - * not need protection. - */ - static void shrink_bucket(struct dlm_ls *ls, int b) { struct rb_node *n, *next; @@ -1701,11 +1670,6 @@ static void shrink_bucket(struct dlm_ls *ls, int b) * list and sending the removal. Keeping this gap small is * important to keep us (the master node) from being out of sync * with the remote dir node for very long. - * - * From the time the rsb is removed from toss until just after - * send_remove, the rsb name is saved in ls_remove_name. A new - * lookup checks this to ensure that a new lookup message for the - * same resource name is not sent just before the remove message. */ for (i = 0; i < remote_count; i++) { @@ -1752,22 +1716,8 @@ static void shrink_bucket(struct dlm_ls *ls, int b) } rb_erase(&r->res_hashnode, &ls->ls_rsbtbl[b].toss); - - /* block lookup of same name until we've sent remove */ - spin_lock(&ls->ls_remove_spin); - ls->ls_remove_len = len; - memcpy(ls->ls_remove_name, name, DLM_RESNAME_MAXLEN); - spin_unlock(&ls->ls_remove_spin); - spin_unlock(&ls->ls_rsbtbl[b].lock); - send_remove(r); - - /* allow lookup of name again */ - spin_lock(&ls->ls_remove_spin); - ls->ls_remove_len = 0; - memset(ls->ls_remove_name, 0, DLM_RESNAME_MAXLEN); - spin_unlock(&ls->ls_remove_spin); - wake_up(&ls->ls_remove_wait); + spin_unlock(&ls->ls_rsbtbl[b].lock); dlm_free_rsb(r); } @@ -2718,8 +2668,6 @@ static int set_master(struct dlm_rsb *r, struct dlm_lkb *lkb) return 0; } - wait_pending_remove(r); - r->res_first_lkid = lkb->lkb_id; send_lookup(r, lkb); return 1; @@ -3813,7 +3761,7 @@ static int send_remove(struct dlm_rsb *r) to_nodeid = dlm_dir_nodeid(r); error = create_message(r, NULL, to_nodeid, DLM_MSG_REMOVE, &ms, &mh, - GFP_NOFS); + GFP_ATOMIC); if (error) goto out; diff --git a/fs/dlm/lockspace.c b/fs/dlm/lockspace.c index 4965737705b72..e7135883cf783 100644 --- a/fs/dlm/lockspace.c +++ b/fs/dlm/lockspace.c @@ -524,9 +524,6 @@ static int new_lockspace(const char *name, const char *cluster, spin_lock_init(&ls->ls_rsbtbl[i].lock); } - spin_lock_init(&ls->ls_remove_spin); - init_waitqueue_head(&ls->ls_remove_wait); - for (i = 0; i < DLM_REMOVE_NAMES_MAX; i++) { ls->ls_remove_names[i] = kzalloc(DLM_RESNAME_MAXLEN+1, GFP_KERNEL); From 194a3fb488f2760eda67c3ab0ce3a095e8006d72 Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Thu, 27 Oct 2022 16:45:24 -0400 Subject: [PATCH 15/37] fs: dlm: relax sending to allow receiving This patch drops additionally the sock_mutex when there is a sending message burst. Since we have acknowledge handling we free sending buffers only when we receive an ack back, but if we are stuck in send_to_sock() looping because dlm sends a lot of messages and we never leave the loop the sending buffer fill up very quickly. We can't receive during this iteration because the sock_mutex is held. This patch will unlock the sock_mutex so it should be possible to receive messages when a burst of sending messages happens. This will allow to free up memory because acks which are already received can be processed. Signed-off-by: Alexander Aring Signed-off-by: David Teigland --- fs/dlm/lowcomms.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c index 871d4e9f49fb6..b05c6d9b5102c 100644 --- a/fs/dlm/lowcomms.c +++ b/fs/dlm/lowcomms.c @@ -1418,7 +1418,10 @@ static void send_to_sock(struct connection *con) const int msg_flags = MSG_DONTWAIT | MSG_NOSIGNAL; struct writequeue_entry *e; int len, offset, ret; - int count = 0; + int count; + +again: + count = 0; mutex_lock(&con->sock_mutex); if (con->sock == NULL) @@ -1453,14 +1456,16 @@ static void send_to_sock(struct connection *con) } else if (ret < 0) goto out; + spin_lock(&con->writequeue_lock); + writequeue_entry_complete(e, ret); + /* Don't starve people filling buffers */ if (++count >= MAX_SEND_MSG_COUNT) { + spin_unlock(&con->writequeue_lock); + mutex_unlock(&con->sock_mutex); cond_resched(); - count = 0; + goto again; } - - spin_lock(&con->writequeue_lock); - writequeue_entry_complete(e, ret); } spin_unlock(&con->writequeue_lock); From 9c693d76abb3956433a28994924046abf1a73ef4 Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Thu, 27 Oct 2022 16:45:25 -0400 Subject: [PATCH 16/37] fs: dlm: catch dlm_add_member() error This patch will catch a possible dlm_add_member() and delivers it to the dlm recovery handling. Signed-off-by: Alexander Aring Signed-off-by: David Teigland --- fs/dlm/member.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/fs/dlm/member.c b/fs/dlm/member.c index 2af2ccfe43a9d..923c01a8a0aa2 100644 --- a/fs/dlm/member.c +++ b/fs/dlm/member.c @@ -573,7 +573,10 @@ int dlm_recover_members(struct dlm_ls *ls, struct dlm_recover *rv, int *neg_out) node = &rv->nodes[i]; if (dlm_is_member(ls, node->nodeid)) continue; - dlm_add_member(ls, node); + error = dlm_add_member(ls, node); + if (error) + return error; + log_rinfo(ls, "add member %d", node->nodeid); } From 3e54c9e80e68b765d8877023d93f1eea1b9d1c54 Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Thu, 27 Oct 2022 16:45:26 -0400 Subject: [PATCH 17/37] fs: dlm: fix log of lowcomms vs midcomms This patch will fix a small issue when printing out that dlm_midcomms_start() failed to start and it was printing out that the dlm subcomponent lowcomms was failed but lowcomms is behind the midcomms layer. Signed-off-by: Alexander Aring Signed-off-by: David Teigland --- fs/dlm/lockspace.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/dlm/lockspace.c b/fs/dlm/lockspace.c index e7135883cf783..96fe8f7ca09cd 100644 --- a/fs/dlm/lockspace.c +++ b/fs/dlm/lockspace.c @@ -391,7 +391,7 @@ static int threads_start(void) /* Thread for sending/receiving messages for all lockspace's */ error = dlm_midcomms_start(); if (error) { - log_print("cannot start dlm lowcomms %d", error); + log_print("cannot start dlm midcomms %d", error); goto scand_fail; } From 775af207464bd28a2086f8399c0b2a3f1f40c7ae Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Thu, 27 Oct 2022 16:45:27 -0400 Subject: [PATCH 18/37] fs: dlm: use WARN_ON_ONCE() instead of WARN_ON() To not get the console spammed about WARN_ON() of invalid states in the dlm midcomms hot path handling we switch to WARN_ON_ONCE() to get it only once that there might be an issue with the midcomms state handling. Signed-off-by: Alexander Aring Signed-off-by: David Teigland --- fs/dlm/midcomms.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/fs/dlm/midcomms.c b/fs/dlm/midcomms.c index 3a4e20d6cd2dc..32194a750fe10 100644 --- a/fs/dlm/midcomms.c +++ b/fs/dlm/midcomms.c @@ -469,7 +469,7 @@ static void dlm_pas_fin_ack_rcv(struct midcomms_node *node) spin_unlock(&node->state_lock); log_print("%s: unexpected state: %d\n", __func__, node->state); - WARN_ON(1); + WARN_ON_ONCE(1); return; } spin_unlock(&node->state_lock); @@ -540,7 +540,7 @@ static void dlm_midcomms_receive_buffer(union dlm_packet *p, spin_unlock(&node->state_lock); log_print("%s: unexpected state: %d\n", __func__, node->state); - WARN_ON(1); + WARN_ON_ONCE(1); return; } spin_unlock(&node->state_lock); @@ -548,7 +548,7 @@ static void dlm_midcomms_receive_buffer(union dlm_packet *p, set_bit(DLM_NODE_FLAG_STOP_RX, &node->flags); break; default: - WARN_ON(test_bit(DLM_NODE_FLAG_STOP_RX, &node->flags)); + WARN_ON_ONCE(test_bit(DLM_NODE_FLAG_STOP_RX, &node->flags)); dlm_receive_buffer_3_2_trace(seq, p); dlm_receive_buffer(p, node->nodeid); set_bit(DLM_NODE_ULP_DELIVERED, &node->flags); @@ -770,7 +770,7 @@ static void dlm_midcomms_receive_buffer_3_2(union dlm_packet *p, int nodeid) goto out; } - WARN_ON(test_bit(DLM_NODE_FLAG_STOP_RX, &node->flags)); + WARN_ON_ONCE(test_bit(DLM_NODE_FLAG_STOP_RX, &node->flags)); dlm_receive_buffer(p, nodeid); break; case DLM_OPTS: @@ -1095,7 +1095,7 @@ struct dlm_mhandle *dlm_midcomms_get_mhandle(int nodeid, int len, } /* this is a bug, however we going on and hope it will be resolved */ - WARN_ON(test_bit(DLM_NODE_FLAG_STOP_TX, &node->flags)); + WARN_ON_ONCE(test_bit(DLM_NODE_FLAG_STOP_TX, &node->flags)); mh = dlm_allocate_mhandle(allocation); if (!mh) @@ -1127,7 +1127,7 @@ struct dlm_mhandle *dlm_midcomms_get_mhandle(int nodeid, int len, break; default: dlm_free_mhandle(mh); - WARN_ON(1); + WARN_ON_ONCE(1); goto err; } @@ -1196,7 +1196,7 @@ void dlm_midcomms_commit_mhandle(struct dlm_mhandle *mh, break; default: srcu_read_unlock(&nodes_srcu, mh->idx); - WARN_ON(1); + WARN_ON_ONCE(1); break; } } @@ -1238,7 +1238,7 @@ static void dlm_act_fin_ack_rcv(struct midcomms_node *node) spin_unlock(&node->state_lock); log_print("%s: unexpected state: %d\n", __func__, node->state); - WARN_ON(1); + WARN_ON_ONCE(1); return; } spin_unlock(&node->state_lock); @@ -1356,7 +1356,7 @@ static void midcomms_node_release(struct rcu_head *rcu) { struct midcomms_node *node = container_of(rcu, struct midcomms_node, rcu); - WARN_ON(atomic_read(&node->send_queue_cnt)); + WARN_ON_ONCE(atomic_read(&node->send_queue_cnt)); kfree(node); } From f217d7ccb9f9ae8d9525958f902c059db1c78355 Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Thu, 17 Nov 2022 17:11:40 -0500 Subject: [PATCH 19/37] fs: dlm: avoid false-positive checker warning This patch avoid the false-positive checker warning about writing 112 bytes into a 88 bytes field "e->request", see: [ 54.891560] dlm: csmb1: dlm_recover_directory 23 out 2 messages [ 54.990542] ------------[ cut here ]------------ [ 54.991012] memcpy: detected field-spanning write (size 112) of single field "&e->request" at fs/dlm/requestqueue.c:47 (size 88) [ 54.992150] WARNING: CPU: 0 PID: 297 at fs/dlm/requestqueue.c:47 dlm_add_requestqueue+0x177/0x180 [ 54.993002] CPU: 0 PID: 297 Comm: kworker/u4:3 Not tainted 6.1.0-rc5-00008-ge01d50cbd6ee #248 [ 54.993878] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.0-1.fc36 04/01/2014 [ 54.994718] Workqueue: dlm_recv process_recv_sockets [ 54.995230] RIP: 0010:dlm_add_requestqueue+0x177/0x180 [ 54.995731] Code: e7 01 0f 85 3b ff ff ff b9 58 00 00 00 48 c7 c2 c0 41 74 82 4c 89 ee 48 c7 c7 20 42 74 82 c6 05 8b 8d 30 02 01 e8 51 07 be 00 <0f> 0b e9 12 ff ff ff 66 90 0f 1f 44 00 00 41 57 48 8d 87 10 08 00 [ 54.997483] RSP: 0018:ffffc90000b1fbe8 EFLAGS: 00010282 [ 54.997990] RAX: 0000000000000000 RBX: ffff888024fc3d00 RCX: 0000000000000000 [ 54.998667] RDX: 0000000000000001 RSI: ffffffff81155014 RDI: fffff52000163f73 [ 54.999342] RBP: ffff88800dbac000 R08: 0000000000000001 R09: ffffc90000b1fa5f [ 54.999997] R10: fffff52000163f4b R11: 203a7970636d656d R12: ffff88800cfb0018 [ 55.000673] R13: 0000000000000070 R14: ffff888024fc3d18 R15: 0000000000000000 [ 55.001344] FS: 0000000000000000(0000) GS:ffff88806d600000(0000) knlGS:0000000000000000 [ 55.002078] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 55.002603] CR2: 00007f35d4f0b9a0 CR3: 0000000025495002 CR4: 0000000000770ef0 [ 55.003258] PKRU: 55555554 [ 55.003514] Call Trace: [ 55.003756] [ 55.003953] dlm_receive_buffer+0x1c0/0x200 [ 55.004348] dlm_process_incoming_buffer+0x46d/0x780 [ 55.004786] ? kernel_recvmsg+0x8b/0xc0 [ 55.005150] receive_from_sock.isra.0+0x168/0x420 [ 55.005582] ? process_listen_recv_socket+0x10/0x10 [ 55.006018] ? finish_task_switch.isra.0+0xe0/0x400 [ 55.006469] ? __switch_to+0x2fe/0x6a0 [ 55.006808] ? read_word_at_a_time+0xe/0x20 [ 55.007197] ? strscpy+0x146/0x190 [ 55.007505] process_one_work+0x3d0/0x6b0 [ 55.007863] worker_thread+0x8d/0x620 [ 55.008209] ? __kthread_parkme+0xd8/0xf0 [ 55.008565] ? process_one_work+0x6b0/0x6b0 [ 55.008937] kthread+0x171/0x1a0 [ 55.009251] ? kthread_exit+0x60/0x60 [ 55.009582] ret_from_fork+0x1f/0x30 [ 55.009903] [ 55.010120] ---[ end trace 0000000000000000 ]--- [ 55.025783] dlm: csmb1: dlm_recover 5 generation 3 done: 201 ms [ 55.026466] gfs2: fsid=smbcluster:csmb1.0: recover generation 3 done It seems the checker is unable to detect the additional length bytes which was allocated additionally for the flexible array in struct dlm_message. To solve it we split the memcpy() into copy for the 88 bytes struct and another memcpy() for the flexible array m_extra field. Signed-off-by: Alexander Aring Signed-off-by: David Teigland --- fs/dlm/requestqueue.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fs/dlm/requestqueue.c b/fs/dlm/requestqueue.c index 036a9a0078f66..8be2893ad15bb 100644 --- a/fs/dlm/requestqueue.c +++ b/fs/dlm/requestqueue.c @@ -44,7 +44,8 @@ void dlm_add_requestqueue(struct dlm_ls *ls, int nodeid, struct dlm_message *ms) e->recover_seq = ls->ls_recover_seq & 0xFFFFFFFF; e->nodeid = nodeid; - memcpy(&e->request, ms, le16_to_cpu(ms->m_header.h_length)); + memcpy(&e->request, ms, sizeof(*ms)); + memcpy(&e->request.m_extra, ms->m_extra, length); atomic_inc(&ls->ls_requestqueue_cnt); mutex_lock(&ls->ls_requestqueue_mutex); From 9267c85769e62c10961451fd28e88de996fdf401 Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Thu, 17 Nov 2022 17:11:41 -0500 Subject: [PATCH 20/37] fs: dlm: drop lkb ref in bug case This patch will drop the lkb reference in an very unlikely case which should in practice not happened. However if it happens we cleanup the reference just in case. Signed-off-by: Alexander Aring Signed-off-by: David Teigland --- fs/dlm/ast.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fs/dlm/ast.c b/fs/dlm/ast.c index 078bbbd43a537..982d093b80cda 100644 --- a/fs/dlm/ast.c +++ b/fs/dlm/ast.c @@ -186,7 +186,7 @@ void dlm_callback_work(struct work_struct *work) spin_unlock(&lkb->lkb_cb_lock); if (WARN_ON(rv == DLM_DEQUEUE_CALLBACK_EMPTY)) - return; + goto out; for (;;) { castfn = lkb->lkb_astfn; @@ -217,6 +217,7 @@ void dlm_callback_work(struct work_struct *work) spin_unlock(&lkb->lkb_cb_lock); } +out: /* undo kref_get from dlm_add_callback, may cause lkb to be freed */ dlm_put_lkb(lkb); } From 740bb8fc10d226d64c7da2271cf0b25dab1538dc Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Thu, 17 Nov 2022 17:11:42 -0500 Subject: [PATCH 21/37] fs: dlm: ast do WARN_ON_ONCE() on hotpath This patch changes the ast hotpath functionality in very unlikely cases that we do WARN_ON_ONCE() instead of WARN_ON() to not spamming the console output if we run into states that it would occur over and over again. Signed-off-by: Alexander Aring Signed-off-by: David Teigland --- fs/dlm/ast.c | 6 +++--- fs/dlm/user.c | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/fs/dlm/ast.c b/fs/dlm/ast.c index 982d093b80cda..7dd072b8ea383 100644 --- a/fs/dlm/ast.c +++ b/fs/dlm/ast.c @@ -161,12 +161,12 @@ void dlm_add_cb(struct dlm_lkb *lkb, uint32_t flags, int mode, int status, spin_unlock(&ls->ls_cb_lock); break; case DLM_ENQUEUE_CALLBACK_FAILURE: - WARN_ON(1); + WARN_ON_ONCE(1); break; case DLM_ENQUEUE_CALLBACK_SUCCESS: break; default: - WARN_ON(1); + WARN_ON_ONCE(1); break; } spin_unlock(&lkb->lkb_cb_lock); @@ -185,7 +185,7 @@ void dlm_callback_work(struct work_struct *work) rv = dlm_dequeue_lkb_callback(lkb, &cb); spin_unlock(&lkb->lkb_cb_lock); - if (WARN_ON(rv == DLM_DEQUEUE_CALLBACK_EMPTY)) + if (WARN_ON_ONCE(rv == DLM_DEQUEUE_CALLBACK_EMPTY)) goto out; for (;;) { diff --git a/fs/dlm/user.c b/fs/dlm/user.c index 6b530db4bc0b2..edf722d6935a7 100644 --- a/fs/dlm/user.c +++ b/fs/dlm/user.c @@ -214,7 +214,7 @@ void dlm_user_add_ast(struct dlm_lkb *lkb, uint32_t flags, int mode, switch (rv) { case DLM_ENQUEUE_CALLBACK_FAILURE: spin_unlock(&proc->asts_spin); - WARN_ON(1); + WARN_ON_ONCE(1); goto out; case DLM_ENQUEUE_CALLBACK_NEED_SCHED: kref_get(&lkb->lkb_ref); @@ -224,7 +224,7 @@ void dlm_user_add_ast(struct dlm_lkb *lkb, uint32_t flags, int mode, case DLM_ENQUEUE_CALLBACK_SUCCESS: break; default: - WARN_ON(1); + WARN_ON_ONCE(1); break; } spin_unlock(&proc->asts_spin); @@ -880,7 +880,7 @@ static ssize_t device_read(struct file *file, char __user *buf, size_t count, spin_unlock(&proc->asts_spin); /* removes ref for proc->asts, may cause lkb to be freed */ dlm_put_lkb(lkb); - WARN_ON(1); + WARN_ON_ONCE(1); goto try_another; case DLM_DEQUEUE_CALLBACK_LAST: list_del_init(&lkb->lkb_cb_list); @@ -890,7 +890,7 @@ static ssize_t device_read(struct file *file, char __user *buf, size_t count, case DLM_DEQUEUE_CALLBACK_SUCCESS: break; default: - WARN_ON(1); + WARN_ON_ONCE(1); break; } spin_unlock(&proc->asts_spin); From 554d849616769339b9dce833a4830251fc4b91ba Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Thu, 17 Nov 2022 17:11:43 -0500 Subject: [PATCH 22/37] fs: dlm: rename DLM_IFL_NEED_SCHED to DLM_IFL_CB_PENDING This patch renames DLM_IFL_NEED_SCHED to DLM_IFL_CB_PENDING because CB_PENDING is a proper name to describe this flag. This flag is set when callback enqueue will return DLM_ENQUEUE_CALLBACK_NEED_SCHED because the callback worker need to be queued. The flag tells that callbacks are currently pending to be called and will be unset if the callback work for the specific lkb is done. The term need schedule is part of this time but a proper name is to say that there are some callbacks pending to being called. Signed-off-by: Alexander Aring Signed-off-by: David Teigland --- fs/dlm/ast.c | 9 ++++----- fs/dlm/dlm_internal.h | 2 +- fs/dlm/user.c | 3 +-- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/fs/dlm/ast.c b/fs/dlm/ast.c index 7dd072b8ea383..26fef9945cc90 100644 --- a/fs/dlm/ast.c +++ b/fs/dlm/ast.c @@ -45,8 +45,7 @@ void dlm_purge_lkb_callbacks(struct dlm_lkb *lkb) kref_put(&cb->ref, dlm_release_callback); } - /* TODO */ - lkb->lkb_flags &= ~DLM_IFL_NEED_SCHED; + lkb->lkb_flags &= ~DLM_IFL_CB_PENDING; /* invalidate */ dlm_callback_set_last_ptr(&lkb->lkb_last_cast, NULL); @@ -104,8 +103,8 @@ int dlm_enqueue_lkb_callback(struct dlm_lkb *lkb, uint32_t flags, int mode, cb->sb_status = status; cb->sb_flags = (sbflags & 0x000000FF); kref_init(&cb->ref); - if (!(lkb->lkb_flags & DLM_IFL_NEED_SCHED)) { - lkb->lkb_flags |= DLM_IFL_NEED_SCHED; + if (!(lkb->lkb_flags & DLM_IFL_CB_PENDING)) { + lkb->lkb_flags |= DLM_IFL_CB_PENDING; rv = DLM_ENQUEUE_CALLBACK_NEED_SCHED; } list_add_tail(&cb->list, &lkb->lkb_callbacks); @@ -210,7 +209,7 @@ void dlm_callback_work(struct work_struct *work) spin_lock(&lkb->lkb_cb_lock); rv = dlm_dequeue_lkb_callback(lkb, &cb); if (rv == DLM_DEQUEUE_CALLBACK_EMPTY) { - lkb->lkb_flags &= ~DLM_IFL_NEED_SCHED; + lkb->lkb_flags &= ~DLM_IFL_CB_PENDING; spin_unlock(&lkb->lkb_cb_lock); break; } diff --git a/fs/dlm/dlm_internal.h b/fs/dlm/dlm_internal.h index 6318e0f51bc90..ab1a55337a6eb 100644 --- a/fs/dlm/dlm_internal.h +++ b/fs/dlm/dlm_internal.h @@ -211,7 +211,7 @@ struct dlm_args { #endif #define DLM_IFL_DEADLOCK_CANCEL 0x01000000 #define DLM_IFL_STUB_MS 0x02000000 /* magic number for m_flags */ -#define DLM_IFL_NEED_SCHED 0x04000000 +#define DLM_IFL_CB_PENDING 0x04000000 /* least significant 2 bytes are message changed, they are full transmitted * but at receive side only the 2 bytes LSB will be set. * diff --git a/fs/dlm/user.c b/fs/dlm/user.c index edf722d6935a7..35129505ddda1 100644 --- a/fs/dlm/user.c +++ b/fs/dlm/user.c @@ -884,8 +884,7 @@ static ssize_t device_read(struct file *file, char __user *buf, size_t count, goto try_another; case DLM_DEQUEUE_CALLBACK_LAST: list_del_init(&lkb->lkb_cb_list); - /* TODO */ - lkb->lkb_flags &= ~DLM_IFL_NEED_SCHED; + lkb->lkb_flags &= ~DLM_IFL_CB_PENDING; break; case DLM_DEQUEUE_CALLBACK_SUCCESS: break; From 81889255c2e6ed1eef448375b5d6330a2f1453de Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Thu, 17 Nov 2022 17:11:44 -0500 Subject: [PATCH 23/37] fs: dlm: rename seq to h_seq for msg tracing This patch renames seq to h_seq as it is named in the dlm header structure. Signed-off-by: Alexander Aring Signed-off-by: David Teigland --- include/trace/events/dlm.h | 44 +++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/include/trace/events/dlm.h b/include/trace/events/dlm.h index 4ec47828d55ed..212f30aec7cf2 100644 --- a/include/trace/events/dlm.h +++ b/include/trace/events/dlm.h @@ -342,12 +342,12 @@ TRACE_EVENT(dlm_unlock_end, DECLARE_EVENT_CLASS(dlm_rcom_template, - TP_PROTO(uint32_t seq, const struct dlm_rcom *rc), + TP_PROTO(uint32_t h_seq, const struct dlm_rcom *rc), - TP_ARGS(seq, rc), + TP_ARGS(h_seq, rc), TP_STRUCT__entry( - __field(uint32_t, seq) + __field(uint32_t, h_seq) __field(uint32_t, h_version) __field(uint32_t, h_lockspace) __field(uint32_t, h_nodeid) @@ -363,7 +363,7 @@ DECLARE_EVENT_CLASS(dlm_rcom_template, ), TP_fast_assign( - __entry->seq = seq; + __entry->h_seq = h_seq; __entry->h_version = le32_to_cpu(rc->rc_header.h_version); __entry->h_lockspace = le32_to_cpu(rc->rc_header.u.h_lockspace); __entry->h_nodeid = le32_to_cpu(rc->rc_header.h_nodeid); @@ -378,10 +378,10 @@ DECLARE_EVENT_CLASS(dlm_rcom_template, __get_dynamic_array_len(rc_buf)); ), - TP_printk("seq=%u, h_version=%s h_lockspace=%u h_nodeid=%u " + TP_printk("h_seq=%u h_version=%s h_lockspace=%u h_nodeid=%u " "h_length=%u h_cmd=%s rc_type=%s rc_result=%d " "rc_id=%llu rc_seq=%llu rc_seq_reply=%llu " - "rc_buf=0x%s", __entry->seq, + "rc_buf=0x%s", __entry->h_seq, show_message_version(__entry->h_version), __entry->h_lockspace, __entry->h_nodeid, __entry->h_length, show_header_cmd(__entry->h_cmd), @@ -394,22 +394,22 @@ DECLARE_EVENT_CLASS(dlm_rcom_template, ); DEFINE_EVENT(dlm_rcom_template, dlm_send_rcom, - TP_PROTO(uint32_t seq, const struct dlm_rcom *rc), - TP_ARGS(seq, rc)); + TP_PROTO(uint32_t h_seq, const struct dlm_rcom *rc), + TP_ARGS(h_seq, rc)); DEFINE_EVENT(dlm_rcom_template, dlm_recv_rcom, - TP_PROTO(uint32_t seq, const struct dlm_rcom *rc), - TP_ARGS(seq, rc)); + TP_PROTO(uint32_t h_seq, const struct dlm_rcom *rc), + TP_ARGS(h_seq, rc)); TRACE_EVENT(dlm_send_message, - TP_PROTO(uint32_t seq, const struct dlm_message *ms, + TP_PROTO(uint32_t h_seq, const struct dlm_message *ms, const void *name, int namelen), - TP_ARGS(seq, ms, name, namelen), + TP_ARGS(h_seq, ms, name, namelen), TP_STRUCT__entry( - __field(uint32_t, seq) + __field(uint32_t, h_seq) __field(uint32_t, h_version) __field(uint32_t, h_lockspace) __field(uint32_t, h_nodeid) @@ -439,7 +439,7 @@ TRACE_EVENT(dlm_send_message, ), TP_fast_assign( - __entry->seq = seq; + __entry->h_seq = h_seq; __entry->h_version = le32_to_cpu(ms->m_header.h_version); __entry->h_lockspace = le32_to_cpu(ms->m_header.u.h_lockspace); __entry->h_nodeid = le32_to_cpu(ms->m_header.h_nodeid); @@ -469,14 +469,14 @@ TRACE_EVENT(dlm_send_message, __get_dynamic_array_len(res_name)); ), - TP_printk("seq=%u h_version=%s h_lockspace=%u h_nodeid=%u " + TP_printk("h_seq=%u h_version=%s h_lockspace=%u h_nodeid=%u " "h_length=%u h_cmd=%s m_type=%s m_nodeid=%u " "m_pid=%u m_lkid=%u m_remid=%u m_parent_lkid=%u " "m_parent_remid=%u m_exflags=%s m_sbflags=%s m_flags=%s " "m_lvbseq=%u m_hash=%u m_status=%d m_grmode=%s " "m_rqmode=%s m_bastmode=%s m_asts=%d m_result=%d " "m_extra=0x%s res_name=0x%s", - __entry->seq, show_message_version(__entry->h_version), + __entry->h_seq, show_message_version(__entry->h_version), __entry->h_lockspace, __entry->h_nodeid, __entry->h_length, show_header_cmd(__entry->h_cmd), show_message_type(__entry->m_type), @@ -499,12 +499,12 @@ TRACE_EVENT(dlm_send_message, TRACE_EVENT(dlm_recv_message, - TP_PROTO(uint32_t seq, const struct dlm_message *ms), + TP_PROTO(uint32_t h_seq, const struct dlm_message *ms), - TP_ARGS(seq, ms), + TP_ARGS(h_seq, ms), TP_STRUCT__entry( - __field(uint32_t, seq) + __field(uint32_t, h_seq) __field(uint32_t, h_version) __field(uint32_t, h_lockspace) __field(uint32_t, h_nodeid) @@ -533,7 +533,7 @@ TRACE_EVENT(dlm_recv_message, ), TP_fast_assign( - __entry->seq = seq; + __entry->h_seq = h_seq; __entry->h_version = le32_to_cpu(ms->m_header.h_version); __entry->h_lockspace = le32_to_cpu(ms->m_header.u.h_lockspace); __entry->h_nodeid = le32_to_cpu(ms->m_header.h_nodeid); @@ -561,14 +561,14 @@ TRACE_EVENT(dlm_recv_message, __get_dynamic_array_len(m_extra)); ), - TP_printk("seq=%u h_version=%s h_lockspace=%u h_nodeid=%u " + TP_printk("h_seq=%u h_version=%s h_lockspace=%u h_nodeid=%u " "h_length=%u h_cmd=%s m_type=%s m_nodeid=%u " "m_pid=%u m_lkid=%u m_remid=%u m_parent_lkid=%u " "m_parent_remid=%u m_exflags=%s m_sbflags=%s m_flags=%s " "m_lvbseq=%u m_hash=%u m_status=%d m_grmode=%s " "m_rqmode=%s m_bastmode=%s m_asts=%d m_result=%d " "m_extra=0x%s", - __entry->seq, show_message_version(__entry->h_version), + __entry->h_seq, show_message_version(__entry->h_version), __entry->h_lockspace, __entry->h_nodeid, __entry->h_length, show_header_cmd(__entry->h_cmd), show_message_type(__entry->m_type), From 17827754e503d6c72b05a1c4603469ec9bf35d48 Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Thu, 17 Nov 2022 17:11:45 -0500 Subject: [PATCH 24/37] fs: dlm: add dst nodeid for msg tracing In DLM when we send a dlm message it is easy to add the lock resource name, but additional lookup is required when to trace the receive message side. The idea here is to move the lookup work to the user by using a lookup to find the right send message with recv message. As note DLM can't drop any message which is guaranteed by a special session layer. For doing the lookup a 3 tupel is required as an unique identification which is dst nodeid, src nodeid and sequence number. This patch adds the destination nodeid to the dlm message trace points. The source nodeid is given by the h_nodeid field inside the header. Signed-off-by: Alexander Aring Signed-off-by: David Teigland --- fs/dlm/midcomms.c | 10 ++++++---- include/trace/events/dlm.h | 38 ++++++++++++++++++++++---------------- 2 files changed, 28 insertions(+), 20 deletions(-) diff --git a/fs/dlm/midcomms.c b/fs/dlm/midcomms.c index 32194a750fe10..960def5ab5304 100644 --- a/fs/dlm/midcomms.c +++ b/fs/dlm/midcomms.c @@ -479,10 +479,10 @@ static void dlm_receive_buffer_3_2_trace(uint32_t seq, union dlm_packet *p) { switch (p->header.h_cmd) { case DLM_MSG: - trace_dlm_recv_message(seq, &p->message); + trace_dlm_recv_message(dlm_our_nodeid(), seq, &p->message); break; case DLM_RCOM: - trace_dlm_recv_rcom(seq, &p->rcom); + trace_dlm_recv_rcom(dlm_our_nodeid(), seq, &p->rcom); break; default: break; @@ -1151,11 +1151,13 @@ static void dlm_midcomms_commit_msg_3_2_trace(const struct dlm_mhandle *mh, { switch (mh->inner_p->header.h_cmd) { case DLM_MSG: - trace_dlm_send_message(mh->seq, &mh->inner_p->message, + trace_dlm_send_message(mh->node->nodeid, mh->seq, + &mh->inner_p->message, name, namelen); break; case DLM_RCOM: - trace_dlm_send_rcom(mh->seq, &mh->inner_p->rcom); + trace_dlm_send_rcom(mh->node->nodeid, mh->seq, + &mh->inner_p->rcom); break; default: /* nothing to trace */ diff --git a/include/trace/events/dlm.h b/include/trace/events/dlm.h index 212f30aec7cf2..37eb79e29b282 100644 --- a/include/trace/events/dlm.h +++ b/include/trace/events/dlm.h @@ -342,11 +342,12 @@ TRACE_EVENT(dlm_unlock_end, DECLARE_EVENT_CLASS(dlm_rcom_template, - TP_PROTO(uint32_t h_seq, const struct dlm_rcom *rc), + TP_PROTO(uint32_t dst, uint32_t h_seq, const struct dlm_rcom *rc), - TP_ARGS(h_seq, rc), + TP_ARGS(dst, h_seq, rc), TP_STRUCT__entry( + __field(uint32_t, dst) __field(uint32_t, h_seq) __field(uint32_t, h_version) __field(uint32_t, h_lockspace) @@ -363,6 +364,7 @@ DECLARE_EVENT_CLASS(dlm_rcom_template, ), TP_fast_assign( + __entry->dst = dst; __entry->h_seq = h_seq; __entry->h_version = le32_to_cpu(rc->rc_header.h_version); __entry->h_lockspace = le32_to_cpu(rc->rc_header.u.h_lockspace); @@ -378,10 +380,10 @@ DECLARE_EVENT_CLASS(dlm_rcom_template, __get_dynamic_array_len(rc_buf)); ), - TP_printk("h_seq=%u h_version=%s h_lockspace=%u h_nodeid=%u " + TP_printk("dst=%u h_seq=%u h_version=%s h_lockspace=%u h_nodeid=%u " "h_length=%u h_cmd=%s rc_type=%s rc_result=%d " "rc_id=%llu rc_seq=%llu rc_seq_reply=%llu " - "rc_buf=0x%s", __entry->h_seq, + "rc_buf=0x%s", __entry->dst, __entry->h_seq, show_message_version(__entry->h_version), __entry->h_lockspace, __entry->h_nodeid, __entry->h_length, show_header_cmd(__entry->h_cmd), @@ -394,21 +396,22 @@ DECLARE_EVENT_CLASS(dlm_rcom_template, ); DEFINE_EVENT(dlm_rcom_template, dlm_send_rcom, - TP_PROTO(uint32_t h_seq, const struct dlm_rcom *rc), - TP_ARGS(h_seq, rc)); + TP_PROTO(uint32_t dst, uint32_t h_seq, const struct dlm_rcom *rc), + TP_ARGS(dst, h_seq, rc)); DEFINE_EVENT(dlm_rcom_template, dlm_recv_rcom, - TP_PROTO(uint32_t h_seq, const struct dlm_rcom *rc), - TP_ARGS(h_seq, rc)); + TP_PROTO(uint32_t dst, uint32_t h_seq, const struct dlm_rcom *rc), + TP_ARGS(dst, h_seq, rc)); TRACE_EVENT(dlm_send_message, - TP_PROTO(uint32_t h_seq, const struct dlm_message *ms, + TP_PROTO(uint32_t dst, uint32_t h_seq, const struct dlm_message *ms, const void *name, int namelen), - TP_ARGS(h_seq, ms, name, namelen), + TP_ARGS(dst, h_seq, ms, name, namelen), TP_STRUCT__entry( + __field(uint32_t, dst) __field(uint32_t, h_seq) __field(uint32_t, h_version) __field(uint32_t, h_lockspace) @@ -439,6 +442,7 @@ TRACE_EVENT(dlm_send_message, ), TP_fast_assign( + __entry->dst = dst; __entry->h_seq = h_seq; __entry->h_version = le32_to_cpu(ms->m_header.h_version); __entry->h_lockspace = le32_to_cpu(ms->m_header.u.h_lockspace); @@ -469,13 +473,13 @@ TRACE_EVENT(dlm_send_message, __get_dynamic_array_len(res_name)); ), - TP_printk("h_seq=%u h_version=%s h_lockspace=%u h_nodeid=%u " + TP_printk("dst=%u h_seq=%u h_version=%s h_lockspace=%u h_nodeid=%u " "h_length=%u h_cmd=%s m_type=%s m_nodeid=%u " "m_pid=%u m_lkid=%u m_remid=%u m_parent_lkid=%u " "m_parent_remid=%u m_exflags=%s m_sbflags=%s m_flags=%s " "m_lvbseq=%u m_hash=%u m_status=%d m_grmode=%s " "m_rqmode=%s m_bastmode=%s m_asts=%d m_result=%d " - "m_extra=0x%s res_name=0x%s", + "m_extra=0x%s res_name=0x%s", __entry->dst, __entry->h_seq, show_message_version(__entry->h_version), __entry->h_lockspace, __entry->h_nodeid, __entry->h_length, show_header_cmd(__entry->h_cmd), @@ -499,11 +503,12 @@ TRACE_EVENT(dlm_send_message, TRACE_EVENT(dlm_recv_message, - TP_PROTO(uint32_t h_seq, const struct dlm_message *ms), + TP_PROTO(uint32_t dst, uint32_t h_seq, const struct dlm_message *ms), - TP_ARGS(h_seq, ms), + TP_ARGS(dst, h_seq, ms), TP_STRUCT__entry( + __field(uint32_t, dst) __field(uint32_t, h_seq) __field(uint32_t, h_version) __field(uint32_t, h_lockspace) @@ -533,6 +538,7 @@ TRACE_EVENT(dlm_recv_message, ), TP_fast_assign( + __entry->dst = dst; __entry->h_seq = h_seq; __entry->h_version = le32_to_cpu(ms->m_header.h_version); __entry->h_lockspace = le32_to_cpu(ms->m_header.u.h_lockspace); @@ -561,13 +567,13 @@ TRACE_EVENT(dlm_recv_message, __get_dynamic_array_len(m_extra)); ), - TP_printk("h_seq=%u h_version=%s h_lockspace=%u h_nodeid=%u " + TP_printk("dst=%u h_seq=%u h_version=%s h_lockspace=%u h_nodeid=%u " "h_length=%u h_cmd=%s m_type=%s m_nodeid=%u " "m_pid=%u m_lkid=%u m_remid=%u m_parent_lkid=%u " "m_parent_remid=%u m_exflags=%s m_sbflags=%s m_flags=%s " "m_lvbseq=%u m_hash=%u m_status=%d m_grmode=%s " "m_rqmode=%s m_bastmode=%s m_asts=%d m_result=%d " - "m_extra=0x%s", + "m_extra=0x%s", __entry->dst, __entry->h_seq, show_message_version(__entry->h_version), __entry->h_lockspace, __entry->h_nodeid, __entry->h_length, show_header_cmd(__entry->h_cmd), From 8b0188b0d60b6f6183b48380bac49fe080c5ded9 Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Thu, 17 Nov 2022 17:11:46 -0500 Subject: [PATCH 25/37] fs: dlm: add midcomms init/start functions This patch introduces leftovers of init, start, stop and exit functionality. The dlm application layer should always call the midcomms layer which getting aware of such event and redirect it to the lowcomms layer. Some functionality which is currently handled inside the start functionality of midcomms and lowcomms should be handled in the init functionality as it only need to be initialized once when dlm is loaded. Signed-off-by: Alexander Aring Signed-off-by: David Teigland --- fs/dlm/lockspace.c | 5 ++--- fs/dlm/lowcomms.c | 16 ++++++++++------ fs/dlm/lowcomms.h | 1 + fs/dlm/main.c | 7 +++++-- fs/dlm/midcomms.c | 17 ++++++++++++++++- fs/dlm/midcomms.h | 3 +++ 6 files changed, 37 insertions(+), 12 deletions(-) diff --git a/fs/dlm/lockspace.c b/fs/dlm/lockspace.c index 96fe8f7ca09cd..d0b4e2181a5f3 100644 --- a/fs/dlm/lockspace.c +++ b/fs/dlm/lockspace.c @@ -17,7 +17,6 @@ #include "recoverd.h" #include "dir.h" #include "midcomms.h" -#include "lowcomms.h" #include "config.h" #include "memory.h" #include "lock.h" @@ -723,7 +722,7 @@ static int __dlm_new_lockspace(const char *name, const char *cluster, if (!ls_count) { dlm_scand_stop(); dlm_midcomms_shutdown(); - dlm_lowcomms_stop(); + dlm_midcomms_stop(); } out: mutex_unlock(&ls_lock); @@ -926,7 +925,7 @@ int dlm_release_lockspace(void *lockspace, int force) if (!error) ls_count--; if (!ls_count) - dlm_lowcomms_stop(); + dlm_midcomms_stop(); mutex_unlock(&ls_lock); return error; diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c index b05c6d9b5102c..0dcb69cc456a8 100644 --- a/fs/dlm/lowcomms.c +++ b/fs/dlm/lowcomms.c @@ -1987,10 +1987,6 @@ static const struct dlm_proto_ops dlm_sctp_ops = { int dlm_lowcomms_start(void) { int error = -EINVAL; - int i; - - for (i = 0; i < CONN_HASH_SIZE; i++) - INIT_HLIST_HEAD(&connection_hash[i]); init_local(); if (!dlm_local_count) { @@ -1999,8 +1995,6 @@ int dlm_lowcomms_start(void) goto fail; } - INIT_WORK(&listen_con.rwork, process_listen_recv_socket); - error = work_start(); if (error) goto fail_local; @@ -2039,6 +2033,16 @@ int dlm_lowcomms_start(void) return error; } +void dlm_lowcomms_init(void) +{ + int i; + + for (i = 0; i < CONN_HASH_SIZE; i++) + INIT_HLIST_HEAD(&connection_hash[i]); + + INIT_WORK(&listen_con.rwork, process_listen_recv_socket); +} + void dlm_lowcomms_exit(void) { struct dlm_node_addr *na, *safe; diff --git a/fs/dlm/lowcomms.h b/fs/dlm/lowcomms.h index 29369feea9916..bbce7a18416dc 100644 --- a/fs/dlm/lowcomms.h +++ b/fs/dlm/lowcomms.h @@ -35,6 +35,7 @@ extern int dlm_allow_conn; int dlm_lowcomms_start(void); void dlm_lowcomms_shutdown(void); void dlm_lowcomms_stop(void); +void dlm_lowcomms_init(void); void dlm_lowcomms_exit(void); int dlm_lowcomms_close(int nodeid); struct dlm_msg *dlm_lowcomms_new_msg(int nodeid, int len, gfp_t allocation, diff --git a/fs/dlm/main.c b/fs/dlm/main.c index 1c5be4b70ac1b..a77338be32371 100644 --- a/fs/dlm/main.c +++ b/fs/dlm/main.c @@ -17,7 +17,7 @@ #include "user.h" #include "memory.h" #include "config.h" -#include "lowcomms.h" +#include "midcomms.h" #define CREATE_TRACE_POINTS #include @@ -30,6 +30,8 @@ static int __init init_dlm(void) if (error) goto out; + dlm_midcomms_init(); + error = dlm_lockspace_init(); if (error) goto out_mem; @@ -66,6 +68,7 @@ static int __init init_dlm(void) out_lockspace: dlm_lockspace_exit(); out_mem: + dlm_midcomms_exit(); dlm_memory_exit(); out: return error; @@ -79,7 +82,7 @@ static void __exit exit_dlm(void) dlm_config_exit(); dlm_memory_exit(); dlm_lockspace_exit(); - dlm_lowcomms_exit(); + dlm_midcomms_exit(); dlm_unregister_debugfs(); } diff --git a/fs/dlm/midcomms.c b/fs/dlm/midcomms.c index 960def5ab5304..6b4c72fb01710 100644 --- a/fs/dlm/midcomms.c +++ b/fs/dlm/midcomms.c @@ -1205,13 +1205,28 @@ void dlm_midcomms_commit_mhandle(struct dlm_mhandle *mh, #endif int dlm_midcomms_start(void) +{ + return dlm_lowcomms_start(); +} + +void dlm_midcomms_stop(void) +{ + dlm_lowcomms_stop(); +} + +void dlm_midcomms_init(void) { int i; for (i = 0; i < CONN_HASH_SIZE; i++) INIT_HLIST_HEAD(&node_hash[i]); - return dlm_lowcomms_start(); + dlm_lowcomms_init(); +} + +void dlm_midcomms_exit(void) +{ + dlm_lowcomms_exit(); } static void dlm_act_fin_ack_rcv(struct midcomms_node *node) diff --git a/fs/dlm/midcomms.h b/fs/dlm/midcomms.h index d6286e80b077a..69296552d5add 100644 --- a/fs/dlm/midcomms.h +++ b/fs/dlm/midcomms.h @@ -21,6 +21,9 @@ void dlm_midcomms_commit_mhandle(struct dlm_mhandle *mh, const void *name, int namelen); int dlm_midcomms_close(int nodeid); int dlm_midcomms_start(void); +void dlm_midcomms_stop(void); +void dlm_midcomms_init(void); +void dlm_midcomms_exit(void); void dlm_midcomms_shutdown(void); void dlm_midcomms_add_member(int nodeid); void dlm_midcomms_remove_member(int nodeid); From 01ea3d7701cb52dece379384f8aa7b8840f1d7c7 Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Thu, 17 Nov 2022 17:11:47 -0500 Subject: [PATCH 26/37] fs: dlm: remove twice INIT_WORK This patch removed a twice INIT_WORK() functionality. We already doing this inside of dlm_lowcomms_init() functionality which is called only once dlm is loaded. Signed-off-by: Alexander Aring Signed-off-by: David Teigland --- fs/dlm/lowcomms.c | 1 - 1 file changed, 1 deletion(-) diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c index 0dcb69cc456a8..3106e7f873447 100644 --- a/fs/dlm/lowcomms.c +++ b/fs/dlm/lowcomms.c @@ -1825,7 +1825,6 @@ static int dlm_listen_for_all(void) save_listen_callbacks(sock); add_listen_sock(sock, &listen_con); - INIT_WORK(&listen_con.rwork, process_listen_recv_socket); result = sock->ops->listen(sock, 5); if (result < 0) { dlm_close_sock(&listen_con.sock); From dd070a56e0fa36f03bcd09fbf1521c733cf2aa21 Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Thu, 17 Nov 2022 17:11:48 -0500 Subject: [PATCH 27/37] fs: dlm: use list_first_entry_or_null Instead of check on list_empty() we can do the same with list_first_entry_or_null() and return NULL if the returned value is NULL. Signed-off-by: Alexander Aring Signed-off-by: David Teigland --- fs/dlm/lowcomms.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c index 3106e7f873447..d3302b10b37e0 100644 --- a/fs/dlm/lowcomms.c +++ b/fs/dlm/lowcomms.c @@ -214,15 +214,12 @@ static struct writequeue_entry *con_next_wq(struct connection *con) { struct writequeue_entry *e; - if (list_empty(&con->writequeue)) - return NULL; - - e = list_first_entry(&con->writequeue, struct writequeue_entry, - list); + e = list_first_entry_or_null(&con->writequeue, struct writequeue_entry, + list); /* if len is zero nothing is to send, if there are users filling * buffers we wait until the users are done so we can send more. */ - if (e->users || e->len == 0) + if (!e || e->users || e->len == 0) return NULL; return e; From 1037c2a94ab51997d8b1ef9e7f6ed697e6e17d84 Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Thu, 17 Nov 2022 17:11:49 -0500 Subject: [PATCH 28/37] fs: dlm: use listen sock as dlm running indicator This patch will switch from dlm_allow_conn to check if dlm lowcomms is running or not to if we actually have a listen socket set or not. The list socket will be set and unset in lowcomms start and shutdown functionality. To synchronize with data_ready() callback we will set the socket callback to NULL while socket lock is held. Signed-off-by: Alexander Aring Signed-off-by: David Teigland --- fs/dlm/config.c | 4 ++-- fs/dlm/lowcomms.c | 17 ++++++----------- fs/dlm/lowcomms.h | 4 ++-- 3 files changed, 10 insertions(+), 15 deletions(-) diff --git a/fs/dlm/config.c b/fs/dlm/config.c index ac8b62106ce0e..20b60709eccf0 100644 --- a/fs/dlm/config.c +++ b/fs/dlm/config.c @@ -183,7 +183,7 @@ static int dlm_check_protocol_and_dlm_running(unsigned int x) return -EINVAL; } - if (dlm_allow_conn) + if (dlm_lowcomms_is_running()) return -EBUSY; return 0; @@ -194,7 +194,7 @@ static int dlm_check_zero_and_dlm_running(unsigned int x) if (!x) return -EINVAL; - if (dlm_allow_conn) + if (dlm_lowcomms_is_running()) return -EBUSY; return 0; diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c index d3302b10b37e0..d9001d40aa6e8 100644 --- a/fs/dlm/lowcomms.c +++ b/fs/dlm/lowcomms.c @@ -176,7 +176,6 @@ static DEFINE_SPINLOCK(dlm_node_addrs_spin); static struct listen_connection listen_con; static struct sockaddr_storage *dlm_local_addr[DLM_MAX_ADDR_COUNT]; static int dlm_local_count; -int dlm_allow_conn; /* Work queues */ static struct workqueue_struct *recv_workqueue; @@ -191,6 +190,11 @@ static const struct dlm_proto_ops *dlm_proto_ops; static void process_recv_sockets(struct work_struct *work); static void process_send_sockets(struct work_struct *work); +bool dlm_lowcomms_is_running(void) +{ + return !!listen_con.sock; +} + static void writequeue_entry_ctor(void *data) { struct writequeue_entry *entry = data; @@ -511,9 +515,6 @@ static void lowcomms_data_ready(struct sock *sk) static void lowcomms_listen_data_ready(struct sock *sk) { - if (!dlm_allow_conn) - return; - queue_work(recv_workqueue, &listen_con.rwork); } @@ -1689,10 +1690,7 @@ void dlm_lowcomms_shutdown(void) { int idx; - /* Set all the flags to prevent any - * socket activity. - */ - dlm_allow_conn = 0; + restore_callbacks(listen_con.sock); if (recv_workqueue) flush_workqueue(recv_workqueue); @@ -1995,8 +1993,6 @@ int dlm_lowcomms_start(void) if (error) goto fail_local; - dlm_allow_conn = 1; - /* Start listening */ switch (dlm_config.ci_protocol) { case DLM_PROTO_TCP: @@ -2021,7 +2017,6 @@ int dlm_lowcomms_start(void) fail_listen: dlm_proto_ops = NULL; fail_proto_ops: - dlm_allow_conn = 0; work_stop(); fail_local: deinit_local(); diff --git a/fs/dlm/lowcomms.h b/fs/dlm/lowcomms.h index bbce7a18416dc..acaf1b0b3885b 100644 --- a/fs/dlm/lowcomms.h +++ b/fs/dlm/lowcomms.h @@ -29,8 +29,8 @@ static inline int nodeid_hash(int nodeid) return nodeid & (CONN_HASH_SIZE-1); } -/* switch to check if dlm is running */ -extern int dlm_allow_conn; +/* check if dlm is running */ +bool dlm_lowcomms_is_running(void); int dlm_lowcomms_start(void); void dlm_lowcomms_shutdown(void); From 4f567acb0b8622fe265aac4b0037a03ef544ba24 Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Thu, 17 Nov 2022 17:11:50 -0500 Subject: [PATCH 29/37] fs: dlm: remove socket shutdown handling Since commit 489d8e559c65 ("fs: dlm: add reliable connection if reconnect") we have functionality like TCP offers for half-closed sockets on dlm application protocol layer. This feature is required because the cluster manager events about leaving resource memberships can be locally already occurred but other cluster nodes having a pending leaving membership over the cluster manager protocol happening. In this time the local dlm node already shutdown it's connection and don't transmit anymore any new dlm messages, but however it still needs to be able to accept dlm messages because the pending leave membership request of the cluster manager protocol which the dlm kernel implementation has no control about it. We have this functionality on the application for two reasons, the main reason is that SCTP does not support such functionality on socket layer. But we can do it inside application layer. Another small issue is that this feature is broken in the TCP world because some NAT devices does not implement such functionality correctly. This is the same reason why the reliable connection session layer in DLM exists. We give up on middle devices in the networking which sends e.g. TCP resets out. In DLM we cannot have any message dropping and we ensure it over a session layer that it can't happen. Back to the half-closed grace shutdown handling. It's not necessary anymore to do it on socket layer (which is only support for TCP sockets) because we do it on application layer. This patch removes this handling, if there are still issues then we have a problem on the application layer for such handling. Signed-off-by: Alexander Aring Signed-off-by: David Teigland --- fs/dlm/lowcomms.c | 127 ++++++++-------------------------------------- fs/dlm/lowcomms.h | 1 + fs/dlm/midcomms.c | 6 ++- 3 files changed, 27 insertions(+), 107 deletions(-) diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c index d9001d40aa6e8..e33bee1beba26 100644 --- a/fs/dlm/lowcomms.c +++ b/fs/dlm/lowcomms.c @@ -65,7 +65,6 @@ /* Number of messages to send before rescheduling */ #define MAX_SEND_MSG_COUNT 25 -#define DLM_SHUTDOWN_WAIT_TIMEOUT msecs_to_jiffies(10000) struct connection { struct socket *sock; /* NULL if not connected */ @@ -79,14 +78,11 @@ struct connection { #define CF_CLOSE 6 #define CF_APP_LIMITED 7 #define CF_CLOSING 8 -#define CF_SHUTDOWN 9 -#define CF_CONNECTED 10 -#define CF_RECONNECT 11 -#define CF_DELAY_CONNECT 12 -#define CF_EOF 13 +#define CF_CONNECTED 9 +#define CF_RECONNECT 10 +#define CF_DELAY_CONNECT 11 struct list_head writequeue; /* List of outgoing writequeue_entries */ spinlock_t writequeue_lock; - atomic_t writequeue_cnt; int retries; #define MAX_CONNECT_RETRIES 3 struct hlist_node list; @@ -94,7 +90,6 @@ struct connection { struct connection *sendcon; struct work_struct rwork; /* Receive workqueue */ struct work_struct swork; /* Send workqueue */ - wait_queue_head_t shutdown_wait; /* wait for graceful shutdown */ unsigned char *rx_buf; int rx_buflen; int rx_leftover; @@ -157,10 +152,6 @@ struct dlm_proto_ops { int (*listen_validate)(void); void (*listen_sockopts)(struct socket *sock); int (*listen_bind)(struct socket *sock); - /* What to do to shutdown */ - void (*shutdown_action)(struct connection *con); - /* What to do to eof check */ - bool (*eof_condition)(struct connection *con); }; static struct listen_sock_callbacks { @@ -241,11 +232,6 @@ static struct connection *__find_con(int nodeid, int r) return NULL; } -static bool tcp_eof_condition(struct connection *con) -{ - return atomic_read(&con->writequeue_cnt); -} - static int dlm_con_init(struct connection *con, int nodeid) { con->rx_buflen = dlm_config.ci_buffer_size; @@ -257,10 +243,8 @@ static int dlm_con_init(struct connection *con, int nodeid) mutex_init(&con->sock_mutex); INIT_LIST_HEAD(&con->writequeue); spin_lock_init(&con->writequeue_lock); - atomic_set(&con->writequeue_cnt, 0); INIT_WORK(&con->swork, process_send_sockets); INIT_WORK(&con->rwork, process_recv_sockets); - init_waitqueue_head(&con->shutdown_wait); return 0; } @@ -771,7 +755,6 @@ static void free_entry(struct writequeue_entry *e) } list_del(&e->list); - atomic_dec(&e->con->writequeue_cnt); kref_put(&e->ref, dlm_page_release); } @@ -834,56 +817,10 @@ static void close_connection(struct connection *con, bool and_other, clear_bit(CF_CONNECTED, &con->flags); clear_bit(CF_DELAY_CONNECT, &con->flags); clear_bit(CF_RECONNECT, &con->flags); - clear_bit(CF_EOF, &con->flags); mutex_unlock(&con->sock_mutex); clear_bit(CF_CLOSING, &con->flags); } -static void shutdown_connection(struct connection *con) -{ - int ret; - - flush_work(&con->swork); - - mutex_lock(&con->sock_mutex); - /* nothing to shutdown */ - if (!con->sock) { - mutex_unlock(&con->sock_mutex); - return; - } - - set_bit(CF_SHUTDOWN, &con->flags); - ret = kernel_sock_shutdown(con->sock, SHUT_WR); - mutex_unlock(&con->sock_mutex); - if (ret) { - log_print("Connection %p failed to shutdown: %d will force close", - con, ret); - goto force_close; - } else { - ret = wait_event_timeout(con->shutdown_wait, - !test_bit(CF_SHUTDOWN, &con->flags), - DLM_SHUTDOWN_WAIT_TIMEOUT); - if (ret == 0) { - log_print("Connection %p shutdown timed out, will force close", - con); - goto force_close; - } - } - - return; - -force_close: - clear_bit(CF_SHUTDOWN, &con->flags); - close_connection(con, false, true, true); -} - -static void dlm_tcp_shutdown(struct connection *con) -{ - if (con->othercon) - shutdown_connection(con->othercon); - shutdown_connection(con); -} - static int con_realloc_receive_buf(struct connection *con, int newlen) { unsigned char *newbuf; @@ -975,19 +912,8 @@ static int receive_from_sock(struct connection *con) log_print("connection %p got EOF from %d", con, con->nodeid); - if (dlm_proto_ops->eof_condition && - dlm_proto_ops->eof_condition(con)) { - set_bit(CF_EOF, &con->flags); - mutex_unlock(&con->sock_mutex); - } else { - mutex_unlock(&con->sock_mutex); - close_connection(con, false, true, false); - - /* handling for tcp shutdown */ - clear_bit(CF_SHUTDOWN, &con->flags); - wake_up(&con->shutdown_wait); - } - + mutex_unlock(&con->sock_mutex); + close_connection(con, false, true, false); /* signal to breaking receive worker */ ret = -1; } else { @@ -1261,7 +1187,6 @@ static struct writequeue_entry *new_wq_entry(struct connection *con, int len, kref_get(&e->ref); *ppc = page_address(e->page); e->end += len; - atomic_inc(&con->writequeue_cnt); if (cb) cb(data); @@ -1467,20 +1392,6 @@ static void send_to_sock(struct connection *con) } spin_unlock(&con->writequeue_lock); - /* close if we got EOF */ - if (test_and_clear_bit(CF_EOF, &con->flags)) { - mutex_unlock(&con->sock_mutex); - close_connection(con, false, false, true); - - /* handling for tcp shutdown */ - clear_bit(CF_SHUTDOWN, &con->flags); - wake_up(&con->shutdown_wait); - } else { - mutex_unlock(&con->sock_mutex); - } - - return; - out: mutex_unlock(&con->sock_mutex); return; @@ -1680,16 +1591,8 @@ static int work_start(void) return 0; } -static void shutdown_conn(struct connection *con) -{ - if (dlm_proto_ops->shutdown_action) - dlm_proto_ops->shutdown_action(con); -} - void dlm_lowcomms_shutdown(void) { - int idx; - restore_callbacks(listen_con.sock); if (recv_workqueue) @@ -1698,9 +1601,25 @@ void dlm_lowcomms_shutdown(void) flush_workqueue(send_workqueue); dlm_close_sock(&listen_con.sock); +} + +void dlm_lowcomms_shutdown_node(int nodeid, bool force) +{ + struct connection *con; + int idx; idx = srcu_read_lock(&connections_srcu); - foreach_conn(shutdown_conn); + con = nodeid2con(nodeid, 0); + if (WARN_ON_ONCE(!con)) { + srcu_read_unlock(&connections_srcu, idx); + return; + } + + WARN_ON_ONCE(!force && !list_empty(&con->writequeue)); + clean_one_writequeue(con); + if (con->othercon) + clean_one_writequeue(con->othercon); + close_connection(con, true, true, true); srcu_read_unlock(&connections_srcu, idx); } @@ -1912,8 +1831,6 @@ static const struct dlm_proto_ops dlm_tcp_ops = { .listen_validate = dlm_tcp_listen_validate, .listen_sockopts = dlm_tcp_listen_sockopts, .listen_bind = dlm_tcp_listen_bind, - .shutdown_action = dlm_tcp_shutdown, - .eof_condition = tcp_eof_condition, }; static int dlm_sctp_bind(struct socket *sock) diff --git a/fs/dlm/lowcomms.h b/fs/dlm/lowcomms.h index acaf1b0b3885b..3e8dca66183b8 100644 --- a/fs/dlm/lowcomms.h +++ b/fs/dlm/lowcomms.h @@ -34,6 +34,7 @@ bool dlm_lowcomms_is_running(void); int dlm_lowcomms_start(void); void dlm_lowcomms_shutdown(void); +void dlm_lowcomms_shutdown_node(int nodeid, bool force); void dlm_lowcomms_stop(void); void dlm_lowcomms_init(void); void dlm_lowcomms_exit(void); diff --git a/fs/dlm/midcomms.c b/fs/dlm/midcomms.c index 6b4c72fb01710..b0e8bdcaab1bc 100644 --- a/fs/dlm/midcomms.c +++ b/fs/dlm/midcomms.c @@ -1426,11 +1426,13 @@ static void midcomms_shutdown(struct midcomms_node *node) pr_debug("active shutdown timed out for node %d with state %s\n", node->nodeid, dlm_state_str(node->state)); midcomms_node_reset(node); + dlm_lowcomms_shutdown_node(node->nodeid, true); return; } pr_debug("active shutdown done for node %d with state %s\n", node->nodeid, dlm_state_str(node->state)); + dlm_lowcomms_shutdown_node(node->nodeid, false); } void dlm_midcomms_shutdown(void) @@ -1438,6 +1440,8 @@ void dlm_midcomms_shutdown(void) struct midcomms_node *node; int i, idx; + dlm_lowcomms_shutdown(); + mutex_lock(&close_lock); idx = srcu_read_lock(&nodes_srcu); for (i = 0; i < CONN_HASH_SIZE; i++) { @@ -1455,8 +1459,6 @@ void dlm_midcomms_shutdown(void) } srcu_read_unlock(&nodes_srcu, idx); mutex_unlock(&close_lock); - - dlm_lowcomms_shutdown(); } int dlm_midcomms_close(int nodeid) From c3d88dfd15835f42c79467eee2c40db3095e5271 Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Thu, 17 Nov 2022 17:11:51 -0500 Subject: [PATCH 30/37] fs: dlm: cleanup listen sock handling This patch removes save_listen_callbacks() and add_listen_sock() as they are only used once in lowcomms functionality. For shutdown lowcomms it's not necessary to whole flush the workqueues to synchronize with restoring the old sk_data_ready() callback. Only the listen con receive work need to be cancelled. For each individual node shutdown we should be sure that last ack was been transmitted which is done by flushing per connection swork worker. Signed-off-by: Alexander Aring Signed-off-by: David Teigland --- fs/dlm/lowcomms.c | 51 ++++++++++++++++------------------------------- 1 file changed, 17 insertions(+), 34 deletions(-) diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c index e33bee1beba26..8bf09c3a0946a 100644 --- a/fs/dlm/lowcomms.c +++ b/fs/dlm/lowcomms.c @@ -647,17 +647,6 @@ static void lowcomms_error_report(struct sock *sk) orig_report(sk); } -/* Note: sk_callback_lock must be locked before calling this function. */ -static void save_listen_callbacks(struct socket *sock) -{ - struct sock *sk = sock->sk; - - listen_sock.sk_data_ready = sk->sk_data_ready; - listen_sock.sk_state_change = sk->sk_state_change; - listen_sock.sk_write_space = sk->sk_write_space; - listen_sock.sk_error_report = sk->sk_error_report; -} - static void restore_callbacks(struct socket *sock) { struct sock *sk = sock->sk; @@ -671,21 +660,6 @@ static void restore_callbacks(struct socket *sock) release_sock(sk); } -static void add_listen_sock(struct socket *sock, struct listen_connection *con) -{ - struct sock *sk = sock->sk; - - lock_sock(sk); - save_listen_callbacks(sock); - con->sock = sock; - - sk->sk_user_data = con; - sk->sk_allocation = GFP_NOFS; - /* Install a data_ready callback */ - sk->sk_data_ready = lowcomms_listen_data_ready; - release_sock(sk); -} - /* Make a socket active */ static void add_sock(struct socket *sock, struct connection *con) { @@ -1593,13 +1567,12 @@ static int work_start(void) void dlm_lowcomms_shutdown(void) { - restore_callbacks(listen_con.sock); - - if (recv_workqueue) - flush_workqueue(recv_workqueue); - if (send_workqueue) - flush_workqueue(send_workqueue); + /* stop lowcomms_listen_data_ready calls */ + lock_sock(listen_con.sock->sk); + listen_con.sock->sk->sk_data_ready = listen_sock.sk_data_ready; + release_sock(listen_con.sock->sk); + cancel_work_sync(&listen_con.rwork); dlm_close_sock(&listen_con.sock); } @@ -1615,6 +1588,7 @@ void dlm_lowcomms_shutdown_node(int nodeid, bool force) return; } + flush_work(&con->swork); WARN_ON_ONCE(!force && !list_empty(&con->writequeue)); clean_one_writequeue(con); if (con->othercon) @@ -1736,8 +1710,17 @@ static int dlm_listen_for_all(void) if (result < 0) goto out; - save_listen_callbacks(sock); - add_listen_sock(sock, &listen_con); + lock_sock(sock->sk); + listen_sock.sk_data_ready = sock->sk->sk_data_ready; + listen_sock.sk_write_space = sock->sk->sk_write_space; + listen_sock.sk_error_report = sock->sk->sk_error_report; + listen_sock.sk_state_change = sock->sk->sk_state_change; + + listen_con.sock = sock; + + sock->sk->sk_allocation = GFP_NOFS; + sock->sk->sk_data_ready = lowcomms_listen_data_ready; + release_sock(sock->sk); result = sock->ops->listen(sock, 5); if (result < 0) { From c51c9cd8addcfbdc097dbefd59f022402183644b Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Thu, 17 Nov 2022 17:11:52 -0500 Subject: [PATCH 31/37] fs: dlm: don't put dlm_local_addrs on heap This patch removes to allocate the dlm_local_addr[] pointers on the heap. Instead we directly store the type of "struct sockaddr_storage". This removes function deinit_local() because it was freeing memory only. Signed-off-by: Alexander Aring Signed-off-by: David Teigland --- fs/dlm/lowcomms.c | 38 ++++++++++++-------------------------- 1 file changed, 12 insertions(+), 26 deletions(-) diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c index 8bf09c3a0946a..0883394cfbeb2 100644 --- a/fs/dlm/lowcomms.c +++ b/fs/dlm/lowcomms.c @@ -165,7 +165,7 @@ static LIST_HEAD(dlm_node_addrs); static DEFINE_SPINLOCK(dlm_node_addrs_spin); static struct listen_connection listen_con; -static struct sockaddr_storage *dlm_local_addr[DLM_MAX_ADDR_COUNT]; +static struct sockaddr_storage dlm_local_addr[DLM_MAX_ADDR_COUNT]; static int dlm_local_count; /* Work queues */ @@ -383,7 +383,7 @@ static int nodeid_to_addr(int nodeid, struct sockaddr_storage *sas_out, if (!sa_out) return 0; - if (dlm_local_addr[0]->ss_family == AF_INET) { + if (dlm_local_addr[0].ss_family == AF_INET) { struct sockaddr_in *in4 = (struct sockaddr_in *) &sas; struct sockaddr_in *ret4 = (struct sockaddr_in *) sa_out; ret4->sin_addr.s_addr = in4->sin_addr.s_addr; @@ -683,7 +683,7 @@ static void add_sock(struct socket *sock, struct connection *con) static void make_sockaddr(struct sockaddr_storage *saddr, uint16_t port, int *addr_len) { - saddr->ss_family = dlm_local_addr[0]->ss_family; + saddr->ss_family = dlm_local_addr[0].ss_family; if (saddr->ss_family == AF_INET) { struct sockaddr_in *in4_addr = (struct sockaddr_in *)saddr; in4_addr->sin_port = cpu_to_be16(port); @@ -1065,7 +1065,7 @@ static int sctp_bind_addrs(struct socket *sock, uint16_t port) int i, addr_len, result = 0; for (i = 0; i < dlm_local_count; i++) { - memcpy(&localaddr, dlm_local_addr[i], sizeof(localaddr)); + memcpy(&localaddr, &dlm_local_addr[i], sizeof(localaddr)); make_sockaddr(&localaddr, port, &addr_len); if (!i) @@ -1085,7 +1085,7 @@ static int sctp_bind_addrs(struct socket *sock, uint16_t port) /* Get local addresses */ static void init_local(void) { - struct sockaddr_storage sas, *addr; + struct sockaddr_storage sas; int i; dlm_local_count = 0; @@ -1093,21 +1093,10 @@ static void init_local(void) if (dlm_our_addr(&sas, i)) break; - addr = kmemdup(&sas, sizeof(*addr), GFP_NOFS); - if (!addr) - break; - dlm_local_addr[dlm_local_count++] = addr; + memcpy(&dlm_local_addr[dlm_local_count++], &sas, sizeof(sas)); } } -static void deinit_local(void) -{ - int i; - - for (i = 0; i < dlm_local_count; i++) - kfree(dlm_local_addr[i]); -} - static struct writequeue_entry *new_writequeue_entry(struct connection *con) { struct writequeue_entry *entry; @@ -1463,7 +1452,7 @@ static void dlm_connect(struct connection *con) } /* Create a socket to communicate with */ - result = sock_create_kern(&init_net, dlm_local_addr[0]->ss_family, + result = sock_create_kern(&init_net, dlm_local_addr[0].ss_family, SOCK_STREAM, dlm_proto_ops->proto, &sock); if (result < 0) goto socket_err; @@ -1679,7 +1668,6 @@ void dlm_lowcomms_stop(void) foreach_conn(free_conn); srcu_read_unlock(&connections_srcu, idx); work_stop(); - deinit_local(); dlm_proto_ops = NULL; } @@ -1696,7 +1684,7 @@ static int dlm_listen_for_all(void) if (result < 0) return result; - result = sock_create_kern(&init_net, dlm_local_addr[0]->ss_family, + result = sock_create_kern(&init_net, dlm_local_addr[0].ss_family, SOCK_STREAM, dlm_proto_ops->proto, &sock); if (result < 0) { log_print("Can't create comms socket: %d", result); @@ -1743,7 +1731,7 @@ static int dlm_tcp_bind(struct socket *sock) /* Bind to our cluster-known address connecting to avoid * routing problems. */ - memcpy(&src_addr, dlm_local_addr[0], sizeof(src_addr)); + memcpy(&src_addr, &dlm_local_addr[0], sizeof(src_addr)); make_sockaddr(&src_addr, 0, &addr_len); result = sock->ops->bind(sock, (struct sockaddr *)&src_addr, @@ -1800,8 +1788,8 @@ static int dlm_tcp_listen_bind(struct socket *sock) int addr_len; /* Bind to our port */ - make_sockaddr(dlm_local_addr[0], dlm_config.ci_tcp_port, &addr_len); - return sock->ops->bind(sock, (struct sockaddr *)dlm_local_addr[0], + make_sockaddr(&dlm_local_addr[0], dlm_config.ci_tcp_port, &addr_len); + return sock->ops->bind(sock, (struct sockaddr *)&dlm_local_addr[0], addr_len); } @@ -1891,7 +1879,7 @@ int dlm_lowcomms_start(void) error = work_start(); if (error) - goto fail_local; + goto fail; /* Start listening */ switch (dlm_config.ci_protocol) { @@ -1918,8 +1906,6 @@ int dlm_lowcomms_start(void) dlm_proto_ops = NULL; fail_proto_ops: work_stop(); -fail_local: - deinit_local(); fail: return error; } From 6f0b0b5d7ae70423c94915989c45b29e87c61ad7 Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Thu, 17 Nov 2022 17:11:53 -0500 Subject: [PATCH 32/37] fs: dlm: remove dlm_node_addrs lookup list This patch merges the dlm_node_addrs lookup list to the connection structure. It is a per node mapping to some configuration setup by configfs. We don't need two lookup structures. The connection hash has now a lifetime like the dlm_node_addrs entries. Means we add only new entries when configure cluster and not while new connections are coming in, remove connection when a node got fenced and cleanup all connection when the dlm exits. It should work the same and even will show more issues because we don't try to somehow keep those two data structures in sync with the current cluster configuration. Signed-off-by: Alexander Aring Signed-off-by: David Teigland --- fs/dlm/lowcomms.c | 290 ++++++++++++++++++++++------------------------ 1 file changed, 136 insertions(+), 154 deletions(-) diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c index 0883394cfbeb2..ed3cd37571991 100644 --- a/fs/dlm/lowcomms.c +++ b/fs/dlm/lowcomms.c @@ -93,6 +93,11 @@ struct connection { unsigned char *rx_buf; int rx_buflen; int rx_leftover; + int mark; + int addr_count; + int curr_addr_index; + struct sockaddr_storage addr[DLM_MAX_ADDR_COUNT]; + spinlock_t addrs_lock; struct rcu_head rcu; }; #define sock2con(x) ((struct connection *)(x)->sk_user_data) @@ -131,15 +136,6 @@ struct dlm_msg { struct kref ref; }; -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]; -}; - struct dlm_proto_ops { bool try_new_addr; const char *name; @@ -161,9 +157,6 @@ static struct listen_sock_callbacks { void (*sk_write_space)(struct sock *); } listen_sock; -static LIST_HEAD(dlm_node_addrs); -static DEFINE_SPINLOCK(dlm_node_addrs_spin); - static struct listen_connection listen_con; static struct sockaddr_storage dlm_local_addr[DLM_MAX_ADDR_COUNT]; static int dlm_local_count; @@ -306,17 +299,6 @@ static void foreach_conn(void (*conn_func)(struct connection *c)) } } -static struct dlm_node_addr *find_node_addr(int nodeid) -{ - struct dlm_node_addr *na; - - list_for_each_entry(na, &dlm_node_addrs, list) { - if (na->nodeid == nodeid) - return na; - } - return NULL; -} - static int addr_compare(const struct sockaddr_storage *x, const struct sockaddr_storage *y) { @@ -350,38 +332,45 @@ static int nodeid_to_addr(int nodeid, struct sockaddr_storage *sas_out, unsigned int *mark) { struct sockaddr_storage sas; - struct dlm_node_addr *na; + struct connection *con; + int idx; if (!dlm_local_count) return -1; - spin_lock(&dlm_node_addrs_spin); - na = find_node_addr(nodeid); - if (na && na->addr_count) { - memcpy(&sas, na->addr[na->curr_addr_index], - sizeof(struct sockaddr_storage)); + idx = srcu_read_lock(&connections_srcu); + con = nodeid2con(nodeid, 0); + if (!con) { + srcu_read_unlock(&connections_srcu, idx); + return -ENOENT; + } - if (try_new_addr) { - na->curr_addr_index++; - if (na->curr_addr_index == na->addr_count) - na->curr_addr_index = 0; - } + spin_lock(&con->addrs_lock); + if (!con->addr_count) { + spin_unlock(&con->addrs_lock); + srcu_read_unlock(&connections_srcu, idx); + return -ENOENT; } - spin_unlock(&dlm_node_addrs_spin); - if (!na) - return -EEXIST; + memcpy(&sas, &con->addr[con->curr_addr_index], + sizeof(struct sockaddr_storage)); - if (!na->addr_count) - return -ENOENT; + if (try_new_addr) { + con->curr_addr_index++; + if (con->curr_addr_index == con->addr_count) + con->curr_addr_index = 0; + } - *mark = na->mark; + *mark = con->mark; + spin_unlock(&con->addrs_lock); if (sas_out) memcpy(sas_out, &sas, sizeof(struct sockaddr_storage)); - if (!sa_out) + if (!sa_out) { + srcu_read_unlock(&connections_srcu, idx); return 0; + } if (dlm_local_addr[0].ss_family == AF_INET) { struct sockaddr_in *in4 = (struct sockaddr_in *) &sas; @@ -393,43 +382,46 @@ static int nodeid_to_addr(int nodeid, struct sockaddr_storage *sas_out, ret6->sin6_addr = in6->sin6_addr; } + srcu_read_unlock(&connections_srcu, idx); return 0; } static int addr_to_nodeid(struct sockaddr_storage *addr, int *nodeid, unsigned int *mark) { - struct dlm_node_addr *na; - int rv = -EEXIST; - int addr_i; - - spin_lock(&dlm_node_addrs_spin); - list_for_each_entry(na, &dlm_node_addrs, list) { - if (!na->addr_count) - continue; - - 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; + struct connection *con; + int i, idx, addr_i; + + idx = srcu_read_lock(&connections_srcu); + for (i = 0; i < CONN_HASH_SIZE; i++) { + hlist_for_each_entry_rcu(con, &connection_hash[i], list) { + WARN_ON_ONCE(!con->addr_count); + + spin_lock(&con->addrs_lock); + for (addr_i = 0; addr_i < con->addr_count; addr_i++) { + if (addr_compare(&con->addr[addr_i], addr)) { + *nodeid = con->nodeid; + *mark = con->mark; + spin_unlock(&con->addrs_lock); + srcu_read_unlock(&connections_srcu, idx); + return 0; + } } + spin_unlock(&con->addrs_lock); } } -unlock: - spin_unlock(&dlm_node_addrs_spin); - return rv; + srcu_read_unlock(&connections_srcu, idx); + + return -ENOENT; } -/* caller need to held dlm_node_addrs_spin lock */ -static bool dlm_lowcomms_na_has_addr(const struct dlm_node_addr *na, - const struct sockaddr_storage *addr) +static bool dlm_lowcomms_con_has_addr(const struct connection *con, + const struct sockaddr_storage *addr) { int i; - for (i = 0; i < na->addr_count; i++) { - if (addr_compare(na->addr[i], addr)) + for (i = 0; i < con->addr_count; i++) { + if (addr_compare(&con->addr[i], addr)) return true; } @@ -438,52 +430,42 @@ static bool dlm_lowcomms_na_has_addr(const struct dlm_node_addr *na, int dlm_lowcomms_addr(int nodeid, struct sockaddr_storage *addr, int len) { - struct sockaddr_storage *new_addr; - struct dlm_node_addr *new_node, *na; - bool ret; - - new_node = kzalloc(sizeof(struct dlm_node_addr), GFP_NOFS); - if (!new_node) - return -ENOMEM; + struct connection *con; + bool ret, idx; - new_addr = kzalloc(sizeof(struct sockaddr_storage), GFP_NOFS); - if (!new_addr) { - kfree(new_node); + idx = srcu_read_lock(&connections_srcu); + con = nodeid2con(nodeid, GFP_NOFS); + if (!con) { + srcu_read_unlock(&connections_srcu, idx); return -ENOMEM; } - memcpy(new_addr, addr, len); - - spin_lock(&dlm_node_addrs_spin); - na = find_node_addr(nodeid); - if (!na) { - 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); + spin_lock(&con->addrs_lock); + if (!con->addr_count) { + memcpy(&con->addr[0], addr, sizeof(*addr)); + con->addr_count = 1; + con->mark = dlm_config.ci_mark; + spin_unlock(&con->addrs_lock); + srcu_read_unlock(&connections_srcu, idx); return 0; } - ret = dlm_lowcomms_na_has_addr(na, addr); + ret = dlm_lowcomms_con_has_addr(con, addr); if (ret) { - spin_unlock(&dlm_node_addrs_spin); - kfree(new_addr); - kfree(new_node); + spin_unlock(&con->addrs_lock); + srcu_read_unlock(&connections_srcu, idx); return -EEXIST; } - if (na->addr_count >= DLM_MAX_ADDR_COUNT) { - spin_unlock(&dlm_node_addrs_spin); - kfree(new_addr); - kfree(new_node); + if (con->addr_count >= DLM_MAX_ADDR_COUNT) { + spin_unlock(&con->addrs_lock); + srcu_read_unlock(&connections_srcu, idx); return -ENOSPC; } - na->addr[na->addr_count++] = new_addr; - spin_unlock(&dlm_node_addrs_spin); - kfree(new_node); + memcpy(&con->addr[con->addr_count++], addr, sizeof(*addr)); + srcu_read_unlock(&connections_srcu, idx); + spin_unlock(&con->addrs_lock); return 0; } @@ -558,10 +540,10 @@ int dlm_lowcomms_connect_node(int nodeid) return 0; idx = srcu_read_lock(&connections_srcu); - con = nodeid2con(nodeid, GFP_NOFS); - if (!con) { + con = nodeid2con(nodeid, 0); + if (WARN_ON_ONCE(!con)) { srcu_read_unlock(&connections_srcu, idx); - return -ENOMEM; + return -ENOENT; } lowcomms_connect_sock(con); @@ -572,18 +554,20 @@ int dlm_lowcomms_connect_node(int nodeid) int dlm_lowcomms_nodes_set_mark(int nodeid, unsigned int mark) { - struct dlm_node_addr *na; + struct connection *con; + int idx; - spin_lock(&dlm_node_addrs_spin); - na = find_node_addr(nodeid); - if (!na) { - spin_unlock(&dlm_node_addrs_spin); + idx = srcu_read_lock(&connections_srcu); + con = nodeid2con(nodeid, 0); + if (!con) { + srcu_read_unlock(&connections_srcu, idx); return -ENOENT; } - na->mark = mark; - spin_unlock(&dlm_node_addrs_spin); - + spin_lock(&con->addrs_lock); + con->mark = mark; + spin_unlock(&con->addrs_lock); + srcu_read_unlock(&connections_srcu, idx); return 0; } @@ -960,10 +944,10 @@ static int accept_from_sock(struct listen_connection *con) * In this case we store the incoming one in "othercon" */ idx = srcu_read_lock(&connections_srcu); - newcon = nodeid2con(nodeid, GFP_NOFS); - if (!newcon) { + newcon = nodeid2con(nodeid, 0); + if (WARN_ON_ONCE(!newcon)) { srcu_read_unlock(&connections_srcu, idx); - result = -ENOMEM; + result = -ENOENT; goto accept_err; } @@ -1210,8 +1194,8 @@ struct dlm_msg *dlm_lowcomms_new_msg(int nodeid, int len, gfp_t allocation, } idx = srcu_read_lock(&connections_srcu); - con = nodeid2con(nodeid, allocation); - if (!con) { + con = nodeid2con(nodeid, 0); + if (WARN_ON_ONCE(!con)) { srcu_read_unlock(&connections_srcu, idx); return NULL; } @@ -1376,35 +1360,44 @@ static void clean_one_writequeue(struct connection *con) spin_unlock(&con->writequeue_lock); } +static void connection_release(struct rcu_head *rcu) +{ + struct connection *con = container_of(rcu, struct connection, rcu); + + kfree(con->rx_buf); + kfree(con); +} + /* Called from recovery when it knows that a node has left the cluster */ int dlm_lowcomms_close(int nodeid) { struct connection *con; - struct dlm_node_addr *na; int idx; log_print("closing connection to node %d", nodeid); + idx = srcu_read_lock(&connections_srcu); con = nodeid2con(nodeid, 0); - if (con) { - set_bit(CF_CLOSE, &con->flags); - close_connection(con, true, true, true); - clean_one_writequeue(con); - if (con->othercon) - clean_one_writequeue(con->othercon); + if (WARN_ON_ONCE(!con)) { + srcu_read_unlock(&connections_srcu, idx); + return -ENOENT; } - srcu_read_unlock(&connections_srcu, idx); - spin_lock(&dlm_node_addrs_spin); - na = find_node_addr(nodeid); - if (na) { - list_del(&na->list); - while (na->addr_count--) - kfree(na->addr[na->addr_count]); - kfree(na); + spin_lock(&connections_lock); + hlist_del_rcu(&con->list); + spin_unlock(&connections_lock); + + close_connection(con, true, true, true); + + clean_one_writequeue(con); + call_srcu(&connections_srcu, &con->rcu, connection_release); + if (con->othercon) { + clean_one_writequeue(con->othercon); + if (con->othercon) + call_srcu(&connections_srcu, &con->othercon->rcu, connection_release); } - spin_unlock(&dlm_node_addrs_spin); + srcu_read_unlock(&connections_srcu, idx); return 0; } @@ -1607,27 +1600,9 @@ static void stop_conn(struct connection *con) _stop_conn(con, true); } -static void connection_release(struct rcu_head *rcu) -{ - struct connection *con = container_of(rcu, struct connection, rcu); - - kfree(con->rx_buf); - kfree(con); -} - static void free_conn(struct connection *con) { close_connection(con, true, true, true); - spin_lock(&connections_lock); - hlist_del_rcu(&con->list); - spin_unlock(&connections_lock); - if (con->othercon) { - clean_one_writequeue(con->othercon); - call_srcu(&connections_srcu, &con->othercon->rcu, - connection_release); - } - clean_one_writequeue(con); - call_srcu(&connections_srcu, &con->rcu, connection_release); } static void work_flush(void) @@ -1922,14 +1897,21 @@ void dlm_lowcomms_init(void) void dlm_lowcomms_exit(void) { - struct dlm_node_addr *na, *safe; + struct connection *con; + int i, idx; - spin_lock(&dlm_node_addrs_spin); - list_for_each_entry_safe(na, safe, &dlm_node_addrs, list) { - list_del(&na->list); - while (na->addr_count--) - kfree(na->addr[na->addr_count]); - kfree(na); + idx = srcu_read_lock(&connections_srcu); + for (i = 0; i < CONN_HASH_SIZE; i++) { + hlist_for_each_entry_rcu(con, &connection_hash[i], list) { + spin_lock(&connections_lock); + hlist_del_rcu(&con->list); + spin_unlock(&connections_lock); + + if (con->othercon) + call_srcu(&connections_srcu, &con->othercon->rcu, + connection_release); + call_srcu(&connections_srcu, &con->rcu, connection_release); + } } - spin_unlock(&dlm_node_addrs_spin); + srcu_read_unlock(&connections_srcu, idx); } From e9dd5fd849f1ac125919a79eb4be66f078dd7f51 Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Thu, 17 Nov 2022 17:11:54 -0500 Subject: [PATCH 33/37] fs: dlm: use sock2con without checking null This patch removes null checks on private data for sockets. If we have a null dereference there we having a bug in our implementation that such callback occurs in this state. Signed-off-by: Alexander Aring Signed-off-by: David Teigland --- fs/dlm/lowcomms.c | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c index ed3cd37571991..677e31144ca0b 100644 --- a/fs/dlm/lowcomms.c +++ b/fs/dlm/lowcomms.c @@ -472,10 +472,9 @@ int dlm_lowcomms_addr(int nodeid, struct sockaddr_storage *addr, int len) /* Data available on socket or listen socket received a connect */ static void lowcomms_data_ready(struct sock *sk) { - struct connection *con; + struct connection *con = sock2con(sk); - con = sock2con(sk); - if (con && !test_and_set_bit(CF_READ_PENDING, &con->flags)) + if (!test_and_set_bit(CF_READ_PENDING, &con->flags)) queue_work(recv_workqueue, &con->rwork); } @@ -486,11 +485,7 @@ static void lowcomms_listen_data_ready(struct sock *sk) static void lowcomms_write_space(struct sock *sk) { - struct connection *con; - - con = sock2con(sk); - if (!con) - return; + struct connection *con = sock2con(sk); if (!test_and_set_bit(CF_CONNECTED, &con->flags)) { log_print("connected to node %d", con->nodeid); @@ -573,14 +568,10 @@ int dlm_lowcomms_nodes_set_mark(int nodeid, unsigned int mark) static void lowcomms_error_report(struct sock *sk) { - struct connection *con; + struct connection *con = sock2con(sk); void (*orig_report)(struct sock *) = NULL; struct inet_sock *inet; - con = sock2con(sk); - if (con == NULL) - goto out; - orig_report = listen_sock.sk_error_report; inet = inet_sk(sk); From c852a6d70698d5e4a9f3670ecabf9656ff00c73c Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Thu, 17 Nov 2022 17:11:55 -0500 Subject: [PATCH 34/37] fs: dlm: use saved sk_error_report() This patch changes the handling of calling the original sk_error_report() by not putting it on the stack and calling it later. If the listen_sock.sk_error_report() is NULL in this moment it indicates a bug in our implementation. Signed-off-by: Alexander Aring Signed-off-by: David Teigland --- fs/dlm/lowcomms.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c index 677e31144ca0b..643f9810ec358 100644 --- a/fs/dlm/lowcomms.c +++ b/fs/dlm/lowcomms.c @@ -569,11 +569,8 @@ int dlm_lowcomms_nodes_set_mark(int nodeid, unsigned int mark) static void lowcomms_error_report(struct sock *sk) { struct connection *con = sock2con(sk); - void (*orig_report)(struct sock *) = NULL; struct inet_sock *inet; - orig_report = listen_sock.sk_error_report; - inet = inet_sk(sk); switch (sk->sk_family) { case AF_INET: @@ -618,8 +615,7 @@ static void lowcomms_error_report(struct sock *sk) queue_work(send_workqueue, &con->swork); out: - if (orig_report) - orig_report(sk); + listen_sock.sk_error_report(sk); } static void restore_callbacks(struct socket *sock) From 1351975ac1377225cef5d858971e17252c06ff51 Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Thu, 17 Nov 2022 17:11:56 -0500 Subject: [PATCH 35/37] fs: dlm: don't init error value This patch removes a init of an error value to -EINVAL which is not necessary. Signed-off-by: Alexander Aring Signed-off-by: David Teigland --- fs/dlm/lowcomms.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c index 643f9810ec358..c6b91f67a2c26 100644 --- a/fs/dlm/lowcomms.c +++ b/fs/dlm/lowcomms.c @@ -1830,7 +1830,7 @@ static const struct dlm_proto_ops dlm_sctp_ops = { int dlm_lowcomms_start(void) { - int error = -EINVAL; + int error; init_local(); if (!dlm_local_count) { From dbb751ffab0b764720e360efd642ba6bf076d87f Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Thu, 17 Nov 2022 17:11:57 -0500 Subject: [PATCH 36/37] fs: dlm: parallelize lowcomms socket handling This patch is rework of lowcomms handling, the main goal was here to handle recvmsg() and sendpage() to run parallel. Parallel in two senses: 1. per connection and 2. that recvmsg()/sendpage() doesn't block each other. Currently recvmsg()/sendpage() cannot run parallel because two workqueues "dlm_recv" and "dlm_send" are ordered workqueues. That means only one work item can be executed. The amount of queue items will be increased about the amount of nodes being inside the cluster. The current two workqueues for sending and receiving can also block each other if the same connection is executed at the same time in dlm_recv and dlm_send workqueue because a per connection mutex for the socket handling. To make it more parallel we introduce one "dlm_io" workqueue which is not an ordered workqueue, the amount of workers are not limited. Due per connection flags SEND/RECV pending we schedule workers ordered per connection and per send and receive task. To get rid of the mutex blocking same workers to do socket handling we switched to a semaphore which handles socket operations as read lock and sock releases as write operations, to prevent sock_release() being called while the socket is being used. There might be more optimization removing the semaphore and replacing it with other synchronization mechanism, however due other circumstances e.g. othercon behaviour it seems complicated to doing this change. I added comments to remove the othercon handling and moving to a different synchronization mechanism as this is done. We need to do that to the next dlm major version upgrade because it is not backwards compatible with the current connect mechanism. The processing of dlm messages need to be still handled by a ordered workqueue. An dlm_process ordered workqueue was introduced which gets filled by the receive worker. This is probably the next bottleneck of DLM but the application can't currently parse dlm messages parallel. A comment was introduced to lift the workqueue context of dlm processing in a non-sleepable softirq to get messages processing done fast. Signed-off-by: Alexander Aring Signed-off-by: David Teigland --- fs/dlm/lowcomms.c | 1024 ++++++++++++++++++++++++--------------------- fs/dlm/midcomms.c | 45 +- fs/dlm/midcomms.h | 1 + 3 files changed, 586 insertions(+), 484 deletions(-) diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c index c6b91f67a2c26..799d1c36eabf3 100644 --- a/fs/dlm/lowcomms.c +++ b/fs/dlm/lowcomms.c @@ -63,35 +63,43 @@ #define NEEDED_RMEM (4*1024*1024) -/* Number of messages to send before rescheduling */ -#define MAX_SEND_MSG_COUNT 25 - struct connection { struct socket *sock; /* NULL if not connected */ uint32_t nodeid; /* So we know who we are in the list */ - struct mutex sock_mutex; + /* this semaphore is used to allow parallel recv/send in read + * lock mode. When we release a sock we need to held the write lock. + * + * However this is locking code and not nice. When we remove the + * othercon handling we can look into other mechanism to synchronize + * io handling to call sock_release() at the right time. + */ + struct rw_semaphore sock_lock; unsigned long flags; -#define CF_READ_PENDING 1 -#define CF_WRITE_PENDING 2 -#define CF_INIT_PENDING 4 +#define CF_APP_LIMITED 0 +#define CF_RECV_PENDING 1 +#define CF_SEND_PENDING 2 +#define CF_RECV_INTR 3 +#define CF_IO_STOP 4 #define CF_IS_OTHERCON 5 -#define CF_CLOSE 6 -#define CF_APP_LIMITED 7 -#define CF_CLOSING 8 -#define CF_CONNECTED 9 -#define CF_RECONNECT 10 -#define CF_DELAY_CONNECT 11 struct list_head writequeue; /* List of outgoing writequeue_entries */ spinlock_t writequeue_lock; int retries; -#define MAX_CONNECT_RETRIES 3 struct hlist_node list; + /* due some connect()/accept() races we currently have this cross over + * connection attempt second connection for one node. + * + * There is a solution to avoid the race by introducing a connect + * rule as e.g. our_nodeid > nodeid_to_connect who is allowed to + * connect. Otherside can connect but will only be considered that + * the other side wants to have a reconnect. + * + * However changing to this behaviour will break backwards compatible. + * In a DLM protocol major version upgrade we should remove this! + */ struct connection *othercon; - struct connection *sendcon; - struct work_struct rwork; /* Receive workqueue */ - struct work_struct swork; /* Send workqueue */ - unsigned char *rx_buf; - int rx_buflen; + struct work_struct rwork; /* receive worker */ + struct work_struct swork; /* send worker */ + unsigned char rx_leftover_buf[DLM_MAX_SOCKET_BUFSIZE]; int rx_leftover; int mark; int addr_count; @@ -136,6 +144,14 @@ struct dlm_msg { struct kref ref; }; +struct processqueue_entry { + unsigned char *buf; + int nodeid; + int buflen; + + struct list_head list; +}; + struct dlm_proto_ops { bool try_new_addr; const char *name; @@ -162,8 +178,8 @@ static struct sockaddr_storage dlm_local_addr[DLM_MAX_ADDR_COUNT]; static int dlm_local_count; /* Work queues */ -static struct workqueue_struct *recv_workqueue; -static struct workqueue_struct *send_workqueue; +static struct workqueue_struct *io_workqueue; +static struct workqueue_struct *process_workqueue; static struct hlist_head connection_hash[CONN_HASH_SIZE]; static DEFINE_SPINLOCK(connections_lock); @@ -171,14 +187,44 @@ DEFINE_STATIC_SRCU(connections_srcu); static const struct dlm_proto_ops *dlm_proto_ops; +#define DLM_IO_SUCCESS 0 +#define DLM_IO_END 1 +#define DLM_IO_EOF 2 +#define DLM_IO_RESCHED 3 + static void process_recv_sockets(struct work_struct *work); static void process_send_sockets(struct work_struct *work); +static void process_dlm_messages(struct work_struct *work); + +static DECLARE_WORK(process_work, process_dlm_messages); +static DEFINE_SPINLOCK(processqueue_lock); +static bool process_dlm_messages_pending; +static LIST_HEAD(processqueue); bool dlm_lowcomms_is_running(void) { return !!listen_con.sock; } +static void lowcomms_queue_swork(struct connection *con) +{ + WARN_ON_ONCE(!lockdep_is_held(&con->writequeue_lock)); + + if (!test_bit(CF_IO_STOP, &con->flags) && + !test_bit(CF_APP_LIMITED, &con->flags) && + !test_and_set_bit(CF_SEND_PENDING, &con->flags)) + queue_work(io_workqueue, &con->swork); +} + +static void lowcomms_queue_rwork(struct connection *con) +{ + WARN_ON_ONCE(!lockdep_sock_is_held(con->sock->sk)); + + if (!test_bit(CF_IO_STOP, &con->flags) && + !test_and_set_bit(CF_RECV_PENDING, &con->flags)) + queue_work(io_workqueue, &con->rwork); +} + static void writequeue_entry_ctor(void *data) { struct writequeue_entry *entry = data; @@ -225,21 +271,15 @@ static struct connection *__find_con(int nodeid, int r) return NULL; } -static int dlm_con_init(struct connection *con, int nodeid) +static void dlm_con_init(struct connection *con, int nodeid) { - con->rx_buflen = dlm_config.ci_buffer_size; - con->rx_buf = kmalloc(con->rx_buflen, GFP_NOFS); - if (!con->rx_buf) - return -ENOMEM; - con->nodeid = nodeid; - mutex_init(&con->sock_mutex); + init_rwsem(&con->sock_lock); INIT_LIST_HEAD(&con->writequeue); spin_lock_init(&con->writequeue_lock); INIT_WORK(&con->swork, process_send_sockets); INIT_WORK(&con->rwork, process_recv_sockets); - - return 0; + spin_lock_init(&con->addrs_lock); } /* @@ -249,7 +289,7 @@ static int dlm_con_init(struct connection *con, int nodeid) static struct connection *nodeid2con(int nodeid, gfp_t alloc) { struct connection *con, *tmp; - int r, ret; + int r; r = nodeid_hash(nodeid); con = __find_con(nodeid, r); @@ -260,11 +300,7 @@ static struct connection *nodeid2con(int nodeid, gfp_t alloc) if (!con) return NULL; - ret = dlm_con_init(con, nodeid); - if (ret) { - kfree(con); - return NULL; - } + dlm_con_init(con, nodeid); spin_lock(&connections_lock); /* Because multiple workqueues/threads calls this function it can @@ -276,7 +312,6 @@ static struct connection *nodeid2con(int nodeid, gfp_t alloc) tmp = __find_con(nodeid, r); if (tmp) { spin_unlock(&connections_lock); - kfree(con->rx_buf); kfree(con); return tmp; } @@ -287,18 +322,6 @@ static struct connection *nodeid2con(int nodeid, gfp_t alloc) return con; } -/* Loop round all connections */ -static void foreach_conn(void (*conn_func)(struct connection *c)) -{ - int i; - struct connection *con; - - for (i = 0; i < CONN_HASH_SIZE; i++) { - hlist_for_each_entry_rcu(con, &connection_hash[i], list) - conn_func(con); - } -} - static int addr_compare(const struct sockaddr_storage *x, const struct sockaddr_storage *y) { @@ -474,56 +497,38 @@ static void lowcomms_data_ready(struct sock *sk) { struct connection *con = sock2con(sk); - if (!test_and_set_bit(CF_READ_PENDING, &con->flags)) - queue_work(recv_workqueue, &con->rwork); -} - -static void lowcomms_listen_data_ready(struct sock *sk) -{ - queue_work(recv_workqueue, &listen_con.rwork); + set_bit(CF_RECV_INTR, &con->flags); + lowcomms_queue_rwork(con); } static void lowcomms_write_space(struct sock *sk) { struct connection *con = sock2con(sk); - if (!test_and_set_bit(CF_CONNECTED, &con->flags)) { - log_print("connected to node %d", con->nodeid); - queue_work(send_workqueue, &con->swork); - return; - } - clear_bit(SOCK_NOSPACE, &con->sock->flags); + spin_lock_bh(&con->writequeue_lock); if (test_and_clear_bit(CF_APP_LIMITED, &con->flags)) { con->sock->sk->sk_write_pending--; clear_bit(SOCKWQ_ASYNC_NOSPACE, &con->sock->flags); } - queue_work(send_workqueue, &con->swork); -} - -static inline void lowcomms_connect_sock(struct connection *con) -{ - if (test_bit(CF_CLOSE, &con->flags)) - return; - queue_work(send_workqueue, &con->swork); - cond_resched(); + lowcomms_queue_swork(con); + spin_unlock_bh(&con->writequeue_lock); } static void lowcomms_state_change(struct sock *sk) { /* SCTP layer is not calling sk_data_ready when the connection - * is done, so we catch the signal through here. Also, it - * doesn't switch socket state when entering shutdown, so we - * skip the write in that case. + * is done, so we catch the signal through here. */ - if (sk->sk_shutdown) { - if (sk->sk_shutdown == RCV_SHUTDOWN) - lowcomms_data_ready(sk); - } else if (sk->sk_state == TCP_ESTABLISHED) { - lowcomms_write_space(sk); - } + if (sk->sk_shutdown == RCV_SHUTDOWN) + lowcomms_data_ready(sk); +} + +static void lowcomms_listen_data_ready(struct sock *sk) +{ + queue_work(io_workqueue, &listen_con.rwork); } int dlm_lowcomms_connect_node(int nodeid) @@ -541,9 +546,16 @@ int dlm_lowcomms_connect_node(int nodeid) return -ENOENT; } - lowcomms_connect_sock(con); + down_read(&con->sock_lock); + if (!con->sock) { + spin_lock_bh(&con->writequeue_lock); + lowcomms_queue_swork(con); + spin_unlock_bh(&con->writequeue_lock); + } + up_read(&con->sock_lock); srcu_read_unlock(&connections_srcu, idx); + cond_resched(); return 0; } @@ -596,39 +608,23 @@ static void lowcomms_error_report(struct sock *sk) "invalid socket family %d set, " "sk_err=%d/%d\n", dlm_our_nodeid(), sk->sk_family, sk->sk_err, sk->sk_err_soft); - goto out; - } - - /* below sendcon only handling */ - if (test_bit(CF_IS_OTHERCON, &con->flags)) - con = con->sendcon; - - switch (sk->sk_err) { - case ECONNREFUSED: - set_bit(CF_DELAY_CONNECT, &con->flags); - break; - default: break; } - if (!test_and_set_bit(CF_RECONNECT, &con->flags)) - queue_work(send_workqueue, &con->swork); + dlm_midcomms_unack_msg_resend(con->nodeid); -out: listen_sock.sk_error_report(sk); } -static void restore_callbacks(struct socket *sock) +static void restore_callbacks(struct sock *sk) { - struct sock *sk = sock->sk; + WARN_ON_ONCE(!lockdep_sock_is_held(sk)); - lock_sock(sk); sk->sk_user_data = NULL; sk->sk_data_ready = listen_sock.sk_data_ready; sk->sk_state_change = listen_sock.sk_state_change; sk->sk_write_space = listen_sock.sk_write_space; sk->sk_error_report = listen_sock.sk_error_report; - release_sock(sk); } /* Make a socket active */ @@ -640,10 +636,10 @@ static void add_sock(struct socket *sock, struct connection *con) con->sock = sock; sk->sk_user_data = con; - /* Install a data_ready callback */ sk->sk_data_ready = lowcomms_data_ready; sk->sk_write_space = lowcomms_write_space; - sk->sk_state_change = lowcomms_state_change; + if (dlm_config.ci_protocol == DLM_PROTO_SCTP) + sk->sk_state_change = lowcomms_state_change; sk->sk_allocation = GFP_NOFS; sk->sk_error_report = lowcomms_error_report; release_sock(sk); @@ -705,37 +701,62 @@ static void free_entry(struct writequeue_entry *e) static void dlm_close_sock(struct socket **sock) { - if (*sock) { - restore_callbacks(*sock); - sock_release(*sock); - *sock = NULL; + lock_sock((*sock)->sk); + restore_callbacks((*sock)->sk); + release_sock((*sock)->sk); + + sock_release(*sock); + *sock = NULL; +} + +static void allow_connection_io(struct connection *con) +{ + if (con->othercon) + clear_bit(CF_IO_STOP, &con->othercon->flags); + clear_bit(CF_IO_STOP, &con->flags); +} + +static void stop_connection_io(struct connection *con) +{ + if (con->othercon) + stop_connection_io(con->othercon); + + down_write(&con->sock_lock); + if (con->sock) { + lock_sock(con->sock->sk); + restore_callbacks(con->sock->sk); + + spin_lock_bh(&con->writequeue_lock); + set_bit(CF_IO_STOP, &con->flags); + spin_unlock_bh(&con->writequeue_lock); + release_sock(con->sock->sk); + } else { + spin_lock_bh(&con->writequeue_lock); + set_bit(CF_IO_STOP, &con->flags); + spin_unlock_bh(&con->writequeue_lock); } + up_write(&con->sock_lock); + + cancel_work_sync(&con->swork); + cancel_work_sync(&con->rwork); } /* Close a remote connection and tidy up */ -static void close_connection(struct connection *con, bool and_other, - bool tx, bool rx) +static void close_connection(struct connection *con, bool and_other) { - bool closing = test_and_set_bit(CF_CLOSING, &con->flags); struct writequeue_entry *e; - if (tx && !closing && cancel_work_sync(&con->swork)) { - log_print("canceled swork for node %d", con->nodeid); - clear_bit(CF_WRITE_PENDING, &con->flags); - } - if (rx && !closing && cancel_work_sync(&con->rwork)) { - log_print("canceled rwork for node %d", con->nodeid); - clear_bit(CF_READ_PENDING, &con->flags); + if (con->othercon && and_other) + close_connection(con->othercon, false); + + down_write(&con->sock_lock); + if (!con->sock) { + up_write(&con->sock_lock); + return; } - mutex_lock(&con->sock_mutex); dlm_close_sock(&con->sock); - if (con->othercon && and_other) { - /* Will only re-enter once. */ - close_connection(con->othercon, false, tx, rx); - } - /* if we send a writequeue entry only a half way, we drop the * whole entry because reconnection and that we not start of the * middle of a msg which will confuse the other end. @@ -747,143 +768,209 @@ static void close_connection(struct connection *con, bool and_other, * our policy is to start on a clean state when disconnects, we don't * know what's send/received on transport layer in this case. */ - spin_lock(&con->writequeue_lock); + spin_lock_bh(&con->writequeue_lock); if (!list_empty(&con->writequeue)) { e = list_first_entry(&con->writequeue, struct writequeue_entry, list); if (e->dirty) free_entry(e); } - spin_unlock(&con->writequeue_lock); + spin_unlock_bh(&con->writequeue_lock); con->rx_leftover = 0; con->retries = 0; clear_bit(CF_APP_LIMITED, &con->flags); - clear_bit(CF_CONNECTED, &con->flags); - clear_bit(CF_DELAY_CONNECT, &con->flags); - clear_bit(CF_RECONNECT, &con->flags); - mutex_unlock(&con->sock_mutex); - clear_bit(CF_CLOSING, &con->flags); + clear_bit(CF_RECV_PENDING, &con->flags); + clear_bit(CF_SEND_PENDING, &con->flags); + up_write(&con->sock_lock); } -static int con_realloc_receive_buf(struct connection *con, int newlen) +static struct processqueue_entry *new_processqueue_entry(int nodeid, + int buflen) { - unsigned char *newbuf; - - newbuf = kmalloc(newlen, GFP_NOFS); - if (!newbuf) - return -ENOMEM; + struct processqueue_entry *pentry; - /* copy any leftover from last receive */ - if (con->rx_leftover) - memmove(newbuf, con->rx_buf, con->rx_leftover); + pentry = kmalloc(sizeof(*pentry), GFP_NOFS); + if (!pentry) + return NULL; - /* swap to new buffer space */ - kfree(con->rx_buf); - con->rx_buflen = newlen; - con->rx_buf = newbuf; + pentry->buf = kmalloc(buflen, GFP_NOFS); + if (!pentry->buf) { + kfree(pentry); + return NULL; + } - return 0; + pentry->nodeid = nodeid; + return pentry; } -/* Data received from remote end */ -static int receive_from_sock(struct connection *con) +static void free_processqueue_entry(struct processqueue_entry *pentry) { - struct msghdr msg; - struct kvec iov; - int ret, buflen; + kfree(pentry->buf); + kfree(pentry); +} + +struct dlm_processed_nodes { + int nodeid; - mutex_lock(&con->sock_mutex); + struct list_head list; +}; - if (con->sock == NULL) { - ret = -EAGAIN; - goto out_close; +static void add_processed_node(int nodeid, struct list_head *processed_nodes) +{ + struct dlm_processed_nodes *n; + + list_for_each_entry(n, processed_nodes, list) { + /* we already remembered this node */ + if (n->nodeid == nodeid) + return; } - /* realloc if we get new buffer size to read out */ - buflen = dlm_config.ci_buffer_size; - if (con->rx_buflen != buflen && con->rx_leftover <= buflen) { - ret = con_realloc_receive_buf(con, buflen); - if (ret < 0) - goto out_resched; + /* if it's fails in worst case we simple don't send an ack back. + * We try it next time. + */ + n = kmalloc(sizeof(*n), GFP_NOFS); + if (!n) + return; + + n->nodeid = nodeid; + list_add(&n->list, processed_nodes); +} + +static void process_dlm_messages(struct work_struct *work) +{ + struct dlm_processed_nodes *n, *n_tmp; + struct processqueue_entry *pentry; + LIST_HEAD(processed_nodes); + + spin_lock(&processqueue_lock); + pentry = list_first_entry_or_null(&processqueue, + struct processqueue_entry, list); + if (WARN_ON_ONCE(!pentry)) { + spin_unlock(&processqueue_lock); + return; } + list_del(&pentry->list); + spin_unlock(&processqueue_lock); + for (;;) { - /* calculate new buffer parameter regarding last receive and - * possible leftover bytes - */ - iov.iov_base = con->rx_buf + con->rx_leftover; - iov.iov_len = con->rx_buflen - con->rx_leftover; - - memset(&msg, 0, sizeof(msg)); - msg.msg_flags = MSG_DONTWAIT | MSG_NOSIGNAL; - ret = kernel_recvmsg(con->sock, &msg, &iov, 1, iov.iov_len, - msg.msg_flags); - trace_dlm_recv(con->nodeid, ret); - if (ret == -EAGAIN) + dlm_process_incoming_buffer(pentry->nodeid, pentry->buf, + pentry->buflen); + add_processed_node(pentry->nodeid, &processed_nodes); + free_processqueue_entry(pentry); + + spin_lock(&processqueue_lock); + pentry = list_first_entry_or_null(&processqueue, + struct processqueue_entry, list); + if (!pentry) { + process_dlm_messages_pending = false; + spin_unlock(&processqueue_lock); break; - else if (ret <= 0) - goto out_close; - - /* new buflen according readed bytes and leftover from last receive */ - buflen = ret + con->rx_leftover; - ret = dlm_process_incoming_buffer(con->nodeid, con->rx_buf, buflen); - if (ret < 0) - goto out_close; - - /* calculate leftover bytes from process and put it into begin of - * the receive buffer, so next receive we have the full message - * at the start address of the receive buffer. - */ - con->rx_leftover = buflen - ret; - if (con->rx_leftover) { - memmove(con->rx_buf, con->rx_buf + ret, - con->rx_leftover); } + + list_del(&pentry->list); + spin_unlock(&processqueue_lock); + } + + /* send ack back after we processed couple of messages */ + list_for_each_entry_safe(n, n_tmp, &processed_nodes, list) { + list_del(&n->list); + dlm_midcomms_receive_done(n->nodeid); + kfree(n); + } +} + +/* Data received from remote end */ +static int receive_from_sock(struct connection *con, int buflen) +{ + struct processqueue_entry *pentry; + int ret, buflen_real; + struct msghdr msg; + struct kvec iov; + + pentry = new_processqueue_entry(con->nodeid, buflen); + if (!pentry) + return DLM_IO_RESCHED; + + memcpy(pentry->buf, con->rx_leftover_buf, con->rx_leftover); + + /* calculate new buffer parameter regarding last receive and + * possible leftover bytes + */ + iov.iov_base = pentry->buf + con->rx_leftover; + iov.iov_len = buflen - con->rx_leftover; + + memset(&msg, 0, sizeof(msg)); + msg.msg_flags = MSG_DONTWAIT | MSG_NOSIGNAL; + clear_bit(CF_RECV_INTR, &con->flags); +again: + ret = kernel_recvmsg(con->sock, &msg, &iov, 1, iov.iov_len, + msg.msg_flags); + trace_dlm_recv(con->nodeid, ret); + if (ret == -EAGAIN) { + lock_sock(con->sock->sk); + if (test_and_clear_bit(CF_RECV_INTR, &con->flags)) { + release_sock(con->sock->sk); + goto again; + } + + clear_bit(CF_RECV_PENDING, &con->flags); + release_sock(con->sock->sk); + free_processqueue_entry(pentry); + return DLM_IO_END; + } else if (ret == 0) { + /* close will clear CF_RECV_PENDING */ + free_processqueue_entry(pentry); + return DLM_IO_EOF; + } else if (ret < 0) { + free_processqueue_entry(pentry); + return ret; } - dlm_midcomms_receive_done(con->nodeid); - mutex_unlock(&con->sock_mutex); - return 0; + /* new buflen according readed bytes and leftover from last receive */ + buflen_real = ret + con->rx_leftover; + ret = dlm_validate_incoming_buffer(con->nodeid, pentry->buf, + buflen_real); + if (ret < 0) { + free_processqueue_entry(pentry); + return ret; + } -out_resched: - if (!test_and_set_bit(CF_READ_PENDING, &con->flags)) - queue_work(recv_workqueue, &con->rwork); - mutex_unlock(&con->sock_mutex); - return -EAGAIN; - -out_close: - if (ret == 0) { - log_print("connection %p got EOF from %d", - con, con->nodeid); - - mutex_unlock(&con->sock_mutex); - close_connection(con, false, true, false); - /* signal to breaking receive worker */ - ret = -1; - } else { - mutex_unlock(&con->sock_mutex); + pentry->buflen = ret; + + /* calculate leftover bytes from process and put it into begin of + * the receive buffer, so next receive we have the full message + * at the start address of the receive buffer. + */ + con->rx_leftover = buflen_real - ret; + memmove(con->rx_leftover_buf, pentry->buf + ret, + con->rx_leftover); + + spin_lock(&processqueue_lock); + list_add_tail(&pentry->list, &processqueue); + if (!process_dlm_messages_pending) { + process_dlm_messages_pending = true; + queue_work(process_workqueue, &process_work); } - return ret; + spin_unlock(&processqueue_lock); + + return DLM_IO_SUCCESS; } /* Listening socket is busy, accept a connection */ -static int accept_from_sock(struct listen_connection *con) +static int accept_from_sock(void) { - int result; struct sockaddr_storage peeraddr; - struct socket *newsock; - int len, idx; - int nodeid; + int len, idx, result, nodeid; struct connection *newcon; - struct connection *addcon; + struct socket *newsock; unsigned int mark; - if (!con->sock) - return -ENOTCONN; - - result = kernel_accept(con->sock, &newsock, O_NONBLOCK); - if (result < 0) + result = kernel_accept(listen_con.sock, &newsock, O_NONBLOCK); + if (result == -EAGAIN) + return DLM_IO_END; + else if (result < 0) goto accept_err; /* Get the connected socket's peer */ @@ -940,7 +1027,7 @@ static int accept_from_sock(struct listen_connection *con) sock_set_mark(newsock->sk, mark); - mutex_lock(&newcon->sock_mutex); + down_write(&newcon->sock_lock); if (newcon->sock) { struct connection *othercon = newcon->othercon; @@ -948,63 +1035,50 @@ static int accept_from_sock(struct listen_connection *con) othercon = kzalloc(sizeof(*othercon), GFP_NOFS); if (!othercon) { log_print("failed to allocate incoming socket"); - mutex_unlock(&newcon->sock_mutex); + up_write(&newcon->sock_lock); srcu_read_unlock(&connections_srcu, idx); result = -ENOMEM; goto accept_err; } - result = dlm_con_init(othercon, nodeid); - if (result < 0) { - kfree(othercon); - mutex_unlock(&newcon->sock_mutex); - srcu_read_unlock(&connections_srcu, idx); - goto accept_err; - } - - lockdep_set_subclass(&othercon->sock_mutex, 1); - set_bit(CF_IS_OTHERCON, &othercon->flags); + dlm_con_init(othercon, nodeid); + lockdep_set_subclass(&othercon->sock_lock, 1); newcon->othercon = othercon; - othercon->sendcon = newcon; + set_bit(CF_IS_OTHERCON, &othercon->flags); } else { /* close other sock con if we have something new */ - close_connection(othercon, false, true, false); + close_connection(othercon, false); } - mutex_lock(&othercon->sock_mutex); + down_write(&othercon->sock_lock); add_sock(newsock, othercon); - addcon = othercon; - mutex_unlock(&othercon->sock_mutex); + + /* check if we receved something while adding */ + lock_sock(othercon->sock->sk); + lowcomms_queue_rwork(othercon); + release_sock(othercon->sock->sk); + up_write(&othercon->sock_lock); } else { /* accept copies the sk after we've saved the callbacks, so we don't want to save them a second time or comm errors will result in calling sk_error_report recursively. */ add_sock(newsock, newcon); - addcon = newcon; - } - - set_bit(CF_CONNECTED, &addcon->flags); - mutex_unlock(&newcon->sock_mutex); - - /* - * Add it to the active queue in case we got data - * between processing the accept adding the socket - * to the read_sockets list - */ - if (!test_and_set_bit(CF_READ_PENDING, &addcon->flags)) - queue_work(recv_workqueue, &addcon->rwork); + /* check if we receved something while adding */ + lock_sock(newcon->sock->sk); + lowcomms_queue_rwork(newcon); + release_sock(newcon->sock->sk); + } + up_write(&newcon->sock_lock); srcu_read_unlock(&connections_srcu, idx); - return 0; + return DLM_IO_SUCCESS; accept_err: if (newsock) sock_release(newsock); - if (result != -EAGAIN) - log_print("error accepting connection from node: %d", result); return result; } @@ -1098,7 +1172,7 @@ static struct writequeue_entry *new_wq_entry(struct connection *con, int len, { struct writequeue_entry *e; - spin_lock(&con->writequeue_lock); + spin_lock_bh(&con->writequeue_lock); if (!list_empty(&con->writequeue)) { e = list_last_entry(&con->writequeue, struct writequeue_entry, list); if (DLM_WQ_REMAIN_BYTES(e) >= len) { @@ -1127,7 +1201,7 @@ static struct writequeue_entry *new_wq_entry(struct connection *con, int len, list_add_tail(&e->list, &con->writequeue); out: - spin_unlock(&con->writequeue_lock); + spin_unlock_bh(&con->writequeue_lock); return e; }; @@ -1176,7 +1250,7 @@ struct dlm_msg *dlm_lowcomms_new_msg(int nodeid, int len, gfp_t allocation, len < sizeof(struct dlm_header)) { BUILD_BUG_ON(PAGE_SIZE < DLM_MAX_SOCKET_BUFSIZE); log_print("failed to allocate a buffer of size %d", len); - WARN_ON(1); + WARN_ON_ONCE(1); return NULL; } @@ -1207,7 +1281,7 @@ static void _dlm_lowcomms_commit_msg(struct dlm_msg *msg) struct connection *con = e->con; int users; - spin_lock(&con->writequeue_lock); + spin_lock_bh(&con->writequeue_lock); kref_get(&msg->ref); list_add(&msg->list, &e->msgs); @@ -1216,13 +1290,11 @@ static void _dlm_lowcomms_commit_msg(struct dlm_msg *msg) goto out; e->len = DLM_WQ_LENGTH_BYTES(e); - spin_unlock(&con->writequeue_lock); - queue_work(send_workqueue, &con->swork); - return; + lowcomms_queue_swork(con); out: - spin_unlock(&con->writequeue_lock); + spin_unlock_bh(&con->writequeue_lock); return; } @@ -1244,7 +1316,7 @@ void dlm_lowcomms_put_msg(struct dlm_msg *msg) kref_put(&msg->ref, dlm_msg_release); } -/* does not held connections_srcu, usage workqueue only */ +/* does not held connections_srcu, usage lowcomms_error_report only */ int dlm_lowcomms_resend_msg(struct dlm_msg *msg) { struct dlm_msg *msg_resend; @@ -1270,88 +1342,78 @@ int dlm_lowcomms_resend_msg(struct dlm_msg *msg) } /* Send a message */ -static void send_to_sock(struct connection *con) +static int send_to_sock(struct connection *con) { const int msg_flags = MSG_DONTWAIT | MSG_NOSIGNAL; struct writequeue_entry *e; int len, offset, ret; - int count; - -again: - count = 0; - mutex_lock(&con->sock_mutex); - if (con->sock == NULL) - goto out_connect; - - spin_lock(&con->writequeue_lock); - for (;;) { - e = con_next_wq(con); - if (!e) - break; - - len = e->len; - offset = e->offset; - BUG_ON(len == 0 && e->users == 0); - spin_unlock(&con->writequeue_lock); - - ret = kernel_sendpage(con->sock, e->page, offset, len, - msg_flags); - trace_dlm_send(con->nodeid, ret); - if (ret == -EAGAIN || ret == 0) { - if (ret == -EAGAIN && - test_bit(SOCKWQ_ASYNC_NOSPACE, &con->sock->flags) && - !test_and_set_bit(CF_APP_LIMITED, &con->flags)) { - /* Notify TCP that we're limited by the - * application window size. - */ - set_bit(SOCK_NOSPACE, &con->sock->flags); - con->sock->sk->sk_write_pending++; - } - cond_resched(); - goto out; - } else if (ret < 0) - goto out; + spin_lock_bh(&con->writequeue_lock); + e = con_next_wq(con); + if (!e) { + clear_bit(CF_SEND_PENDING, &con->flags); + spin_unlock_bh(&con->writequeue_lock); + return DLM_IO_END; + } - spin_lock(&con->writequeue_lock); - writequeue_entry_complete(e, ret); + len = e->len; + offset = e->offset; + WARN_ON_ONCE(len == 0 && e->users == 0); + spin_unlock_bh(&con->writequeue_lock); - /* Don't starve people filling buffers */ - if (++count >= MAX_SEND_MSG_COUNT) { - spin_unlock(&con->writequeue_lock); - mutex_unlock(&con->sock_mutex); - cond_resched(); - goto again; + ret = kernel_sendpage(con->sock, e->page, offset, len, + msg_flags); + trace_dlm_send(con->nodeid, ret); + if (ret == -EAGAIN || ret == 0) { + lock_sock(con->sock->sk); + spin_lock_bh(&con->writequeue_lock); + if (test_bit(SOCKWQ_ASYNC_NOSPACE, &con->sock->flags) && + !test_and_set_bit(CF_APP_LIMITED, &con->flags)) { + /* Notify TCP that we're limited by the + * application window size. + */ + set_bit(SOCK_NOSPACE, &con->sock->sk->sk_socket->flags); + con->sock->sk->sk_write_pending++; + + clear_bit(CF_SEND_PENDING, &con->flags); + spin_unlock_bh(&con->writequeue_lock); + release_sock(con->sock->sk); + + /* wait for write_space() event */ + return DLM_IO_END; } + spin_unlock_bh(&con->writequeue_lock); + release_sock(con->sock->sk); + + return DLM_IO_RESCHED; + } else if (ret < 0) { + return ret; } - spin_unlock(&con->writequeue_lock); -out: - mutex_unlock(&con->sock_mutex); - return; + spin_lock_bh(&con->writequeue_lock); + writequeue_entry_complete(e, ret); + spin_unlock_bh(&con->writequeue_lock); -out_connect: - mutex_unlock(&con->sock_mutex); - queue_work(send_workqueue, &con->swork); - cond_resched(); + return DLM_IO_SUCCESS; } static void clean_one_writequeue(struct connection *con) { struct writequeue_entry *e, *safe; - spin_lock(&con->writequeue_lock); + spin_lock_bh(&con->writequeue_lock); list_for_each_entry_safe(e, safe, &con->writequeue, list) { free_entry(e); } - spin_unlock(&con->writequeue_lock); + spin_unlock_bh(&con->writequeue_lock); } static void connection_release(struct rcu_head *rcu) { struct connection *con = container_of(rcu, struct connection, rcu); - kfree(con->rx_buf); + WARN_ON_ONCE(!list_empty(&con->writequeue)); + WARN_ON_ONCE(con->sock); kfree(con); } @@ -1371,12 +1433,14 @@ int dlm_lowcomms_close(int nodeid) return -ENOENT; } + stop_connection_io(con); + log_print("io handling for node: %d stopped", nodeid); + close_connection(con, true); + spin_lock(&connections_lock); hlist_del_rcu(&con->list); spin_unlock(&connections_lock); - close_connection(con, true, true, true); - clean_one_writequeue(con); call_srcu(&connections_srcu, &con->rcu, connection_release); if (con->othercon) { @@ -1386,148 +1450,238 @@ int dlm_lowcomms_close(int nodeid) } srcu_read_unlock(&connections_srcu, idx); + /* for debugging we print when we are done to compare with other + * messages in between. This function need to be correctly synchronized + * with io handling + */ + log_print("closing connection to node %d done", nodeid); + return 0; } -/* Receive workqueue function */ +/* Receive worker function */ static void process_recv_sockets(struct work_struct *work) { struct connection *con = container_of(work, struct connection, rwork); + int ret, buflen; - clear_bit(CF_READ_PENDING, &con->flags); - receive_from_sock(con); + down_read(&con->sock_lock); + if (!con->sock) { + up_read(&con->sock_lock); + return; + } + + buflen = READ_ONCE(dlm_config.ci_buffer_size); + do { + ret = receive_from_sock(con, buflen); + } while (ret == DLM_IO_SUCCESS); + up_read(&con->sock_lock); + + switch (ret) { + case DLM_IO_END: + /* CF_RECV_PENDING cleared */ + break; + case DLM_IO_EOF: + close_connection(con, false); + /* CF_RECV_PENDING cleared */ + break; + case DLM_IO_RESCHED: + cond_resched(); + queue_work(io_workqueue, &con->rwork); + /* CF_RECV_PENDING not cleared */ + break; + default: + if (ret < 0) { + if (test_bit(CF_IS_OTHERCON, &con->flags)) { + close_connection(con, false); + } else { + spin_lock_bh(&con->writequeue_lock); + lowcomms_queue_swork(con); + spin_unlock_bh(&con->writequeue_lock); + } + + /* CF_RECV_PENDING cleared for othercon + * we trigger send queue if not already done + * and process_send_sockets will handle it + */ + break; + } + + WARN_ON_ONCE(1); + break; + } } static void process_listen_recv_socket(struct work_struct *work) { int ret; + if (WARN_ON_ONCE(!listen_con.sock)) + return; + do { - ret = accept_from_sock(&listen_con); - } while (!ret); + ret = accept_from_sock(); + } while (ret == DLM_IO_SUCCESS); + + if (ret < 0) + log_print("critical error accepting connection: %d", ret); } -static void dlm_connect(struct connection *con) +static int dlm_connect(struct connection *con) { struct sockaddr_storage addr; int result, addr_len; struct socket *sock; unsigned int mark; - /* Some odd races can cause double-connects, ignore them */ - if (con->retries++ > MAX_CONNECT_RETRIES) - return; - - if (con->sock) { - log_print("node %d already connected.", con->nodeid); - return; - } - memset(&addr, 0, sizeof(addr)); result = nodeid_to_addr(con->nodeid, &addr, NULL, dlm_proto_ops->try_new_addr, &mark); if (result < 0) { log_print("no address for nodeid %d", con->nodeid); - return; + return result; } /* Create a socket to communicate with */ result = sock_create_kern(&init_net, dlm_local_addr[0].ss_family, SOCK_STREAM, dlm_proto_ops->proto, &sock); if (result < 0) - goto socket_err; + return result; sock_set_mark(sock->sk, mark); dlm_proto_ops->sockopts(sock); - add_sock(sock, con); - result = dlm_proto_ops->bind(sock); - if (result < 0) - goto add_sock_err; + if (result < 0) { + sock_release(sock); + return result; + } + + add_sock(sock, con); log_print_ratelimited("connecting to %d", con->nodeid); make_sockaddr(&addr, dlm_config.ci_tcp_port, &addr_len); result = dlm_proto_ops->connect(con, sock, (struct sockaddr *)&addr, addr_len); - if (result < 0) - goto add_sock_err; - - return; - -add_sock_err: - dlm_close_sock(&con->sock); + switch (result) { + case -EINPROGRESS: + /* not an error */ + fallthrough; + case 0: + break; + default: + if (result < 0) + dlm_close_sock(&con->sock); -socket_err: - /* - * Some errors are fatal and this list might need adjusting. For other - * errors we try again until the max number of retries is reached. - */ - if (result != -EHOSTUNREACH && - result != -ENETUNREACH && - result != -ENETDOWN && - result != -EINVAL && - result != -EPROTONOSUPPORT) { - log_print("connect %d try %d error %d", con->nodeid, - con->retries, result); - msleep(1000); - lowcomms_connect_sock(con); + break; } + + return result; } -/* Send workqueue function */ +/* Send worker function */ static void process_send_sockets(struct work_struct *work) { struct connection *con = container_of(work, struct connection, swork); + int ret; - WARN_ON(test_bit(CF_IS_OTHERCON, &con->flags)); + WARN_ON_ONCE(test_bit(CF_IS_OTHERCON, &con->flags)); + + down_read(&con->sock_lock); + if (!con->sock) { + up_read(&con->sock_lock); + down_write(&con->sock_lock); + if (!con->sock) { + ret = dlm_connect(con); + switch (ret) { + case 0: + break; + case -EINPROGRESS: + /* avoid spamming resched on connection + * we might can switch to a state_change + * event based mechanism if established + */ + msleep(100); + break; + default: + /* CF_SEND_PENDING not cleared */ + up_write(&con->sock_lock); + log_print("connect to node %d try %d error %d", + con->nodeid, con->retries++, ret); + msleep(1000); + /* For now we try forever to reconnect. In + * future we should send a event to cluster + * manager to fence itself after certain amount + * of retries. + */ + queue_work(io_workqueue, &con->swork); + return; + } + } + downgrade_write(&con->sock_lock); + } - clear_bit(CF_WRITE_PENDING, &con->flags); + do { + ret = send_to_sock(con); + } while (ret == DLM_IO_SUCCESS); + up_read(&con->sock_lock); - if (test_and_clear_bit(CF_RECONNECT, &con->flags)) { - close_connection(con, false, false, true); - dlm_midcomms_unack_msg_resend(con->nodeid); - } + switch (ret) { + case DLM_IO_END: + /* CF_SEND_PENDING cleared */ + break; + case DLM_IO_RESCHED: + /* CF_SEND_PENDING not cleared */ + cond_resched(); + queue_work(io_workqueue, &con->swork); + break; + default: + if (ret < 0) { + close_connection(con, false); - if (con->sock == NULL) { - if (test_and_clear_bit(CF_DELAY_CONNECT, &con->flags)) - msleep(1000); + /* CF_SEND_PENDING cleared */ + spin_lock_bh(&con->writequeue_lock); + lowcomms_queue_swork(con); + spin_unlock_bh(&con->writequeue_lock); + break; + } - mutex_lock(&con->sock_mutex); - dlm_connect(con); - mutex_unlock(&con->sock_mutex); + WARN_ON_ONCE(1); + break; } - - if (!list_empty(&con->writequeue)) - send_to_sock(con); } static void work_stop(void) { - if (recv_workqueue) { - destroy_workqueue(recv_workqueue); - recv_workqueue = NULL; + if (io_workqueue) { + destroy_workqueue(io_workqueue); + io_workqueue = NULL; } - if (send_workqueue) { - destroy_workqueue(send_workqueue); - send_workqueue = NULL; + if (process_workqueue) { + destroy_workqueue(process_workqueue); + process_workqueue = NULL; } } static int work_start(void) { - recv_workqueue = alloc_ordered_workqueue("dlm_recv", WQ_MEM_RECLAIM); - if (!recv_workqueue) { - log_print("can't start dlm_recv"); + io_workqueue = alloc_workqueue("dlm_io", WQ_HIGHPRI | WQ_MEM_RECLAIM, + 0); + if (!io_workqueue) { + log_print("can't start dlm_io"); return -ENOMEM; } - send_workqueue = alloc_ordered_workqueue("dlm_send", WQ_MEM_RECLAIM); - if (!send_workqueue) { - log_print("can't start dlm_send"); - destroy_workqueue(recv_workqueue); - recv_workqueue = NULL; + /* ordered dlm message process queue, + * should be converted to a tasklet + */ + process_workqueue = alloc_ordered_workqueue("dlm_process", + WQ_HIGHPRI | WQ_MEM_RECLAIM); + if (!process_workqueue) { + log_print("can't start dlm_process"); + destroy_workqueue(io_workqueue); + io_workqueue = NULL; return -ENOMEM; } @@ -1543,6 +1697,8 @@ void dlm_lowcomms_shutdown(void) cancel_work_sync(&listen_con.rwork); dlm_close_sock(&listen_con.sock); + + flush_workqueue(process_workqueue); } void dlm_lowcomms_shutdown_node(int nodeid, bool force) @@ -1558,79 +1714,19 @@ void dlm_lowcomms_shutdown_node(int nodeid, bool force) } flush_work(&con->swork); + stop_connection_io(con); WARN_ON_ONCE(!force && !list_empty(&con->writequeue)); + close_connection(con, true); clean_one_writequeue(con); if (con->othercon) clean_one_writequeue(con->othercon); - close_connection(con, true, true, true); + allow_connection_io(con); srcu_read_unlock(&connections_srcu, idx); } -static void _stop_conn(struct connection *con, bool and_other) -{ - mutex_lock(&con->sock_mutex); - set_bit(CF_CLOSE, &con->flags); - set_bit(CF_READ_PENDING, &con->flags); - set_bit(CF_WRITE_PENDING, &con->flags); - if (con->sock && con->sock->sk) { - lock_sock(con->sock->sk); - con->sock->sk->sk_user_data = NULL; - release_sock(con->sock->sk); - } - if (con->othercon && and_other) - _stop_conn(con->othercon, false); - mutex_unlock(&con->sock_mutex); -} - -static void stop_conn(struct connection *con) -{ - _stop_conn(con, true); -} - -static void free_conn(struct connection *con) -{ - close_connection(con, true, true, true); -} - -static void work_flush(void) -{ - int ok; - int i; - struct connection *con; - - do { - ok = 1; - foreach_conn(stop_conn); - if (recv_workqueue) - flush_workqueue(recv_workqueue); - if (send_workqueue) - flush_workqueue(send_workqueue); - for (i = 0; i < CONN_HASH_SIZE && ok; i++) { - hlist_for_each_entry_rcu(con, &connection_hash[i], - list) { - ok &= test_bit(CF_READ_PENDING, &con->flags); - ok &= test_bit(CF_WRITE_PENDING, &con->flags); - if (con->othercon) { - ok &= test_bit(CF_READ_PENDING, - &con->othercon->flags); - ok &= test_bit(CF_WRITE_PENDING, - &con->othercon->flags); - } - } - } - } while (!ok); -} - void dlm_lowcomms_stop(void) { - int idx; - - idx = srcu_read_lock(&connections_srcu); - work_flush(); - foreach_conn(free_conn); - srcu_read_unlock(&connections_srcu, idx); work_stop(); - dlm_proto_ops = NULL; } @@ -1709,17 +1805,7 @@ static int dlm_tcp_bind(struct socket *sock) static int dlm_tcp_connect(struct connection *con, struct socket *sock, struct sockaddr *addr, int addr_len) { - int ret; - - ret = sock->ops->connect(sock, addr, addr_len, O_NONBLOCK); - switch (ret) { - case -EINPROGRESS: - fallthrough; - case 0: - return 0; - } - - return ret; + return sock->ops->connect(sock, addr, addr_len, O_NONBLOCK); } static int dlm_tcp_listen_validate(void) @@ -1784,13 +1870,7 @@ static int dlm_sctp_connect(struct connection *con, struct socket *sock, sock_set_sndtimeo(sock->sk, 5); ret = sock->ops->connect(sock, addr, addr_len, 0); sock_set_sndtimeo(sock->sk, 0); - if (ret < 0) - return ret; - - if (!test_and_set_bit(CF_CONNECTED, &con->flags)) - log_print("connected to node %d", con->nodeid); - - return 0; + return ret; } static int dlm_sctp_listen_validate(void) diff --git a/fs/dlm/midcomms.c b/fs/dlm/midcomms.c index b0e8bdcaab1bc..fc015a6abe178 100644 --- a/fs/dlm/midcomms.c +++ b/fs/dlm/midcomms.c @@ -306,11 +306,11 @@ static void dlm_send_queue_flush(struct midcomms_node *node) pr_debug("flush midcomms send queue of node %d\n", node->nodeid); rcu_read_lock(); - spin_lock(&node->send_queue_lock); + spin_lock_bh(&node->send_queue_lock); list_for_each_entry_rcu(mh, &node->send_queue, list) { dlm_mhandle_delete(node, mh); } - spin_unlock(&node->send_queue_lock); + spin_unlock_bh(&node->send_queue_lock); rcu_read_unlock(); } @@ -437,7 +437,7 @@ static void dlm_receive_ack(struct midcomms_node *node, uint32_t seq) } } - spin_lock(&node->send_queue_lock); + spin_lock_bh(&node->send_queue_lock); list_for_each_entry_rcu(mh, &node->send_queue, list) { if (before(mh->seq, seq)) { dlm_mhandle_delete(node, mh); @@ -446,7 +446,7 @@ static void dlm_receive_ack(struct midcomms_node *node, uint32_t seq) break; } } - spin_unlock(&node->send_queue_lock); + spin_unlock_bh(&node->send_queue_lock); rcu_read_unlock(); } @@ -890,12 +890,7 @@ static void dlm_midcomms_receive_buffer_3_1(union dlm_packet *p, int nodeid) dlm_receive_buffer(p, nodeid); } -/* - * Called from the low-level comms layer to process a buffer of - * commands. - */ - -int dlm_process_incoming_buffer(int nodeid, unsigned char *buf, int len) +int dlm_validate_incoming_buffer(int nodeid, unsigned char *buf, int len) { const unsigned char *ptr = buf; const struct dlm_header *hd; @@ -930,6 +925,32 @@ int dlm_process_incoming_buffer(int nodeid, unsigned char *buf, int len) if (msglen > len) break; + ret += msglen; + len -= msglen; + ptr += msglen; + } + + return ret; +} + +/* + * Called from the low-level comms layer to process a buffer of + * commands. + */ +int dlm_process_incoming_buffer(int nodeid, unsigned char *buf, int len) +{ + const unsigned char *ptr = buf; + const struct dlm_header *hd; + uint16_t msglen; + int ret = 0; + + while (len >= sizeof(struct dlm_header)) { + hd = (struct dlm_header *)ptr; + + msglen = le16_to_cpu(hd->h_length); + if (msglen > len) + break; + switch (hd->h_version) { case cpu_to_le32(DLM_VERSION_3_1): dlm_midcomms_receive_buffer_3_1((union dlm_packet *)ptr, nodeid); @@ -1046,9 +1067,9 @@ static void midcomms_new_msg_cb(void *data) atomic_inc(&mh->node->send_queue_cnt); - spin_lock(&mh->node->send_queue_lock); + spin_lock_bh(&mh->node->send_queue_lock); list_add_tail_rcu(&mh->list, &mh->node->send_queue); - spin_unlock(&mh->node->send_queue_lock); + spin_unlock_bh(&mh->node->send_queue_lock); mh->seq = mh->node->seq_send++; } diff --git a/fs/dlm/midcomms.h b/fs/dlm/midcomms.h index 69296552d5add..bea1cee4279c3 100644 --- a/fs/dlm/midcomms.h +++ b/fs/dlm/midcomms.h @@ -14,6 +14,7 @@ struct midcomms_node; +int dlm_validate_incoming_buffer(int nodeid, unsigned char *buf, int len); int dlm_process_incoming_buffer(int nodeid, unsigned char *buf, int buflen); struct dlm_mhandle *dlm_midcomms_get_mhandle(int nodeid, int len, gfp_t allocation, char **ppc); From 7a5e9f1f83e3271a9f05933a80b870fe55ebbb3d Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Tue, 22 Nov 2022 09:48:01 -0500 Subject: [PATCH 37/37] fs: dlm: fix building without lockdep This patch uses assert_spin_locked() instead of lockdep_is_held() where it's available to use because lockdep_is_held() is only available if CONFIG_LOCKDEP is set. In other cases like lockdep_sock_is_held() we surround it by a CONFIG_LOCKDEP idef. Fixes: dbb751ffab0b ("fs: dlm: parallelize lowcomms socket handling") Reported-by: kernel test robot Signed-off-by: Alexander Aring Signed-off-by: David Teigland --- fs/dlm/lowcomms.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c index 799d1c36eabf3..8b80ca0cd65fd 100644 --- a/fs/dlm/lowcomms.c +++ b/fs/dlm/lowcomms.c @@ -208,7 +208,7 @@ bool dlm_lowcomms_is_running(void) static void lowcomms_queue_swork(struct connection *con) { - WARN_ON_ONCE(!lockdep_is_held(&con->writequeue_lock)); + assert_spin_locked(&con->writequeue_lock); if (!test_bit(CF_IO_STOP, &con->flags) && !test_bit(CF_APP_LIMITED, &con->flags) && @@ -218,7 +218,9 @@ static void lowcomms_queue_swork(struct connection *con) static void lowcomms_queue_rwork(struct connection *con) { +#ifdef CONFIG_LOCKDEP WARN_ON_ONCE(!lockdep_sock_is_held(con->sock->sk)); +#endif if (!test_bit(CF_IO_STOP, &con->flags) && !test_and_set_bit(CF_RECV_PENDING, &con->flags)) @@ -618,7 +620,9 @@ static void lowcomms_error_report(struct sock *sk) static void restore_callbacks(struct sock *sk) { +#ifdef CONFIG_LOCKDEP WARN_ON_ONCE(!lockdep_sock_is_held(sk)); +#endif sk->sk_user_data = NULL; sk->sk_data_ready = listen_sock.sk_data_ready;