• Glauber Costa's avatar
    cfq: fix lock imbalance with failed allocations · a3cc86c2
    Glauber Costa authored
    While stress-running very-small container scenarios with the Kernel Memory
    Controller, I've run into a lockdep-detected lock imbalance in
    cfq-iosched.c.
    
    I'll apologize beforehand for not posting a backlog: I didn't anticipate
    it would be so hard to reproduce, so I didn't save my serial output and
    went directly on debugging.  Turns out that it did not happen again in
    more than 20 runs, making it a quite rare pattern.
    
    But here is my analysis:
    
    When we are in very low-memory situations, we will arrive at
    cfq_find_alloc_queue and may not find a queue, having to resort to the oom
    queue, in an rcu-locked condition:
    
      if (!cfqq || cfqq == &cfqd->oom_cfqq)
          [ ... ]
    
    Next, we will release the rcu lock, and try to allocate a queue, retrying
    if we succeed:
    
      rcu_read_unlock();
      spin_unlock_irq(cfqd->queue->queue_lock);
      new_cfqq = kmem_cache_alloc_node(cfq_pool,
                      gfp_mask | __GFP_ZERO,
                      cfqd->queue->node);
       spin_lock_irq(cfqd->queue->queue_lock);
       if (new_cfqq)
           goto retry;
    
    We are unlocked at this point, but it should be fine, since we will
    reacquire the rcu_read_lock when we retry.
    
    Except of course, that we may not retry: the allocation may very well fail
    and we'll keep on going through the flow:
    
    The next branch is:
    
        if (cfqq) {
    	[ ... ]
        } else
            cfqq = &cfqd->oom_cfqq;
    
    And right before exiting, we'll issue rcu_read_unlock().
    
    Being already unlocked, this is the likely source of our imbalance.  Since
    cfqq is either already NULL or made NULL in the first statement of the
    outter branch, the only viable alternative here seems to be to return the
    oom queue right away in case of allocation failure.
    
    Please review the following patch and apply if you agree with my analysis.
    Signed-off-by: default avatarGlauber Costa <glommer@parallels.com>
    Cc: Jens Axboe <axboe@kernel.dk>
    Cc: Tejun Heo <tj@kernel.org>
    Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
    Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
    a3cc86c2
cfq-iosched.c 119 KB