[LU-6516] double spin unlock in ofd_inconsistency_verification_main

• Type: Bug
• Resolution: Fixed
• Priority: Minor
• Fix Version/s: Lustre 2.8.0
• Labels: patch
• Severity: 3
• Assignee: Jian Yu
• Reporter: Oleg Drokin

Description

found by smatch, in ofd_inconsistency_verification_main:

        spin_unlock(&ofd->ofd_inconsistency_lock);
        if (rc != 0)
                RETURN(rc);

        OBD_ALLOC_PTR(lr);
        if (unlikely(lr == NULL))
                GOTO(out, rc = -ENOMEM);
...
        thread_set_flags(thread, SVC_STOPPED);
        wake_up_all(&thread->t_ctl_waitq);
        spin_unlock(&ofd->ofd_inconsistency_lock);

Comments

[Peter Jones]

Landed for 2.8

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch http://review.whamcloud.com/14875/
Subject: LU-6516 ofd: fix double spin_unlock
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 490ea83a486c0c44274f4d4f6f77d8e0e16835d5

[Gerrit Updater]

Ulka Vaze ([e-mail]) uploaded a new patch: http://review.whamcloud.com/14875
Subject: LU-6516 ofd: double unlock of spinlock
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 1095e21567f7cfb5906d9e7178e950567fb924a4

[Ulka Vaze (Inactive)]

Hi,

Yes this seems to be issue. Earlier we are taking lock and waking up threads.
See this code -

spin_lock(&ofd->ofd_inconsistency_lock);
    thread_set_flags(thread, rc != 0 ? SVC_STOPPED : SVC_RUNNING);
    wake_up_all(&thread->t_ctl_waitq);
    spin_unlock(&ofd->ofd_inconsistency_lock);
 

However in out we did not lock again.

thread_set_flags(thread, SVC_STOPPED);
    wake_up_all(&thread->t_ctl_waitq);
    spin_unlock(&ofd->ofd_inconsistency_lock);
    lu_env_fini(&env);

    return rc;

This is because in normal case we have spinlock already taken.
But if memory allocation failed then this will not work.
Probable solution is -
OBD_ALLOC_PTR(lr);
if (unlikely(lr == NULL))
GOTO(out1, rc = ENOMEM);<--- added new lable

out1:
spin_lock(&ofd->ofd_inconsistency_lock);
thread_set_flags(thread, rc != 0 ? SVC_STOPPED : SVC_RUNNING);
wake_up_all(&thread->t_ctl_waitq);
spin_unlock(&ofd->ofd_inconsistency_lock);
lu_env_fini(&env);

return rc;

Please advice.
-Ulka