[Cluster-devel] [PATCH dlm/next] fs: dlm: add dlm_is_member() check if fenced

[Cluster-devel] [PATCH dlm/next] fs: dlm: add dlm_is_member() check if fenced

[Alexander Aring]

This patch adds an additional check if the "fenced" node is part of any
lockspace. A node should not be part of any lockspace when it's got
fenced. This is an additional requirement besides that all lockspace
activity should be stopped before calling dlm_midcomms_close().

Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
 fs/dlm/lockspace.c | 5 ++++-
 fs/dlm/lockspace.h | 2 +-
 fs/dlm/midcomms.c  | 2 +-
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/fs/dlm/lockspace.c b/fs/dlm/lockspace.c
index 0c3613d09c5e..a4f492189a08 100644
--- a/fs/dlm/lockspace.c
+++ b/fs/dlm/lockspace.c
@@ -923,12 +923,15 @@ void dlm_stop_lockspaces(void)
 		log_print("dlm user daemon left %d lockspaces", count);
 }
 
-void dlm_stop_lockspaces_check(void)
+void dlm_stop_lockspaces_check(int nodeid)
 {
 	struct dlm_ls *ls;
 
 	spin_lock(&lslist_lock);
 	list_for_each_entry(ls, &lslist, ls_list) {
+		if (WARN_ON(dlm_is_member(ls, nodeid)))
+			break;
+
 		if (WARN_ON(!rwsem_is_locked(&ls->ls_in_recovery) ||
 			    !dlm_locking_stopped(ls)))
 			break;
diff --git a/fs/dlm/lockspace.h b/fs/dlm/lockspace.h
index 306fc4f4ea15..9d1ea1013650 100644
--- a/fs/dlm/lockspace.h
+++ b/fs/dlm/lockspace.h
@@ -19,7 +19,7 @@ struct dlm_ls *dlm_find_lockspace_local(void *id);
 struct dlm_ls *dlm_find_lockspace_device(int minor);
 void dlm_put_lockspace(struct dlm_ls *ls);
 void dlm_stop_lockspaces(void);
-void dlm_stop_lockspaces_check(void);
+void dlm_stop_lockspaces_check(int nodeid);
 
 #endif				/* __LOCKSPACE_DOT_H__ */
 
diff --git a/fs/dlm/midcomms.c b/fs/dlm/midcomms.c
index 6489bc22ad61..22037183f565 100644
--- a/fs/dlm/midcomms.c
+++ b/fs/dlm/midcomms.c
@@ -1413,7 +1413,7 @@ int dlm_midcomms_close(int nodeid)
 	if (nodeid == dlm_our_nodeid())
 		return 0;
 
-	dlm_stop_lockspaces_check();
+	dlm_stop_lockspaces_check(nodeid);
 
 	idx = srcu_read_lock(&nodes_srcu);
 	/* Abort pending close/remove operation */
-- 
2.31.1

[Cluster-devel] [PATCH dlm/next] fs: dlm: add dlm_is_member() check if fenced

[Alexander Aring]

Hi,

On Wed, Jun 15, 2022 at 10:15 AM Alexander Aring <aahringo@redhat.com> wrote:
>
> This patch adds an additional check if the "fenced" node is part of any
> lockspace. A node should not be part of any lockspace when it's got
> fenced. This is an additional requirement besides that all lockspace
> activity should be stopped before calling dlm_midcomms_close().
>
> Signed-off-by: Alexander Aring <aahringo@redhat.com>
> ---
>  fs/dlm/lockspace.c | 5 ++++-
>  fs/dlm/lockspace.h | 2 +-
>  fs/dlm/midcomms.c  | 2 +-
>  3 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/fs/dlm/lockspace.c b/fs/dlm/lockspace.c
> index 0c3613d09c5e..a4f492189a08 100644
> --- a/fs/dlm/lockspace.c
> +++ b/fs/dlm/lockspace.c
> @@ -923,12 +923,15 @@ void dlm_stop_lockspaces(void)
>                 log_print("dlm user daemon left %d lockspaces", count);
>  }
>
> -void dlm_stop_lockspaces_check(void)
> +void dlm_stop_lockspaces_check(int nodeid)
>  {
>         struct dlm_ls *ls;
>
>         spin_lock(&lslist_lock);
>         list_for_each_entry(ls, &lslist, ls_list) {
> +               if (WARN_ON(dlm_is_member(ls, nodeid)))
> +                       break;
> +

I will drop this patch. I currently run into this warning. We can
assume that there are no dlm messages processed that we transmit any
new reply message. Also no new message can be triggered by e.g.
dlm_lock(), because the previously checked if recovery lock is held
and the lockspace running check.

There exist two kinds of "memberships" one is nodeid (lockspace level)
and one is node IP (socket level). This one will cut socket level and
there is a MUST requirement which is probably done afterwards to
remove the node from all lockspaces. I probably would like to see
first the lockspace member being removed THEN the socket connection
gets closed...  however it's probably the other way around and maybe
due the fact of corosync chronological order callback handling (e.g.
lost quorum vs resource membership), I will look again into that in
another time to see how things can maybe be changed... (if possible)

Maybe midcomms_close can also remove the lockspace member and if there
is a "nodeid from resource membership removal" afterwards, we simply
don't care if it's already removed...

- Alex