Bug #4500

mptsas_hash_traverse() is unsafe, leads to missing devices

Added by Robert Mustacchi almost 4 years ago. Updated almost 4 years ago.

Status:ResolvedStart date:2014-01-17
Priority:NormalDue date:
Assignee:Keith Wesolowski% Done:

100%

Category:kernel
Target version:-
Difficulty:Medium Tags:

Description

The observed behaviour is seen on a Dell C8220X, though there's nothing really specific to this system that causes it. Upon booting, a limited number of disks is visible, even though there are 40 physically attached (4 on one controller, 36 on another). Each time devfsadm -c disk is run, 2 more disks show up.

--

The basic problem here is mptsas_hash_traverse()'s implementation. This hash table, including the single traversal cursor it contains, is protected by the mpt structure's m_mutex. A caller in the midst of traversal who drops this lock has no guarantee that the cursor has not been moved by another thread in the meantime. That's exactly what mptsas_config_all() does:

        ptgt = (mptsas_target_t *)mptsas_hash_traverse(&mpt->m_active->m_tgttbl,
            MPTSAS_HASH_FIRST);
        while (ptgt != NULL) {
                phy_mask = ptgt->m_phymask;
                if (phy_mask == phymask) {
                        mutex_exit(&mpt->m_mutex);
                        (void) mptsas_config_target(pdip, ptgt);
                        mutex_enter(&mpt->m_mutex);
                }
                ptgt = (mptsas_target_t *)mptsas_hash_traverse(
                    &mpt->m_active->m_tgttbl, MPTSAS_HASH_NEXT);        
        }
        mutex_exit(&mpt->m_mutex);

It turns out that the timeout watchdog callout fires every second. If there are enough devices to be enumerated such that configuring them takes a significant fraction of a second, it's highly likely (or even certain) that it will fire before this thread has finished walking the hash table and configuring targets. In this manner, we see the full complement of targets in the hash table but those toward the end of it will not be configured. Indeed, since configuring a target requires doing I/O, it is not terribly surprising that we manage to configure only a small number of targets on each run. Since the timeout callout has set the cursor to the end of the hash table, when we come back from configuring a target, our next walk of the hash table returns NULL and we think we've configured all the targets.

History

#1 Updated by Robert Mustacchi almost 4 years ago

  • % Done changed from 90 to 100
  • Status changed from New to Resolved

Resolved in da5ab83fc888325fc812733d8a54bc5eab65c65c.

Also available in: Atom