lkml.org 
[lkml]   [2017]   [Aug]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Date
    SubjectRe: [PATCH 1/2] sched/wait: Break up long wake list walk
    On Fri, Aug 18, 2017 at 11:54 AM, Mel Gorman
    <mgorman@techsingularity.net> wrote:
    >
    > One option to mitigate (but not eliminate) the problem is to record when
    > the page lock is contended and pass in TNF_PAGE_CONTENDED (new flag) to
    > task_numa_fault().

    Well, finding it contended is fairly easy - just look at the page wait
    queue, and if it's not empty, assume it's due to contention.

    I also wonder if we could be even *more* hacky, and in the whole
    __migration_entry_wait() path, change the logic from:

    - wait on page lock before retrying the fault

    to

    - yield()

    which is hacky, but there's a rationale for it:

    (a) avoid the crazy long wait queues ;)

    (b) we know that migration is *supposed* to be CPU-bound (not IO
    bound), so yielding the CPU and retrying may just be the right thing
    to do.

    It's possible that we could just do a hybrid approach, and introduce a
    "wait_on_page_lock_or_yield()", that does a sleeping wait if the
    wait-queue is short, and a yield otherwise, but it might be worth just
    testing the truly stupid patch.

    Because that code sequence doesn't actually depend on
    "wait_on_page_lock()" for _correctness_ anyway, afaik. Anybody who
    does "migration_entry_wait()" _has_ to retry anyway, since the page
    table contents may have changed by waiting.

    So I'm not proud of the attached patch, and I don't think it's really
    acceptable as-is, but maybe it's worth testing? And maybe it's
    arguably no worse than what we have now?

    Comments?

    (Yeah, if we take this approach, we might even just say "screw the
    spinlock - just do ACCESS_ONCE() and do a yield() if it looks like a
    migration entry")

    Linus
    mm/migrate.c | 15 +--------------
    1 file changed, 1 insertion(+), 14 deletions(-)

    diff --git a/mm/migrate.c b/mm/migrate.c
    index d68a41da6abb..a28908305841 100644
    --- a/mm/migrate.c
    +++ b/mm/migrate.c
    @@ -284,7 +284,6 @@ void __migration_entry_wait(struct mm_struct *mm, pte_t *ptep,
    {
    pte_t pte;
    swp_entry_t entry;
    - struct page *page;

    spin_lock(ptl);
    pte = *ptep;
    @@ -295,20 +294,8 @@ void __migration_entry_wait(struct mm_struct *mm, pte_t *ptep,
    if (!is_migration_entry(entry))
    goto out;

    - page = migration_entry_to_page(entry);
    -
    - /*
    - * Once radix-tree replacement of page migration started, page_count
    - * *must* be zero. And, we don't want to call wait_on_page_locked()
    - * against a page without get_page().
    - * So, we use get_page_unless_zero(), here. Even failed, page fault
    - * will occur again.
    - */
    - if (!get_page_unless_zero(page))
    - goto out;
    pte_unmap_unlock(ptep, ptl);
    - wait_on_page_locked(page);
    - put_page(page);
    + yield();
    return;
    out:
    pte_unmap_unlock(ptep, ptl);
    \
     
     \ /
      Last update: 2017-08-18 21:14    [W:2.660 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site