Messages in this thread Patch in this message | | | Date | Tue, 17 Jan 2006 09:29:10 -0800 (PST) | From | Christoph Lameter <> | Subject | Re: Race in new page migration code? |
| |
On Mon, 16 Jan 2006, Hugh Dickins wrote:
> Hmm, that battery of unusual tests at the start of migrate_page_add > is odd: the tests don't quite match the comment, and it isn't clear > what reasoning lies behind the comment anyway.
Here is patch to clarify the test. I'd be glad if someone could make the tests more accurate. This ultimately comes down to a concept of ownership of page by a process / mm_struct that we have to approximate.
===
Explain the complicated check in migrate_page_add by putting the logic into a separate function migration_check. This way any enhancements can be easily added.
Signed-off-by: Christoph Lameter <clameter@sgi.com>
Index: linux-2.6.15/mm/mempolicy.c =================================================================== --- linux-2.6.15.orig/mm/mempolicy.c 2006-01-14 10:56:28.000000000 -0800 +++ linux-2.6.15/mm/mempolicy.c 2006-01-17 09:24:20.000000000 -0800 @@ -551,6 +551,37 @@ out: return rc; } +static inline int migration_check(struct mm_struct *mm, struct page *page) +{ + /* + * If the page has no mapping then we do not track reverse mappings. + * Thus the page is not mapped by other mms, so its safe to move. + */ + if (page->mapping) + return 1; + + /* + * We cannot determine "ownership" of anonymous pages. + * However, this is the primary set of pages a user would like + * to move. So move the page regardless of sharing. + */ + if (PageAnon(page)) + return 1; + + /* + * If the mapping is writable then its reasonable to assume that + * it is okay to move the page. + */ + if (mapping_writably_mapped(page->mapping)) + return 1; + + /* + * Its a read only file backed mapping. Only migrate the page + * if we are the only process mapping that file. + */ + return single_mm_mapping(mm, page->mapping); +} + /* * Add a page to be migrated to the pagelist */ @@ -558,11 +589,17 @@ static void migrate_page_add(struct vm_a struct page *page, struct list_head *pagelist, unsigned long flags) { /* - * Avoid migrating a page that is shared by others and not writable. + * MPOL_MF_MOVE_ALL migrates all pages. However, migrating all + * pages may also move commonly shared pages (like for example glibc + * pages referenced by all processes). If these are included in + * migration then these pages may be uselessly moved back and + * forth. Migration may also affect the performance of other + * processes. + * + * If MPOL_MF_MOVE_ALL is not set then we try to avoid migrating + * these shared pages. */ - if ((flags & MPOL_MF_MOVE_ALL) || !page->mapping || PageAnon(page) || - mapping_writably_mapped(page->mapping) || - single_mm_mapping(vma->vm_mm, page->mapping)) + if ((flags & MPOL_MF_MOVE_ALL) || migration_check(vma->vm_mm, page)) if (isolate_lru_page(page) == 1) list_add(&page->lru, pagelist); } - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |