From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexander Aring Date: Fri, 26 Mar 2021 13:33:37 -0400 Subject: [Cluster-devel] [PATCHv3 dlm/next 8/8] fs: dlm: don't allow half transmitted messages In-Reply-To: <20210326173337.44231-1-aahringo@redhat.com> References: <20210326173337.44231-1-aahringo@redhat.com> Message-ID: <20210326173337.44231-9-aahringo@redhat.com> List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit This patch will clean a dirty page buffer if a reconnect occurs. If a page buffer was half transmitted we cannot start inside the middle of a dlm message if a node connects again. I observed invalid length receptions errors and was guessing that this behaviour occurs, after this patch I never saw an invalid message length again. This patch might drops more messages for dlm version 3.1 but 3.1 can't deal with half messages as well, for 3.2 it might trigger more re-transmissions but will not leave dlm in a broken state. Signed-off-by: Alexander Aring --- fs/dlm/lowcomms.c | 84 ++++++++++++++++++++++++++++++----------------- 1 file changed, 54 insertions(+), 30 deletions(-) diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c index 5cb6d7c9ddc1..d70a554c6b2b 100644 --- a/fs/dlm/lowcomms.c +++ b/fs/dlm/lowcomms.c @@ -112,6 +112,7 @@ struct writequeue_entry { int len; int end; int users; + bool dirty; struct connection *con; struct list_head msgs; struct kref ref; @@ -671,6 +672,36 @@ static void make_sockaddr(struct sockaddr_storage *saddr, uint16_t port, memset((char *)saddr + *addr_len, 0, sizeof(struct sockaddr_storage) - *addr_len); } +static void dlm_page_release(struct kref *kref) +{ + struct writequeue_entry *e = container_of(kref, struct writequeue_entry, + ref); + + __free_page(e->page); + kfree(e); +} + +static void dlm_msg_release(struct kref *kref) +{ + struct dlm_msg *msg = container_of(kref, struct dlm_msg, ref); + + kref_put(&msg->entry->ref, dlm_page_release); + kfree(msg); +} + +static void free_entry(struct writequeue_entry *e) +{ + struct dlm_msg *msg, *tmp; + + list_for_each_entry_safe(msg, tmp, &e->msgs, list) { + list_del(&msg->list); + kref_put(&msg->ref, dlm_msg_release); + } + + list_del(&e->list); + kref_put(&e->ref, dlm_page_release); +} + static void dlm_close_sock(struct socket **sock) { if (*sock) { @@ -685,6 +716,7 @@ static void close_connection(struct connection *con, bool and_other, bool tx, bool rx) { 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); @@ -703,6 +735,26 @@ static void close_connection(struct connection *con, bool and_other, close_connection(con->othercon, false, true, true); } + /* 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. + * + * we can always drop messages because retransmits, but what we + * cannot allow is to transmit half messages which may be processed + * at the other side. + * + * 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_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_bh(&con->writequeue_lock); + con->rx_leftover = 0; con->retries = 0; clear_bit(CF_CONNECTED, &con->flags); @@ -978,36 +1030,6 @@ static int accept_from_sock(struct listen_connection *con) return result; } -static void dlm_page_release(struct kref *kref) -{ - struct writequeue_entry *e = container_of(kref, struct writequeue_entry, - ref); - - __free_page(e->page); - kfree(e); -} - -static void dlm_msg_release(struct kref *kref) -{ - struct dlm_msg *msg = container_of(kref, struct dlm_msg, ref); - - kref_put(&msg->entry->ref, dlm_page_release); - kfree(msg); -} - -static void free_entry(struct writequeue_entry *e) -{ - struct dlm_msg *msg, *tmp; - - list_for_each_entry_safe(msg, tmp, &e->msgs, list) { - list_del(&msg->list); - kref_put(&msg->ref, dlm_msg_release); - } - - list_del(&e->list); - kref_put(&e->ref, dlm_page_release); -} - /* * writequeue_entry_complete - try to delete and free write queue entry * @e: write queue entry to try to delete @@ -1019,6 +1041,8 @@ static void writequeue_entry_complete(struct writequeue_entry *e, int completed) { e->offset += completed; e->len -= completed; + /* signal that page was half way transmitted */ + e->dirty = true; if (e->len == 0 && e->users == 0) free_entry(e); -- 2.26.3