[LU-1432] Race condition between lprocfs_exp_setup() and lprocfs_free_per_client_stats() causes LBUG

• Type: Bug
• Resolution: Fixed
• Priority: Minor
• Fix Version/s: Lustre 2.3.0, Lustre 2.1.3
• Environment: FEFS based on Lustre 1.8.5, RIKEN K-computer
• Severity: 3
• Assignee: Lai Siyao
• Reporter: Hiroya Nozaki (Inactive)

Description

We've often happened to see the below LBUG until now, nevertheless we already applied the putch of LU-39 to FEFS.

        LASSERTF(atomic_read(&client_stat->nid_exp_ref_count) == 0,
                 "nid %s:count %d\n", libcfs_nid2str(client_stat->nid),
                 atomic_read(&client_stat->nid_exp_ref_count));

Then, today, I finally found out the race condition between lprocfs_exp_setup() and lprocfs_free_per_client_stats(). And as far as I can see, it appears that the LBUG could be the case in Lustre-1.8 and Lustre-2.x. So I decided to report it here.

the account of the case is below.

• umount command is executed and set obd_stopping 1 in class_cleanup()

• but some ll_ost_xxx threads which have already passed the below if-statement in target_handle_connect() can go ahead anyway.

int target_handle_connect(struct ptlrpc_request *req, svc_handler_t handler)
{

        ...
        if (!target || target->obd_stopping || !target->obd_set_up) {
                LCONSOLE_ERROR_MSG(0x137, "UUID '%s' is not available "
                                   " for connect (%s)\n", str,
                                   !target ? "no target" :
                                   (target->obd_stopping ? "stopping" :
                                   "not set up"));
                GOTO(out, rc = -ENODEV);
        }
        ...

}

• the threads will reach out the below codes

int lprocfs_exp_setup(struct obd_export *exp, lnet_nid_t *nid, int *newnid)
{
        ...
        old_stat = lustre_hash_findadd_unique(obd->obd_nid_stats_hash,
                                              nid, &new_stat->nid_hash);
        CDEBUG(D_INFO, "Found stats %p for nid %s - ref %d\n",
               old_stat, libcfs_nid2str(*nid),
               atomic_read(&new_stat->nid_exp_ref_count));

        /* we need to release old stats because lprocfs_exp_cleanup() hasn't
         * been and will never be called. */
        if (exp->exp_nid_stats) {
                nidstat_putref(exp->exp_nid_stats);
                exp->exp_nid_stats = NULL;
        }
        ...
}

• if exp->exp_nid_stats is not NULL, lustre_hash_findadd_unique() increments the ref count of old_stat and returns it. So, it means that the old_stat is temporalily one bigger, until nidstat_putref() is called.

• Then, at the same timing, if the umount is running in lprocfs_free_client_stats() and is handling the same stats as the above old_stat. it causes the LBUG beause the nid_stat ref count is one bigger than expected.

You can cause the same problem when you add some waiting code into the part of lprocfs_exp_setup() and execute umount command. in this way, I've actually caused the problem in purpose.

and now, I'm creating the patch to the issue and I'll upload that soon but it's for 1.8.8.
I'm really sorry that I don't have the machine for Lustre-2.x X(

Issue Links

• Trackbacks: Lustre 1.8.x known issues tracker While testing against Lustre b18 branch, we would hit known bugs which were already reported in Lustre Bugzilla https://bugzilla.lustre.org/. In order to move away from relying on Bugzilla, we would create a JIRA
• Trackbacks: Changelog 2.1 Changes from version 2.1.2 to version 2.1.3 Server support for kernels: 2.6.18308.13.1.el5 (RHEL5) 2.6.32279.2.1.el6 (RHEL6) Client support for unpatched kernels: 2.6.18308.13.1.el5 (RHEL5) 2.6.32279.2.1....

Comments

[Jay Lan (Inactive)]

In the "ISSUE Links" section of this ticket, LU-1592 is said to be a duplicate
of this problem. I do not think that is correct. Can we remove the link?

[Peter Jones]

Landed for 2.1.3 and 2.3

[Jay Lan (Inactive)]

Can you offically land the above patch? We have been running that patch in our production systems for ~2 weeks. I would like to see it landed. BTW, a port to b2_1 branch would be appreciated.

[Lai Siyao]

Patch for master is at: http://review.whamcloud.com/#change,3244

[Hiroya Nozaki (Inactive)]

ah, the above comment was my mistake. I forgot to add one put. I'm really sorry.

[Lai Siyao]

Could you upload full log? I need to know where the export refcount is taken.