Commit 8191acbd authored by Mel Gorman's avatar Mel Gorman Committed by Ingo Molnar
Browse files

mm: numa: Sanitize task_numa_fault() callsites 



There are three callers of task_numa_fault():

 - do_huge_pmd_numa_page():
     Accounts against the current node, not the node where the
     page resides, unless we migrated, in which case it accounts
     against the node we migrated to.

 - do_numa_page():
     Accounts against the current node, not the node where the
     page resides, unless we migrated, in which case it accounts
     against the node we migrated to.

 - do_pmd_numa_page():
     Accounts not at all when the page isn't migrated, otherwise
     accounts against the node we migrated towards.

This seems wrong to me; all three sites should have the same
sementaics, furthermore we should accounts against where the page
really is, we already know where the task is.

So modify all three sites to always account; we did after all receive
the fault; and always account to where the page is after migration,
regardless of success.

They all still differ on when they clear the PTE/PMD; ideally that
would get sorted too.

Signed-off-by: default avatarMel Gorman <mgorman@suse.de>
Reviewed-by: default avatarRik van Riel <riel@redhat.com>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Signed-off-by: default avatarPeter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1381141781-10992-8-git-send-email-mgorman@suse.de


Signed-off-by: default avatarIngo Molnar <mingo@kernel.org>
parent b8916634

mm/huge_memory.c

+13 −12
Original line number Original line Diff line number Diff line
@@ -1281,18 +1281,19 @@ int do_huge_pmd_numa_page(struct mm_struct *mm, struct vm_area_struct *vma, 
	struct anon_vma *anon_vma = NULL;

	struct anon_vma *anon_vma = NULL;

	struct page *page;

	struct page *page;

	unsigned long haddr = addr & HPAGE_PMD_MASK;

	unsigned long haddr = addr & HPAGE_PMD_MASK;

	int page_nid = -1, this_nid = numa_node_id();

	int target_nid;

	int target_nid;

	int current_nid = -1;

	bool page_locked;

	bool migrated, page_locked;

	bool migrated = false;





	spin_lock(&mm->page_table_lock);

	spin_lock(&mm->page_table_lock);

	if (unlikely(!pmd_same(pmd, *pmdp)))

	if (unlikely(!pmd_same(pmd, *pmdp)))

		goto out_unlock;

		goto out_unlock;





	page = pmd_page(pmd);

	page = pmd_page(pmd);

	current_nid = page_to_nid(page);

	page_nid = page_to_nid(page);

	count_vm_numa_event(NUMA_HINT_FAULTS);

	count_vm_numa_event(NUMA_HINT_FAULTS);

	if (current_nid == numa_node_id())

	if (page_nid == this_nid)

		count_vm_numa_event(NUMA_HINT_FAULTS_LOCAL);

		count_vm_numa_event(NUMA_HINT_FAULTS_LOCAL);





	/*

	/*
@@ -1335,19 +1336,18 @@ int do_huge_pmd_numa_page(struct mm_struct *mm, struct vm_area_struct *vma, 
	spin_unlock(&mm->page_table_lock);

	spin_unlock(&mm->page_table_lock);

	migrated = migrate_misplaced_transhuge_page(mm, vma,

	migrated = migrate_misplaced_transhuge_page(mm, vma,

				pmdp, pmd, addr, page, target_nid);

				pmdp, pmd, addr, page, target_nid);

	if (!migrated)

	if (migrated)

		page_nid = target_nid;

	else

		goto check_same;

		goto check_same;





	task_numa_fault(target_nid, HPAGE_PMD_NR, true);

	goto out;

	if (anon_vma)

		page_unlock_anon_vma_read(anon_vma);

	return 0;





check_same:

check_same:

	spin_lock(&mm->page_table_lock);

	spin_lock(&mm->page_table_lock);

	if (unlikely(!pmd_same(pmd, *pmdp))) {

	if (unlikely(!pmd_same(pmd, *pmdp))) {

		/* Someone else took our fault */

		/* Someone else took our fault */

		current_nid = -1;

		page_nid = -1;

		goto out_unlock;

		goto out_unlock;

	}

	}

clear_pmdnuma:

clear_pmdnuma:
@@ -1362,8 +1362,9 @@ out: 
	if (anon_vma)

	if (anon_vma)

		page_unlock_anon_vma_read(anon_vma);

		page_unlock_anon_vma_read(anon_vma);





	if (current_nid != -1)

	if (page_nid != -1)

		task_numa_fault(current_nid, HPAGE_PMD_NR, false);

		task_numa_fault(page_nid, HPAGE_PMD_NR, migrated);



	return 0;

	return 0;

}

}

mm/memory.c

+21 −32
Original line number Original line Diff line number Diff line
@@ -3519,12 +3519,12 @@ static int do_nonlinear_fault(struct mm_struct *mm, struct vm_area_struct *vma, 
}

}





int numa_migrate_prep(struct page *page, struct vm_area_struct *vma,

int numa_migrate_prep(struct page *page, struct vm_area_struct *vma,

				unsigned long addr, int current_nid)

				unsigned long addr, int page_nid)

{

{

	get_page(page);

	get_page(page);





	count_vm_numa_event(NUMA_HINT_FAULTS);

	count_vm_numa_event(NUMA_HINT_FAULTS);

	if (current_nid == numa_node_id())

	if (page_nid == numa_node_id())

		count_vm_numa_event(NUMA_HINT_FAULTS_LOCAL);

		count_vm_numa_event(NUMA_HINT_FAULTS_LOCAL);





	return mpol_misplaced(page, vma, addr);

	return mpol_misplaced(page, vma, addr);
@@ -3535,7 +3535,7 @@ int do_numa_page(struct mm_struct *mm, struct vm_area_struct *vma, 
{

{

	struct page *page = NULL;

	struct page *page = NULL;

	spinlock_t *ptl;

	spinlock_t *ptl;

	int current_nid = -1;

	int page_nid = -1;

	int target_nid;

	int target_nid;

	bool migrated = false;

	bool migrated = false;
@@ -3565,15 +3565,10 @@ int do_numa_page(struct mm_struct *mm, struct vm_area_struct *vma, 
		return 0;

		return 0;

	}

	}





	current_nid = page_to_nid(page);

	page_nid = page_to_nid(page);

	target_nid = numa_migrate_prep(page, vma, addr, current_nid);

	target_nid = numa_migrate_prep(page, vma, addr, page_nid);

	pte_unmap_unlock(ptep, ptl);

	pte_unmap_unlock(ptep, ptl);

	if (target_nid == -1) {

	if (target_nid == -1) {

		/*

		 * Account for the fault against the current node if it not

		 * being replaced regardless of where the page is located.

		 */

		current_nid = numa_node_id();

		put_page(page);

		put_page(page);

		goto out;

		goto out;

	}

	}
@@ -3581,11 +3576,11 @@ int do_numa_page(struct mm_struct *mm, struct vm_area_struct *vma, 
	/* Migrate to the requested node */

	/* Migrate to the requested node */

	migrated = migrate_misplaced_page(page, target_nid);

	migrated = migrate_misplaced_page(page, target_nid);

	if (migrated)

	if (migrated)

		current_nid = target_nid;

		page_nid = target_nid;





out:

out:

	if (current_nid != -1)

	if (page_nid != -1)

		task_numa_fault(current_nid, 1, migrated);

		task_numa_fault(page_nid, 1, migrated);

	return 0;

	return 0;

}

}
@@ -3600,7 +3595,6 @@ static int do_pmd_numa_page(struct mm_struct *mm, struct vm_area_struct *vma, 
	unsigned long offset;

	unsigned long offset;

	spinlock_t *ptl;

	spinlock_t *ptl;

	bool numa = false;

	bool numa = false;

	int local_nid = numa_node_id();





	spin_lock(&mm->page_table_lock);

	spin_lock(&mm->page_table_lock);

	pmd = *pmdp;

	pmd = *pmdp;
@@ -3623,9 +3617,10 @@ static int do_pmd_numa_page(struct mm_struct *mm, struct vm_area_struct *vma, 
	for (addr = _addr + offset; addr < _addr + PMD_SIZE; pte++, addr += PAGE_SIZE) {

	for (addr = _addr + offset; addr < _addr + PMD_SIZE; pte++, addr += PAGE_SIZE) {

		pte_t pteval = *pte;

		pte_t pteval = *pte;

		struct page *page;

		struct page *page;

		int curr_nid = local_nid;

		int page_nid = -1;

		int target_nid;

		int target_nid;

		bool migrated;

		bool migrated = false;



		if (!pte_present(pteval))

		if (!pte_present(pteval))

			continue;

			continue;

		if (!pte_numa(pteval))

		if (!pte_numa(pteval))
@@ -3647,25 +3642,19 @@ static int do_pmd_numa_page(struct mm_struct *mm, struct vm_area_struct *vma, 
		if (unlikely(page_mapcount(page) != 1))

		if (unlikely(page_mapcount(page) != 1))

			continue;

			continue;





		/*

		page_nid = page_to_nid(page);

		 * Note that the NUMA fault is later accounted to either

		target_nid = numa_migrate_prep(page, vma, addr, page_nid);

		 * the node that is currently running or where the page is

		 * migrated to.

		 */

		curr_nid = local_nid;

		target_nid = numa_migrate_prep(page, vma, addr,

					       page_to_nid(page));

		if (target_nid == -1) {

			put_page(page);

			continue;

		}



		/* Migrate to the requested node */

		pte_unmap_unlock(pte, ptl);

		pte_unmap_unlock(pte, ptl);

		if (target_nid != -1) {

			migrated = migrate_misplaced_page(page, target_nid);

			migrated = migrate_misplaced_page(page, target_nid);

			if (migrated)

			if (migrated)

			curr_nid = target_nid;

				page_nid = target_nid;

		task_numa_fault(curr_nid, 1, migrated);

		} else {

			put_page(page);

		}



		if (page_nid != -1)

			task_numa_fault(page_nid, 1, migrated);





		pte = pte_offset_map_lock(mm, pmdp, addr, &ptl);

		pte = pte_offset_map_lock(mm, pmdp, addr, &ptl);

	}

	}

CRA Git | Maintained and supported by SUSTech CRA and CCSE