[lkml]   [2017]   [Aug]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
Patch in this message
SubjectRe: [PATCH 1/2] sched/wait: Break up long wake list walk
On Fri, Aug 18, 2017 at 11:54 AM, Mel Gorman
<> 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


- 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?


(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")

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;

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();
pte_unmap_unlock(ptep, ptl);
 \ /
  Last update: 2017-08-18 21:14    [W:0.081 / U:21.252 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site