[lkml]   [2007]   [Feb]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: PREEMPT_RCU breaks anon_vma locking ?
    On Sat, Feb 24, 2007 at 12:23:03AM +0300, Oleg Nesterov wrote:
    > If my understanding correct, vmscan can find a page which lives in a already
    > anon_vma_unlink'ed vma. This is ok, the page is pinned, and page->mapping is
    > not cleared until free_hot_cold_page().
    > So page_lock_anon_vma() works correctly due to SLAB_DESTROY_BY_RCU even if
    > anon_vma_unlink() has already freed anon_vma. In that case we should see
    > list_empty(&anon_vma->head), we are safe.
    > However, we are doing spin_unlock(anon_vma->lock) after page_lock_anon_vma(),
    > and this looks unsafe to me because page_lock_anon_vma() does rcu_read_unlock()
    > on return.

    This would indeed be bad when using CONFIG_PREEMPT_RCU! Good catch!!!

    > This worked before because spin_lock() implied rcu_read_lock(), so rcu was
    > blocked if page_lock_anon_vma() returns !NULL. With CONFIG_PREEMPT_RCU this
    > is not true (yes?), so it is possible that the slab returns the memory to
    > the system and it is re-used when we write to anon_vma->lock.
    > IOW, don't we need something like this
    > static struct anon_vma *page_lock_anon_vma(struct page *page)
    > {
    > struct anon_vma *anon_vma;
    > unsigned long anon_mapping;
    > rcu_read_lock();
    > anon_mapping = (unsigned long) page->mapping;
    > if (!(anon_mapping & PAGE_MAPPING_ANON))
    > goto out;
    > if (!page_mapped(page))
    > goto out;
    > anon_vma = (struct anon_vma *) (anon_mapping - PAGE_MAPPING_ANON);
    > spin_lock(&anon_vma->lock);
    > return anon_vma;
    > out:
    > rcu_read_unlock();
    > return NULL;
    > }
    > static inline void page_lock_anon_vma(struct anon_vma *anon_vma)
    > {
    > spin_unlock(&anon_vma->lock);
    > rcu_read_unlock();
    > }
    > ?

    This look like a valid fix to me, at least as long as the lock is never
    dropped in the meantime (e.g., to do I/O). If the lock -is- dropped in
    the meantime, then presumably whatever is done to keep the page from
    vanishing should allow an rcu_read_unlock() to be placed after each
    spin_unlock(&...->lock) and an rcu_read_lock() to be placed before each

    Thanx, Paul
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to
    More majordomo info at
    Please read the FAQ at

     \ /
      Last update: 2007-02-23 23:45    [W:0.024 / U:5.520 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site