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 <aahringo@redhat.com>
Signed-off-by: David Teigland <teigland@redhat.com>

diff --git a/fs/dlm/lock.c b/fs/dlm/lock.c
index 720715ddaf486ceaa5519796cb33267b1b8e9f4f..30aec123a483317b3ca42b57037352840d7763c8 100644 (file)
--- 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 4ed8d36f9c6d8336047427d9e0eecae600e0bd89..b23d7b854ed4680010c8ff1a9874d064438fdaf1 100644 (file)
--- 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 a7ee7fd2b9d3e5c4c6852b54f4b190b7a172bee9..c9661906568a151f2af44a43246ff8fd18da20c8 100644 (file)
--- 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 c7afb428a2b4d8fe2d1cb016eed72ad38a82faf2..2e1169c81c6e70e6f968f6087809b6e8794faaa7 100644 (file)
--- 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);