From d47b822974b8d4da6f22be5341afd4ce6bca6a9f Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Fri, 2 Aug 2024 13:26:41 -0400 Subject: [PATCH] dlm: warn about invalid nodeid comparsions This patch adds a warn on if is_master() and dlm_is_removed() checks on invalid nodeid states that are probably not what the caller wants to do here. The is_master() function checking on r->res_nodeid is invalid when it is set to -1, whereas the dlm_is_removed() has a different meaning as "nodeid member" and also 0 is invalid. We run into these cases and this patch changes those cases as we never will run into them. There should be no functional changes as the condition should return the same result. However this patch signals now on caller level that there might be an "extra" case to handle here. Signed-off-by: Alexander Aring Signed-off-by: David Teigland --- fs/dlm/lock.c | 6 +++--- fs/dlm/lock.h | 2 ++ fs/dlm/member.c | 2 ++ fs/dlm/recover.c | 9 +++++---- 4 files changed, 12 insertions(+), 7 deletions(-) diff --git a/fs/dlm/lock.c b/fs/dlm/lock.c index 720715ddaf48..30aec123a483 100644 --- a/fs/dlm/lock.c +++ b/fs/dlm/lock.c @@ -1151,7 +1151,7 @@ static void __dlm_master_lookup(struct dlm_ls *ls, struct dlm_rsb *r, int our_no r->res_dir_nodeid = our_nodeid; } - if (fix_master && dlm_is_removed(ls, r->res_master_nodeid)) { + if (fix_master && r->res_master_nodeid && dlm_is_removed(ls, r->res_master_nodeid)) { /* Recovery uses this function to set a new master when * the previous master failed. Setting NEW_MASTER will * force dlm_recover_masters to call recover_master on this @@ -5283,7 +5283,7 @@ int dlm_recover_waiters_post(struct dlm_ls *ls) case DLM_MSG_LOOKUP: case DLM_MSG_REQUEST: _request_lock(r, lkb); - if (is_master(r)) + if (r->res_nodeid != -1 && is_master(r)) confirm_master(r, 0); break; case DLM_MSG_CONVERT: @@ -5396,7 +5396,7 @@ void dlm_recover_purge(struct dlm_ls *ls, const struct list_head *root_list) list_for_each_entry(r, root_list, res_root_list) { lock_rsb(r); - if (is_master(r)) { + if (r->res_nodeid != -1 && is_master(r)) { purge_dead_list(ls, r, &r->res_grantqueue, nodeid_gone, &lkb_count); purge_dead_list(ls, r, &r->res_convertqueue, diff --git a/fs/dlm/lock.h b/fs/dlm/lock.h index 4ed8d36f9c6d..b23d7b854ed4 100644 --- a/fs/dlm/lock.h +++ b/fs/dlm/lock.h @@ -66,6 +66,8 @@ int dlm_debug_add_lkb_to_waiters(struct dlm_ls *ls, uint32_t lkb_id, static inline int is_master(struct dlm_rsb *r) { + WARN_ON_ONCE(r->res_nodeid == -1); + return !r->res_nodeid; } diff --git a/fs/dlm/member.c b/fs/dlm/member.c index a7ee7fd2b9d3..c9661906568a 100644 --- a/fs/dlm/member.c +++ b/fs/dlm/member.c @@ -366,6 +366,8 @@ int dlm_is_member(struct dlm_ls *ls, int nodeid) int dlm_is_removed(struct dlm_ls *ls, int nodeid) { + WARN_ON_ONCE(!nodeid || nodeid == -1); + if (find_memb(&ls->ls_nodes_gone, nodeid)) return 1; return 0; diff --git a/fs/dlm/recover.c b/fs/dlm/recover.c index c7afb428a2b4..2e1169c81c6e 100644 --- a/fs/dlm/recover.c +++ b/fs/dlm/recover.c @@ -452,10 +452,11 @@ static int recover_master(struct dlm_rsb *r, unsigned int *count, uint64_t seq) int is_removed = 0; int error; - if (is_master(r)) + if (r->res_nodeid != -1 && is_master(r)) return 0; - is_removed = dlm_is_removed(ls, r->res_nodeid); + if (r->res_nodeid != -1) + is_removed = dlm_is_removed(ls, r->res_nodeid); if (!is_removed && !rsb_flag(r, RSB_NEW_MASTER)) return 0; @@ -664,7 +665,7 @@ int dlm_recover_locks(struct dlm_ls *ls, uint64_t seq, int error, count = 0; list_for_each_entry(r, root_list, res_root_list) { - if (is_master(r)) { + if (r->res_nodeid != -1 && is_master(r)) { rsb_clear_flag(r, RSB_NEW_MASTER); continue; } @@ -858,7 +859,7 @@ void dlm_recover_rsbs(struct dlm_ls *ls, const struct list_head *root_list) list_for_each_entry(r, root_list, res_root_list) { lock_rsb(r); - if (is_master(r)) { + if (r->res_nodeid != -1 && is_master(r)) { if (rsb_flag(r, RSB_RECOVER_CONVERT)) recover_conversion(r);