Ocfs2 devel - Mar 2010 - [PATCH] ocfs2: be within spinlock when checking the owner of a lockres

The checking of lockres owner in dlm_update_lvb() is not with spinlock
protection. I don't see problem in current call path of dlm_update_lvb().
But just for code robustness.

Signed-off-by: Wengang Wang <wen.gang.wang at oracle.com>
---
 fs/ocfs2/dlm/dlmast.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/fs/ocfs2/dlm/dlmast.c b/fs/ocfs2/dlm/dlmast.c
index dccc439..b7a25ef 100644
--- a/fs/ocfs2/dlm/dlmast.c
+++ b/fs/ocfs2/dlm/dlmast.c
@@ -185,9 +185,8 @@ static void dlm_update_lvb(struct dlm_ctxt *dlm, struct
dlm_lock_resource *res,
 	BUG_ON(!lksb);
 
 	/* only updates if this node masters the lockres */
+	spin_lock(&res->spinlock);
 	if (res->owner == dlm->node_num) {
-
-		spin_lock(&res->spinlock);
 		/* check the lksb flags for the direction */
 		if (lksb->flags & DLM_LKSB_GET_LVB) {
 			mlog(0, "getting lvb from lockres for %s node\n",
@@ -202,8 +201,8 @@ static void dlm_update_lvb(struct dlm_ctxt *dlm, struct
dlm_lock_resource *res,
  		 * here. In the future we might want to clear it at the time
  		 * the put is actually done.
 		 */
-		spin_unlock(&res->spinlock);
 	}
+	spin_unlock(&res->spinlock);
 
 	/* reset any lvb flags on the lksb */
 	lksb->flags &= ~(DLM_LKSB_PUT_LVB|DLM_LKSB_GET_LVB);
-- 
1.6.6.1

Signed-off-by: Sunil Mushran <sunil.mushran at oracle.com>


Wengang Wang wrote:
> The checking of lockres owner in dlm_update_lvb() is not with spinlock
> protection. I don't see problem in current call path of
dlm_update_lvb().
> But just for code robustness.
>
> Signed-off-by: Wengang Wang <wen.gang.wang at oracle.com>
> ---
>  fs/ocfs2/dlm/dlmast.c |    5 ++---
>  1 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/fs/ocfs2/dlm/dlmast.c b/fs/ocfs2/dlm/dlmast.c
> index dccc439..b7a25ef 100644
> --- a/fs/ocfs2/dlm/dlmast.c
> +++ b/fs/ocfs2/dlm/dlmast.c
> @@ -185,9 +185,8 @@ static void dlm_update_lvb(struct dlm_ctxt *dlm, struct
dlm_lock_resource *res,
>  	BUG_ON(!lksb);
>  
>  	/* only updates if this node masters the lockres */
> +	spin_lock(&res->spinlock);
>  	if (res->owner == dlm->node_num) {
> -
> -		spin_lock(&res->spinlock);
>  		/* check the lksb flags for the direction */
>  		if (lksb->flags & DLM_LKSB_GET_LVB) {
>  			mlog(0, "getting lvb from lockres for %s node\n",
> @@ -202,8 +201,8 @@ static void dlm_update_lvb(struct dlm_ctxt *dlm, struct
dlm_lock_resource *res,
>   		 * here. In the future we might want to clear it at the time
>   		 * the put is actually done.
>  		 */
> -		spin_unlock(&res->spinlock);
>  	}
> +	spin_unlock(&res->spinlock);
>  
>  	/* reset any lvb flags on the lksb */
>  	lksb->flags &= ~(DLM_LKSB_PUT_LVB|DLM_LKSB_GET_LVB);
>

On Wed, Mar 24, 2010 at 10:40:44PM +0800, Wengang Wang
wrote:
> The checking of lockres owner in dlm_update_lvb() is not with spinlock
> protection. I don't see problem in current call path of
dlm_update_lvb().
> But just for code robustness.
> 
> Signed-off-by: Wengang Wang <wen.gang.wang at oracle.com>
	This patch is now in the fixes branch of ocfs2.git.

Joel

-- 

"You must remember this:
 A kiss is just a kiss,
 A sigh is just a sigh.
 The fundamental rules apply
 As time goes by."

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127