[Ocfs2-devel] [PATCH 1/2] ocfs2: dlmfs: not clear USER_LOCK_ATTACHED when destroy lock

[Junxiao Bi]

On 5/17/22 6:54 PM, Joseph Qi wrote:
>
> On 5/18/22 12:12 AM, Junxiao Bi wrote:
>> On 5/16/22 6:58 PM, Joseph Qi wrote:
>>> On 5/17/22 12:30 AM, Junxiao Bi wrote:
>>>> On 5/15/22 7:57 AM, Joseph Qi wrote:
>>>>> On 5/14/22 12:27 AM, Junxiao Bi wrote:
>>>>>> 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?
>>>>>>
>>>>> lockres may be reused and if we don't reinitialized, the left flag can
>>>>> cause unexpected behavior.
>>>> I don't know how it could get reused since it's going to be removed. Anyway USER_LOCK_IN_TEARDOWN is still set in lockres. All the flow will bail out because of this flag.
>>>>
>>> dlmfs_inode_private is allocated from kmem_cache.
>>> The case I'm thinking about is, calling user_dlm_destroy_lock() without
>>> a valid ast comming before. So checking USER_LOCK_ATTACHED here may be
>>> incorrect.
>>> But look more closer, it seems that lockres is unused for directories.
>>> So it won't be a real issue.
>> Yes, lock is only for file, not direcotry.
>>> Could you please send a new version with update description?
>> Sorry, little confused, which part of description needs update?
>>
> The typo that user_ast() is for granting lock request. And better to include
> the information we discussed above.

got it, will send a v2

Thanks,

Junxiao.