On 2017/12/21 14:36, alex chen wrote:
> Hi Joseph,
>
> On 2017/12/21 9:30, Joseph Qi wrote:
>> Hi Alex,
>>
>> On 17/12/21 08:55, alex chen wrote:
>>> In dlm_reset_mleres_owner(), we will lock
>>> dlm_lock_resource->spinlock after locking dlm_ctxt->master_lock,
>>> which breaks the spinlock lock ordering:
>>> dlm_domain_lock
>>> struct dlm_ctxt->spinlock
>>> struct dlm_lock_resource->spinlock
>>> struct dlm_ctxt->master_lock
>>>
>>> Fix it by unlocking dlm_ctxt->master_lock before locking
>>> dlm_lock_resource->spinlock and restarting to clean master list.
>>>
>>> Signed-off-by: Alex Chen <alex.c...@huawei.com>
>>> Reviewed-by: Jun Piao <piao...@huawei.com>
>>> ---
>>> fs/ocfs2/dlm/dlmmaster.c | 14 ++++++++++----
>>> 1 file changed, 10 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/fs/ocfs2/dlm/dlmmaster.c b/fs/ocfs2/dlm/dlmmaster.c
>>> index 3e04279..d83ccdc 100644
>>> --- a/fs/ocfs2/dlm/dlmmaster.c
>>> +++ b/fs/ocfs2/dlm/dlmmaster.c
>>> @@ -3287,16 +3287,22 @@ static struct dlm_lock_resource
>>> *dlm_reset_mleres_owner(struct dlm_ctxt *dlm,
>>> {
>>> struct dlm_lock_resource *res;
>>>
>>> + assert_spin_locked(&dlm->spinlock);
>>> + assert_spin_locked(&dlm->master_lock);
>>> +
>>> /* Find the lockres associated to the mle and set its owner to UNK */
>>> - res = __dlm_lookup_lockres(dlm, mle->mname, mle->mnamelen,
>>> + res = __dlm_lookup_lockres_full(dlm, mle->mname, mle->mnamelen,
>>> mle->mnamehash);
>>> if (res) {
>>> spin_unlock(&dlm->master_lock);
>>>
>>> - /* move lockres onto recovery list */
>>> spin_lock(&res->spinlock);
>>> - dlm_set_lockres_owner(dlm, res, DLM_LOCK_RES_OWNER_UNKNOWN);
>>> - dlm_move_lockres_to_recovery_list(dlm, res);
>>> + if (!(res->state & DLM_LOCK_RES_DROPPING_REF)) {
>>> + /* move lockres onto recovery list */
>>> + dlm_set_lockres_owner(dlm, res,
>>> DLM_LOCK_RES_OWNER_UNKNOWN);
>>> + dlm_move_lockres_to_recovery_list(dlm, res);
>>> + }
>>> +
>> I don't think this change is lock re-ordering *only*. It definitely
>> changes the logic of resetting mle resource owner.
>> Why do you detach mle from heartbeat if lock resource is in the process
>> of dropping its mastery reference? And why we have to restart in this
>> case?
> I think if the lock resource is being purge we don't need to set its owner to
> UNKNOWN and
> it is the same as the original logic. We should drop the master lock if we
> want to judge
> if the state of the lock resource is DLM_LOCK_RES_DROPPING_REF. Once we drop
> the master lock
> we should restart to clean master list.
> Here the mle is not useful and will be released, so we detach it from
> heartbeat.
> In fact, the mle has been detached in dlm_clean_migration_mle().
Hi Alex,
Perhaps, you can just judge if lock resource is marked
DLM_LOCK_RES_DROPPING_REF and if so directly
return NULL with ::master_lock locked :)
Thanks,
Changwei
>
> Thanks,
> Alex
>>
>> Thanks,
>> Joseph
>>
>>> spin_unlock(&res->spinlock);
>>> dlm_lockres_put(res);
>>>
>>
>> .
>>
>
>
> _______________________________________________
> Ocfs2-devel mailing list
> Ocfs2-devel@oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/ocfs2-devel
>
_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel
