Joseph Qi
2022-May-13 02:05 UTC
[Ocfs2-devel] [PATCH 1/2] ocfs2: dlmfs: not clear USER_LOCK_ATTACHED when destroy lock
On 5/11/22 7:22 AM, Junxiao Bi wrote:> The following function is the only place that check USER_LOCK_ATTACHED, > this flag is set when lock request is granted through user_bast() and > only the following function will clear it. >user_ast?> Checking of this flag here is to make sure ocfs2_dlm_unlock is not > issued if this lock is never granted. For example, lock file is created > and then get removed, open file never happens. > > Clearing the flag here is not necessary because this is the only function > that checks it, if another flow is executing user_dlm_destroy_lock(), it > will bail out at the beginning because of USER_LOCK_IN_TEARDOWN and never > check USER_LOCK_ATTACHED. > Drop the clear, so we don't need take care it for the following > error handling patch. >Seems it depends on initializing lockres every time, but it seems this is not true for directory now. Thanks, Joseph> int user_dlm_destroy_lock(struct user_lock_res *lockres) > { > ... > > status = 0; > if (!(lockres->l_flags & USER_LOCK_ATTACHED)) { > spin_unlock(&lockres->l_lock); > goto bail; > } > > lockres->l_flags &= ~USER_LOCK_ATTACHED; > lockres->l_flags |= USER_LOCK_BUSY; > spin_unlock(&lockres->l_lock); > > status = ocfs2_dlm_unlock(conn, &lockres->l_lksb, DLM_LKF_VALBLK); > if (status) { > user_log_dlm_error("ocfs2_dlm_unlock", status, lockres); > goto bail; > } > ... > } > > Cc: <stable at vger.kernel.org> > Signed-off-by: Junxiao Bi <junxiao.bi at oracle.com> > --- > fs/ocfs2/dlmfs/userdlm.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/fs/ocfs2/dlmfs/userdlm.c b/fs/ocfs2/dlmfs/userdlm.c > index 29f183a15798..af0be612589c 100644 > --- a/fs/ocfs2/dlmfs/userdlm.c > +++ b/fs/ocfs2/dlmfs/userdlm.c > @@ -619,7 +619,6 @@ int user_dlm_destroy_lock(struct user_lock_res *lockres) > goto bail; > } > > - lockres->l_flags &= ~USER_LOCK_ATTACHED; > lockres->l_flags |= USER_LOCK_BUSY; > spin_unlock(&lockres->l_lock); >
Junxiao Bi
2022-May-13 16:27 UTC
[Ocfs2-devel] [PATCH 1/2] ocfs2: dlmfs: not clear USER_LOCK_ATTACHED when destroy lock
On 5/12/22 7:05 PM, Joseph Qi wrote:> > On 5/11/22 7:22 AM, Junxiao Bi wrote: >> The following function is the only place that check USER_LOCK_ATTACHED, >> this flag is set when lock request is granted through user_bast() and >> only the following function will clear it. >> > user_ast?Good catch, that's a typo, should be user_ast.> >> Checking of this flag here is to make sure ocfs2_dlm_unlock is not >> issued if this lock is never granted. For example, lock file is created >> and then get removed, open file never happens. >> >> Clearing the flag here is not necessary because this is the only function >> that checks it, if another flow is executing user_dlm_destroy_lock(), it >> will bail out at the beginning because of USER_LOCK_IN_TEARDOWN and never >> check USER_LOCK_ATTACHED. >> Drop the clear, so we don't need take care it for the following >> error handling patch. >> > Seems it depends on initializing lockres every time, but it seems this > is not true for directory now.Sorry, i didn't get this. Can you elaborate this? Thanks, Junxiao.> > Thanks, > Joseph > >> int user_dlm_destroy_lock(struct user_lock_res *lockres) >> { >> ... >> >> status = 0; >> if (!(lockres->l_flags & USER_LOCK_ATTACHED)) { >> spin_unlock(&lockres->l_lock); >> goto bail; >> } >> >> lockres->l_flags &= ~USER_LOCK_ATTACHED; >> lockres->l_flags |= USER_LOCK_BUSY; >> spin_unlock(&lockres->l_lock); >> >> status = ocfs2_dlm_unlock(conn, &lockres->l_lksb, DLM_LKF_VALBLK); >> if (status) { >> user_log_dlm_error("ocfs2_dlm_unlock", status, lockres); >> goto bail; >> } >> ... >> } >> >> Cc: <stable at vger.kernel.org> >> Signed-off-by: Junxiao Bi <junxiao.bi at oracle.com> >> --- >> fs/ocfs2/dlmfs/userdlm.c | 1 - >> 1 file changed, 1 deletion(-) >> >> diff --git a/fs/ocfs2/dlmfs/userdlm.c b/fs/ocfs2/dlmfs/userdlm.c >> index 29f183a15798..af0be612589c 100644 >> --- a/fs/ocfs2/dlmfs/userdlm.c >> +++ b/fs/ocfs2/dlmfs/userdlm.c >> @@ -619,7 +619,6 @@ int user_dlm_destroy_lock(struct user_lock_res *lockres) >> goto bail; >> } >> >> - lockres->l_flags &= ~USER_LOCK_ATTACHED; >> lockres->l_flags |= USER_LOCK_BUSY; >> spin_unlock(&lockres->l_lock); >>