lkml.org 
[lkml]   [2011]   [Jun]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    SubjectRe: REGRESSION: Performance regressions from switching anon_vma->lock to mutex
    From
    Date
    On Fri, 2011-06-17 at 20:18 +0200, Peter Zijlstra wrote:
    > On Fri, 2011-06-17 at 11:01 -0700, Linus Torvalds wrote:
    >
    > > So I do think that "page_referenced_anon()" should do a trylock, and
    > > return "referenced" if the trylock fails. Comments?
    >
    > The only problem I can immediately see with that is when a single
    > process' anon memory is dominant, then such an allocation will never
    > succeed in freeing these pages because the one lock will pin pretty much
    > all anon. Then again, there's always a few file pages to drop.
    >
    > That said, its rather unlikely, and iirc people were working on removing
    > direct reclaim, or at least rely less on it.
    >
    >

    something like so I guess, completely untested etc..

    Also, there's a page_lock_anon_vma() user in split_huge_page(), which is
    used in mm/swap_state.c:add_to_swap(), which is also in the reclaim
    path, not quite sure what to do there.

    Aside from the THP thing there's a user in memory-failure.c, which looks
    to be broken as it is because its calling things under tasklist_lock
    which isn't preemptible, but it looks like we can simply swap the
    tasklist_lock vs page_lock_anon_vma.

    ---
    kernel/Makefile | 1 +
    mm/rmap.c | 39 +++++++++++++++++++++++++++++++++++++--
    2 files changed, 38 insertions(+), 2 deletions(-)

    diff --git a/kernel/Makefile b/kernel/Makefile
    index 2d64cfc..f6d05de 100644
    --- a/kernel/Makefile
    +++ b/kernel/Makefile
    @@ -101,6 +101,7 @@ obj-$(CONFIG_RING_BUFFER) += trace/
    obj-$(CONFIG_TRACEPOINTS) += trace/
    obj-$(CONFIG_SMP) += sched_cpupri.o
    obj-$(CONFIG_IRQ_WORK) += irq_work.o
    +obj-m += test.o

    obj-$(CONFIG_PERF_EVENTS) += events/

    diff --git a/mm/rmap.c b/mm/rmap.c
    index 0eb463e..40cd399 100644
    --- a/mm/rmap.c
    +++ b/mm/rmap.c
    @@ -400,6 +400,41 @@ out:
    return anon_vma;
    }

    +struct anon_vma *page_trylock_anon_vma(struct page *page)
    +{
    + struct anon_vma *anon_vma = NULL;
    + struct anon_vma *root_anon_vma;
    + unsigned long anon_mapping;
    +
    + rcu_read_lock();
    + anon_mapping = (unsigned long) ACCESS_ONCE(page->mapping);
    + if ((anon_mapping & PAGE_MAPPING_FLAGS) != PAGE_MAPPING_ANON)
    + goto out;
    + if (!page_mapped(page))
    + goto out;
    +
    + anon_vma = (struct anon_vma *) (anon_mapping - PAGE_MAPPING_ANON);
    + root_anon_vma = ACCESS_ONCE(anon_vma->root);
    + if (!mutex_trylock(&root_anon_vma->mutex)) {
    + anon_vma = NULL;
    + goto out;
    + }
    +
    + /*
    + * If the page is still mapped, then this anon_vma is still
    + * its anon_vma, and holding the mutex ensures that it will
    + * not go away, see anon_vma_free().
    + */
    + if (!page_mapped(page)) {
    + mutex_unlock(&root_anon_vma->mutex);
    + anon_vma = NULL;
    + }
    +
    +out:
    + rcu_read_unlock();
    + return anon_vma;
    +}
    +
    /*
    * Similar to page_get_anon_vma() except it locks the anon_vma.
    *
    @@ -694,7 +729,7 @@ static int page_referenced_anon(struct page *page,
    struct anon_vma_chain *avc;
    int referenced = 0;

    - anon_vma = page_lock_anon_vma(page);
    + anon_vma = page_trylock_anon_vma(page);
    if (!anon_vma)
    return referenced;

    @@ -1396,7 +1431,7 @@ static int try_to_unmap_anon(struct page *page, enum ttu_flags flags)
    struct anon_vma_chain *avc;
    int ret = SWAP_AGAIN;

    - anon_vma = page_lock_anon_vma(page);
    + anon_vma = page_trylock_anon_vma(page);
    if (!anon_vma)
    return ret;




    \
     
     \ /
      Last update: 2011-06-17 20:31    [W:0.028 / U:31.108 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site