Re: [patch] mm, compaction: avoid isolating pinned pages fix
On Mon, 3 Feb 2014, David Rientjes wrote: > On Tue, 4 Feb 2014, Joonsoo Kim wrote: > > > > > Okay. It can't fix your situation. Anyway, *normal* anon pages may be > > > > mapped > > > > and have positive page_count(), so your code such as > > > > '!page_mapping(page) && page_count(page)' makes compaction skip these > > > > *normal* > > > > anon pages and this is incorrect behaviour. > > > > > > > > > > So how does that work with migrate_page_move_mapping() which demands > > > page_count(page) == 1 and the get_page_unless_zero() in > > > __isolate_lru_page()? > > > > Before doing migrate_page_move_mapping(), try_to_unmap() is called so that > > all > > mapping is unmapped. Then, remained page_count() is 1 which is grabbed by > > __isolate_lru_page(). Am I missing something? > > > > Ah, good point. I wonder if we can get away with > page_count(page) - page_mapcount(page) > 1 to avoid the get_user_pages() > pin? Something like that. But please go back to migrate_page_move_mapping() to factor in what it's additionally considering. Whether you can share code with it, I don't know - it has to do some things under a lock you cannot take at the preliminary stage - you haven't isolated or locked the page yet. There is a separate issue, that a mapping may supply its own non-default mapping->a_ops->migratepage(): can we assume that the page_counting is the same whatever migratepage() is in use? I'm not sure. If you stick to special-casing PageAnon pages, you won't face that issue; but your proposed change would be a lot more satisfying if we can convince ourselves that it's good for !PageAnon too. May need a trawl through the different migratepage() methods that exist in tree. Hugh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] mm, compaction: avoid isolating pinned pages fix
On Tue, 4 Feb 2014, Joonsoo Kim wrote: > > > Okay. It can't fix your situation. Anyway, *normal* anon pages may be > > > mapped > > > and have positive page_count(), so your code such as > > > '!page_mapping(page) && page_count(page)' makes compaction skip these > > > *normal* > > > anon pages and this is incorrect behaviour. > > > > > > > So how does that work with migrate_page_move_mapping() which demands > > page_count(page) == 1 and the get_page_unless_zero() in > > __isolate_lru_page()? > > Before doing migrate_page_move_mapping(), try_to_unmap() is called so that all > mapping is unmapped. Then, remained page_count() is 1 which is grabbed by > __isolate_lru_page(). Am I missing something? > Ah, good point. I wonder if we can get away with page_count(page) - page_mapcount(page) > 1 to avoid the get_user_pages() pin? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] mm, compaction: avoid isolating pinned pages fix
On Mon, Feb 03, 2014 at 06:00:56PM -0800, David Rientjes wrote: > On Tue, 4 Feb 2014, Joonsoo Kim wrote: > > > Okay. It can't fix your situation. Anyway, *normal* anon pages may be mapped > > and have positive page_count(), so your code such as > > '!page_mapping(page) && page_count(page)' makes compaction skip these > > *normal* > > anon pages and this is incorrect behaviour. > > > > So how does that work with migrate_page_move_mapping() which demands > page_count(page) == 1 and the get_page_unless_zero() in > __isolate_lru_page()? Before doing migrate_page_move_mapping(), try_to_unmap() is called so that all mapping is unmapped. Then, remained page_count() is 1 which is grabbed by __isolate_lru_page(). Am I missing something? Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] mm, compaction: avoid isolating pinned pages fix
On Tue, 4 Feb 2014, Joonsoo Kim wrote: > Okay. It can't fix your situation. Anyway, *normal* anon pages may be mapped > and have positive page_count(), so your code such as > '!page_mapping(page) && page_count(page)' makes compaction skip these *normal* > anon pages and this is incorrect behaviour. > So how does that work with migrate_page_move_mapping() which demands page_count(page) == 1 and the get_page_unless_zero() in __isolate_lru_page()? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] mm, compaction: avoid isolating pinned pages fix
On Mon, Feb 03, 2014 at 05:20:46PM -0800, David Rientjes wrote: > On Tue, 4 Feb 2014, Joonsoo Kim wrote: > > > I think that you need more code to skip this type of page correctly. > > Without page_mapped() check, this code makes migratable pages be skipped, > > since if page_mapped() case, page_count() may be more than zero. > > > > So I think that you need following change. > > > > (!page_mapping(page) && !page_mapped(page) && page_count(page)) > > > > These pages returned by get_user_pages() will have a mapcount of 1 so this > wouldn't actually fix the massive lock contention. page_mapping() is only > going to be NULL for pages off the lru like these are for > PAGE_MAPPING_ANON. Okay. It can't fix your situation. Anyway, *normal* anon pages may be mapped and have positive page_count(), so your code such as '!page_mapping(page) && page_count(page)' makes compaction skip these *normal* anon pages and this is incorrect behaviour. Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[patch] mm, compaction: avoid isolating pinned pages fix
On Tue, 4 Feb 2014, Joonsoo Kim wrote: > I think that you need more code to skip this type of page correctly. > Without page_mapped() check, this code makes migratable pages be skipped, > since if page_mapped() case, page_count() may be more than zero. > > So I think that you need following change. > > (!page_mapping(page) && !page_mapped(page) && page_count(page)) > These pages returned by get_user_pages() will have a mapcount of 1 so this wouldn't actually fix the massive lock contention. page_mapping() is only going to be NULL for pages off the lru like these are for PAGE_MAPPING_ANON. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[patch] mm, compaction: avoid isolating pinned pages fix
On Tue, 4 Feb 2014, Joonsoo Kim wrote: I think that you need more code to skip this type of page correctly. Without page_mapped() check, this code makes migratable pages be skipped, since if page_mapped() case, page_count() may be more than zero. So I think that you need following change. (!page_mapping(page) !page_mapped(page) page_count(page)) These pages returned by get_user_pages() will have a mapcount of 1 so this wouldn't actually fix the massive lock contention. page_mapping() is only going to be NULL for pages off the lru like these are for PAGE_MAPPING_ANON. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] mm, compaction: avoid isolating pinned pages fix
On Mon, Feb 03, 2014 at 05:20:46PM -0800, David Rientjes wrote: On Tue, 4 Feb 2014, Joonsoo Kim wrote: I think that you need more code to skip this type of page correctly. Without page_mapped() check, this code makes migratable pages be skipped, since if page_mapped() case, page_count() may be more than zero. So I think that you need following change. (!page_mapping(page) !page_mapped(page) page_count(page)) These pages returned by get_user_pages() will have a mapcount of 1 so this wouldn't actually fix the massive lock contention. page_mapping() is only going to be NULL for pages off the lru like these are for PAGE_MAPPING_ANON. Okay. It can't fix your situation. Anyway, *normal* anon pages may be mapped and have positive page_count(), so your code such as '!page_mapping(page) page_count(page)' makes compaction skip these *normal* anon pages and this is incorrect behaviour. Thanks. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] mm, compaction: avoid isolating pinned pages fix
On Tue, 4 Feb 2014, Joonsoo Kim wrote: Okay. It can't fix your situation. Anyway, *normal* anon pages may be mapped and have positive page_count(), so your code such as '!page_mapping(page) page_count(page)' makes compaction skip these *normal* anon pages and this is incorrect behaviour. So how does that work with migrate_page_move_mapping() which demands page_count(page) == 1 and the get_page_unless_zero() in __isolate_lru_page()? -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] mm, compaction: avoid isolating pinned pages fix
On Mon, Feb 03, 2014 at 06:00:56PM -0800, David Rientjes wrote: On Tue, 4 Feb 2014, Joonsoo Kim wrote: Okay. It can't fix your situation. Anyway, *normal* anon pages may be mapped and have positive page_count(), so your code such as '!page_mapping(page) page_count(page)' makes compaction skip these *normal* anon pages and this is incorrect behaviour. So how does that work with migrate_page_move_mapping() which demands page_count(page) == 1 and the get_page_unless_zero() in __isolate_lru_page()? Before doing migrate_page_move_mapping(), try_to_unmap() is called so that all mapping is unmapped. Then, remained page_count() is 1 which is grabbed by __isolate_lru_page(). Am I missing something? Thanks. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] mm, compaction: avoid isolating pinned pages fix
On Tue, 4 Feb 2014, Joonsoo Kim wrote: Okay. It can't fix your situation. Anyway, *normal* anon pages may be mapped and have positive page_count(), so your code such as '!page_mapping(page) page_count(page)' makes compaction skip these *normal* anon pages and this is incorrect behaviour. So how does that work with migrate_page_move_mapping() which demands page_count(page) == 1 and the get_page_unless_zero() in __isolate_lru_page()? Before doing migrate_page_move_mapping(), try_to_unmap() is called so that all mapping is unmapped. Then, remained page_count() is 1 which is grabbed by __isolate_lru_page(). Am I missing something? Ah, good point. I wonder if we can get away with page_count(page) - page_mapcount(page) 1 to avoid the get_user_pages() pin? -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] mm, compaction: avoid isolating pinned pages fix
On Mon, 3 Feb 2014, David Rientjes wrote: On Tue, 4 Feb 2014, Joonsoo Kim wrote: Okay. It can't fix your situation. Anyway, *normal* anon pages may be mapped and have positive page_count(), so your code such as '!page_mapping(page) page_count(page)' makes compaction skip these *normal* anon pages and this is incorrect behaviour. So how does that work with migrate_page_move_mapping() which demands page_count(page) == 1 and the get_page_unless_zero() in __isolate_lru_page()? Before doing migrate_page_move_mapping(), try_to_unmap() is called so that all mapping is unmapped. Then, remained page_count() is 1 which is grabbed by __isolate_lru_page(). Am I missing something? Ah, good point. I wonder if we can get away with page_count(page) - page_mapcount(page) 1 to avoid the get_user_pages() pin? Something like that. But please go back to migrate_page_move_mapping() to factor in what it's additionally considering. Whether you can share code with it, I don't know - it has to do some things under a lock you cannot take at the preliminary stage - you haven't isolated or locked the page yet. There is a separate issue, that a mapping may supply its own non-default mapping-a_ops-migratepage(): can we assume that the page_counting is the same whatever migratepage() is in use? I'm not sure. If you stick to special-casing PageAnon pages, you won't face that issue; but your proposed change would be a lot more satisfying if we can convince ourselves that it's good for !PageAnon too. May need a trawl through the different migratepage() methods that exist in tree. Hugh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
