Re: [Ocfs2-devel] [PATCH 2/2] ocfs2: o2dlm fix race in purge lockres and newlock (orabug 9094491)
On 6/17/2010 7:11 PM, Sunil Mushran wrote: This patch looks ok on the surface. Should be usable. But checking into the repo is another matter. ok, won't checkin but will give this as one-off fix for now. My problem is that this flag is very much like inflight_locks but is not the same. inflight_locks are taken only on lockres' that are mastered by the node. This new flag does the same but for non mastered locks too. A better solution will be to merge the two. And that means cleaning up inflight_locks. will look into this. The reason for the NAK for check in is that the code is adding more messiness to an area that is already very messy. Sunil On 06/15/2010 09:43 PM, Srinivas Eeda wrote: This patch fixes the following hole. dlmlock tries to create a new lock on a lockres that is on purge list. It calls dlm_get_lockresource and later adds a lock to blocked list. But in this window, dlm_thread can purge the lockres and unhash it. This will cause a BUG, as when the AST comes back from the master lockres is not found This patch marks the lockres with a new state DLM_LOCK_RES_IN_USE which would protect lockres from dlm_thread purging it. Signed-off-by: Srinivas Eedasrinivas.e...@oracle.com --- fs/ocfs2/dlm/dlmcommon.h |1 + fs/ocfs2/dlm/dlmlock.c |4 fs/ocfs2/dlm/dlmmaster.c |5 - fs/ocfs2/dlm/dlmthread.c |1 + 4 files changed, 10 insertions(+), 1 deletions(-) diff --git a/fs/ocfs2/dlm/dlmcommon.h b/fs/ocfs2/dlm/dlmcommon.h index 0102be3..6cf3c08 100644 --- a/fs/ocfs2/dlm/dlmcommon.h +++ b/fs/ocfs2/dlm/dlmcommon.h @@ -279,6 +279,7 @@ static inline void __dlm_set_joining_node(struct dlm_ctxt *dlm, #define DLM_LOCK_RES_DIRTY0x0008 #define DLM_LOCK_RES_IN_PROGRESS 0x0010 #define DLM_LOCK_RES_MIGRATING0x0020 +#define DLM_LOCK_RES_IN_USE 0x0100 #define DLM_LOCK_RES_DROPPING_REF 0x0040 #define DLM_LOCK_RES_BLOCK_DIRTY 0x1000 #define DLM_LOCK_RES_SETREF_INPROG0x2000 diff --git a/fs/ocfs2/dlm/dlmlock.c b/fs/ocfs2/dlm/dlmlock.c index 777..501ac40 100644 --- a/fs/ocfs2/dlm/dlmlock.c +++ b/fs/ocfs2/dlm/dlmlock.c @@ -134,6 +134,8 @@ static enum dlm_status dlmlock_master(struct dlm_ctxt *dlm, if (status != DLM_NORMAL lock-ml.node != dlm-node_num) { /* erf. state changed after lock was dropped. */ +/* DLM_LOCK_RES_IN_USE is set in dlm_get_lock_resource */ +res-state= ~DLM_LOCK_RES_IN_USE; spin_unlock(res-spinlock); dlm_error(status); return status; @@ -180,6 +182,7 @@ static enum dlm_status dlmlock_master(struct dlm_ctxt *dlm, kick_thread = 1; } } +res-state= ~DLM_LOCK_RES_IN_USE; /* reduce the inflight count, this may result in the lockres * being purged below during calc_usage */ if (lock-ml.node == dlm-node_num) @@ -246,6 +249,7 @@ static enum dlm_status dlmlock_remote(struct dlm_ctxt *dlm, spin_lock(res-spinlock); res-state= ~DLM_LOCK_RES_IN_PROGRESS; +res-state= ~DLM_LOCK_RES_IN_USE; lock-lock_pending = 0; if (status != DLM_NORMAL) { if (status == DLM_RECOVERING diff --git a/fs/ocfs2/dlm/dlmmaster.c b/fs/ocfs2/dlm/dlmmaster.c index 9289b43..f0f2d97 100644 --- a/fs/ocfs2/dlm/dlmmaster.c +++ b/fs/ocfs2/dlm/dlmmaster.c @@ -719,6 +719,7 @@ lookup: if (tmpres) { int dropping_ref = 0; +tmpres-state |= DLM_LOCK_RES_IN_USE; spin_unlock(dlm-spinlock); spin_lock(tmpres-spinlock); @@ -731,8 +732,10 @@ lookup: if (tmpres-owner == dlm-node_num) { BUG_ON(tmpres-state DLM_LOCK_RES_DROPPING_REF); dlm_lockres_grab_inflight_ref(dlm, tmpres); -} else if (tmpres-state DLM_LOCK_RES_DROPPING_REF) +} else if (tmpres-state DLM_LOCK_RES_DROPPING_REF) { +tmpres-state= ~DLM_LOCK_RES_IN_USE; dropping_ref = 1; +} spin_unlock(tmpres-spinlock); /* wait until done messaging the master, drop our ref to allow diff --git a/fs/ocfs2/dlm/dlmthread.c b/fs/ocfs2/dlm/dlmthread.c index fb0be6c..2b82e0b 100644 --- a/fs/ocfs2/dlm/dlmthread.c +++ b/fs/ocfs2/dlm/dlmthread.c @@ -93,6 +93,7 @@ int __dlm_lockres_has_locks(struct dlm_lock_resource *res) int __dlm_lockres_unused(struct dlm_lock_resource *res) { if (!__dlm_lockres_has_locks(res) +!(res-state DLM_LOCK_RES_IN_USE) (list_empty(res-dirty) !(res-state DLM_LOCK_RES_DIRTY))) { /* try not to scan the bitmap unless the first two * conditions are already true */ ___ Ocfs2-devel mailing list Ocfs2-devel@oss.oracle.com http://oss.oracle.com/mailman/listinfo/ocfs2-devel
Re: [Ocfs2-devel] [PATCH 2/2] ocfs2: o2dlm fix race in purge lockres and newlock (orabug 9094491)
This patch looks ok on the surface. Should be usable. But checking into the repo is another matter. My problem is that this flag is very much like inflight_locks but is not the same. inflight_locks are taken only on lockres' that are mastered by the node. This new flag does the same but for non mastered locks too. A better solution will be to merge the two. And that means cleaning up inflight_locks. The reason for the NAK for check in is that the code is adding more messiness to an area that is already very messy. Sunil On 06/15/2010 09:43 PM, Srinivas Eeda wrote: This patch fixes the following hole. dlmlock tries to create a new lock on a lockres that is on purge list. It calls dlm_get_lockresource and later adds a lock to blocked list. But in this window, dlm_thread can purge the lockres and unhash it. This will cause a BUG, as when the AST comes back from the master lockres is not found This patch marks the lockres with a new state DLM_LOCK_RES_IN_USE which would protect lockres from dlm_thread purging it. Signed-off-by: Srinivas Eedasrinivas.e...@oracle.com --- fs/ocfs2/dlm/dlmcommon.h |1 + fs/ocfs2/dlm/dlmlock.c |4 fs/ocfs2/dlm/dlmmaster.c |5 - fs/ocfs2/dlm/dlmthread.c |1 + 4 files changed, 10 insertions(+), 1 deletions(-) diff --git a/fs/ocfs2/dlm/dlmcommon.h b/fs/ocfs2/dlm/dlmcommon.h index 0102be3..6cf3c08 100644 --- a/fs/ocfs2/dlm/dlmcommon.h +++ b/fs/ocfs2/dlm/dlmcommon.h @@ -279,6 +279,7 @@ static inline void __dlm_set_joining_node(struct dlm_ctxt *dlm, #define DLM_LOCK_RES_DIRTY0x0008 #define DLM_LOCK_RES_IN_PROGRESS 0x0010 #define DLM_LOCK_RES_MIGRATING0x0020 +#define DLM_LOCK_RES_IN_USE 0x0100 #define DLM_LOCK_RES_DROPPING_REF 0x0040 #define DLM_LOCK_RES_BLOCK_DIRTY 0x1000 #define DLM_LOCK_RES_SETREF_INPROG0x2000 diff --git a/fs/ocfs2/dlm/dlmlock.c b/fs/ocfs2/dlm/dlmlock.c index 777..501ac40 100644 --- a/fs/ocfs2/dlm/dlmlock.c +++ b/fs/ocfs2/dlm/dlmlock.c @@ -134,6 +134,8 @@ static enum dlm_status dlmlock_master(struct dlm_ctxt *dlm, if (status != DLM_NORMAL lock-ml.node != dlm-node_num) { /* erf. state changed after lock was dropped. */ + /* DLM_LOCK_RES_IN_USE is set in dlm_get_lock_resource */ + res-state= ~DLM_LOCK_RES_IN_USE; spin_unlock(res-spinlock); dlm_error(status); return status; @@ -180,6 +182,7 @@ static enum dlm_status dlmlock_master(struct dlm_ctxt *dlm, kick_thread = 1; } } + res-state= ~DLM_LOCK_RES_IN_USE; /* reduce the inflight count, this may result in the lockres * being purged below during calc_usage */ if (lock-ml.node == dlm-node_num) @@ -246,6 +249,7 @@ static enum dlm_status dlmlock_remote(struct dlm_ctxt *dlm, spin_lock(res-spinlock); res-state= ~DLM_LOCK_RES_IN_PROGRESS; + res-state= ~DLM_LOCK_RES_IN_USE; lock-lock_pending = 0; if (status != DLM_NORMAL) { if (status == DLM_RECOVERING diff --git a/fs/ocfs2/dlm/dlmmaster.c b/fs/ocfs2/dlm/dlmmaster.c index 9289b43..f0f2d97 100644 --- a/fs/ocfs2/dlm/dlmmaster.c +++ b/fs/ocfs2/dlm/dlmmaster.c @@ -719,6 +719,7 @@ lookup: if (tmpres) { int dropping_ref = 0; + tmpres-state |= DLM_LOCK_RES_IN_USE; spin_unlock(dlm-spinlock); spin_lock(tmpres-spinlock); @@ -731,8 +732,10 @@ lookup: if (tmpres-owner == dlm-node_num) { BUG_ON(tmpres-state DLM_LOCK_RES_DROPPING_REF); dlm_lockres_grab_inflight_ref(dlm, tmpres); - } else if (tmpres-state DLM_LOCK_RES_DROPPING_REF) + } else if (tmpres-state DLM_LOCK_RES_DROPPING_REF) { + tmpres-state= ~DLM_LOCK_RES_IN_USE; dropping_ref = 1; + } spin_unlock(tmpres-spinlock); /* wait until done messaging the master, drop our ref to allow diff --git a/fs/ocfs2/dlm/dlmthread.c b/fs/ocfs2/dlm/dlmthread.c index fb0be6c..2b82e0b 100644 --- a/fs/ocfs2/dlm/dlmthread.c +++ b/fs/ocfs2/dlm/dlmthread.c @@ -93,6 +93,7 @@ int __dlm_lockres_has_locks(struct dlm_lock_resource *res) int __dlm_lockres_unused(struct dlm_lock_resource *res) { if (!__dlm_lockres_has_locks(res) + !(res-state DLM_LOCK_RES_IN_USE) (list_empty(res-dirty) !(res-state DLM_LOCK_RES_DIRTY))) { /* try not to scan the bitmap unless the first two * conditions are already true */ ___ Ocfs2-devel mailing list Ocfs2-devel@oss.oracle.com http://oss.oracle.com/mailman/listinfo/ocfs2-devel
[Ocfs2-devel] [PATCH 2/2] ocfs2: o2dlm fix race in purge lockres and newlock (orabug 9094491)
This patch fixes the following hole. dlmlock tries to create a new lock on a lockres that is on purge list. It calls dlm_get_lockresource and later adds a lock to blocked list. But in this window, dlm_thread can purge the lockres and unhash it. This will cause a BUG, as when the AST comes back from the master lockres is not found This patch marks the lockres with a new state DLM_LOCK_RES_IN_USE which would protect lockres from dlm_thread purging it. Signed-off-by: Srinivas Eeda srinivas.e...@oracle.com --- fs/ocfs2/dlm/dlmcommon.h |1 + fs/ocfs2/dlm/dlmlock.c |4 fs/ocfs2/dlm/dlmmaster.c |5 - fs/ocfs2/dlm/dlmthread.c |1 + 4 files changed, 10 insertions(+), 1 deletions(-) diff --git a/fs/ocfs2/dlm/dlmcommon.h b/fs/ocfs2/dlm/dlmcommon.h index 0102be3..6cf3c08 100644 --- a/fs/ocfs2/dlm/dlmcommon.h +++ b/fs/ocfs2/dlm/dlmcommon.h @@ -279,6 +279,7 @@ static inline void __dlm_set_joining_node(struct dlm_ctxt *dlm, #define DLM_LOCK_RES_DIRTY0x0008 #define DLM_LOCK_RES_IN_PROGRESS 0x0010 #define DLM_LOCK_RES_MIGRATING0x0020 +#define DLM_LOCK_RES_IN_USE 0x0100 #define DLM_LOCK_RES_DROPPING_REF 0x0040 #define DLM_LOCK_RES_BLOCK_DIRTY 0x1000 #define DLM_LOCK_RES_SETREF_INPROG0x2000 diff --git a/fs/ocfs2/dlm/dlmlock.c b/fs/ocfs2/dlm/dlmlock.c index 777..501ac40 100644 --- a/fs/ocfs2/dlm/dlmlock.c +++ b/fs/ocfs2/dlm/dlmlock.c @@ -134,6 +134,8 @@ static enum dlm_status dlmlock_master(struct dlm_ctxt *dlm, if (status != DLM_NORMAL lock-ml.node != dlm-node_num) { /* erf. state changed after lock was dropped. */ + /* DLM_LOCK_RES_IN_USE is set in dlm_get_lock_resource */ + res-state = ~DLM_LOCK_RES_IN_USE; spin_unlock(res-spinlock); dlm_error(status); return status; @@ -180,6 +182,7 @@ static enum dlm_status dlmlock_master(struct dlm_ctxt *dlm, kick_thread = 1; } } + res-state = ~DLM_LOCK_RES_IN_USE; /* reduce the inflight count, this may result in the lockres * being purged below during calc_usage */ if (lock-ml.node == dlm-node_num) @@ -246,6 +249,7 @@ static enum dlm_status dlmlock_remote(struct dlm_ctxt *dlm, spin_lock(res-spinlock); res-state = ~DLM_LOCK_RES_IN_PROGRESS; + res-state = ~DLM_LOCK_RES_IN_USE; lock-lock_pending = 0; if (status != DLM_NORMAL) { if (status == DLM_RECOVERING diff --git a/fs/ocfs2/dlm/dlmmaster.c b/fs/ocfs2/dlm/dlmmaster.c index 9289b43..f0f2d97 100644 --- a/fs/ocfs2/dlm/dlmmaster.c +++ b/fs/ocfs2/dlm/dlmmaster.c @@ -719,6 +719,7 @@ lookup: if (tmpres) { int dropping_ref = 0; + tmpres-state |= DLM_LOCK_RES_IN_USE; spin_unlock(dlm-spinlock); spin_lock(tmpres-spinlock); @@ -731,8 +732,10 @@ lookup: if (tmpres-owner == dlm-node_num) { BUG_ON(tmpres-state DLM_LOCK_RES_DROPPING_REF); dlm_lockres_grab_inflight_ref(dlm, tmpres); - } else if (tmpres-state DLM_LOCK_RES_DROPPING_REF) + } else if (tmpres-state DLM_LOCK_RES_DROPPING_REF) { + tmpres-state = ~DLM_LOCK_RES_IN_USE; dropping_ref = 1; + } spin_unlock(tmpres-spinlock); /* wait until done messaging the master, drop our ref to allow diff --git a/fs/ocfs2/dlm/dlmthread.c b/fs/ocfs2/dlm/dlmthread.c index fb0be6c..2b82e0b 100644 --- a/fs/ocfs2/dlm/dlmthread.c +++ b/fs/ocfs2/dlm/dlmthread.c @@ -93,6 +93,7 @@ int __dlm_lockres_has_locks(struct dlm_lock_resource *res) int __dlm_lockres_unused(struct dlm_lock_resource *res) { if (!__dlm_lockres_has_locks(res) + !(res-state DLM_LOCK_RES_IN_USE) (list_empty(res-dirty) !(res-state DLM_LOCK_RES_DIRTY))) { /* try not to scan the bitmap unless the first two * conditions are already true */ -- 1.5.6.5 ___ Ocfs2-devel mailing list Ocfs2-devel@oss.oracle.com http://oss.oracle.com/mailman/listinfo/ocfs2-devel