[LU-11113] bad destroyed lock logic in __ldlm_handle2lock()

• Status: Open
• Project: Lustre
• Type: Bug
• Priority: Minor
• Reporter: John Hammond
• Assignee: WC Triage
• Resolution: Unresolved
• Severity: 3

Description

In __ldlm_handle2lock() the logic around the first check of ldlm_is_destroyed() seems to be reversed after commit d8adb505 (b24336 ldlm_resource::lr_lvb_data is protected by wrong lock). We check ldlm_is_destroyed() before locking the lock and possibly again after:

struct ldlm_lock *__ldlm_handle2lock(const struct lustre_handle *handle,
                                     __u64 flags)
{
        struct ldlm_lock *lock;
        ENTRY;

        LASSERT(handle);

        lock = class_handle2object(handle->cookie, NULL);
        if (lock == NULL)
		RETURN(NULL);

        if (lock->l_export != NULL && lock->l_export->exp_failed) {
                CDEBUG(D_INFO, "lock export failed: lock %p, exp %p\n",
	               lock, lock->l_export);
	        LDLM_LOCK_PUT(lock);
                RETURN(NULL);
	}

        /* It's unlikely but possible that someone marked the lock as                                                            
         * destroyed after we did handle2object on it */
        if ((flags == 0) && !ldlm_is_destroyed(lock)) {
                lu_ref_add(&lock->l_reference, "handle", current);
                RETURN(lock);
        }

        lock_res_and_lock(lock);

	LASSERT(lock->l_resource != NULL);

        lu_ref_add_atomic(&lock->l_reference, "handle", current);
	if (unlikely(ldlm_is_destroyed(lock))) {
                ...

But the logic seems to be the opposite of what we would want. It should be:

1. If the lock is destroyed then return NULL.
2. Lock the lock.
3. If the lock is destroyed then return NULL.
4. ...