Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1408905
| From | Vlastimil Babka <vbabka@suse.cz> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH v6 02/12] mm: migrate: support non-lru movable page migration |
| Date | 2016-05-30 11:10 +0200 |
| Message-ID | <rErmW-2bT-27@gated-at.bofh.it> (permalink) |
| References | <rATB8-6Bl-11@gated-at.bofh.it> <rATB8-6Bl-29@gated-at.bofh.it> <rDqVX-4AO-1@gated-at.bofh.it> <rEkls-5QP-3@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
On 05/30/2016 03:33 AM, Minchan Kim wrote:
>>
>>
>>> + page->mapping = (void *)((unsigned long)page->mapping &
>>> + PAGE_MAPPING_MOVABLE);
>>
>> This should be negated to clear... use ~PAGE_MAPPING_MOVABLE ?
>
> No.
>
> The intention is to clear only mapping value but PAGE_MAPPING_MOVABLE
> flag. So, any new migration trial will be failed because PageMovable
> checks page's mapping value but ongoing migraion handling can catch
> whether it's movable page or not with the type bit.
Oh, OK, I got that wrong. I'll point out in the reply to the v6v2 what
misled me :)
>>
>> So this effectively prevents movable compound pages from being
>> migrated. Are you sure no users of this functionality are going to
>> have compound pages? I assumed that they could, and so made the code
>> like this, with the is_lru variable (which is redundant after your
>> change).
>
> This implementation at the moment disables effectively non-lru compound
> page migration but I'm not a god so I can't make sure no one doesn't want
> it in future. If someone want it, we can support it then because this work
> doesn't prevent it by design.
Oh well. As long as the balloon pages or zsmalloc don't already use
compound pages...
>
> I thouht PageCompound check right before isolate_movable_page in
> isolate_migratepages_block will filter it out mostly but yeah
> it is racy without zone->lru_lock so it could reach to isolate_movable_page.
> However, PageMovable check in there investigates mapping, mapping->a_ops,
> and a_ops->isolate_page to verify whether it's movable page or not.
>
> I thought it's sufficient to filter THP page.
I guess, yeah.
>>
>> [...]
>>
>>> @@ -755,33 +844,69 @@ static int move_to_new_page(struct page *newpage, struct page *page,
>>> enum migrate_mode mode)
>>> {
>>> struct address_space *mapping;
>>> - int rc;
>>> + int rc = -EAGAIN;
>>> + bool is_lru = !__PageMovable(page);
>>>
>>> VM_BUG_ON_PAGE(!PageLocked(page), page);
>>> VM_BUG_ON_PAGE(!PageLocked(newpage), newpage);
>>>
>>> mapping = page_mapping(page);
>>> - if (!mapping)
>>> - rc = migrate_page(mapping, newpage, page, mode);
>>> - else if (mapping->a_ops->migratepage)
>>> - /*
>>> - * Most pages have a mapping and most filesystems provide a
>>> - * migratepage callback. Anonymous pages are part of swap
>>> - * space which also has its own migratepage callback. This
>>> - * is the most common path for page migration.
>>> - */
>>> - rc = mapping->a_ops->migratepage(mapping, newpage, page, mode);
>>> - else
>>> - rc = fallback_migrate_page(mapping, newpage, page, mode);
>>> + /*
>>> + * In case of non-lru page, it could be released after
>>> + * isolation step. In that case, we shouldn't try
>>> + * fallback migration which is designed for LRU pages.
>>> + */
>>
>> Hmm but is_lru was determined from !__PageMovable() above, also well
>> after the isolation step. So if the driver already released it, we
>> wouldn't detect it? And this function is all under same page lock,
>> so if __PageMovable was true above, so will be PageMovable below?
>
> You are missing what I mentioned above.
> We should keep the type bit to catch what you are saying(i.e., driver
> already released).
>
> __PageMovable just checks PAGE_MAPPING_MOVABLE flag and PageMovable
> checks page->mapping valid while __ClearPageMovable reset only
> valid vaule of mapping, not PAGE_MAPPING_MOVABLE flag.
>
> I wrote it down in Documentation/vm/page_migration.
>
> "For testing of non-lru movable page, VM supports __PageMovable function.
> However, it doesn't guarantee to identify non-lru movable page because
> page->mapping field is unified with other variables in struct page.
> As well, if driver releases the page after isolation by VM, page->mapping
> doesn't have stable value although it has PAGE_MAPPING_MOVABLE
> (Look at __ClearPageMovable). But __PageMovable is cheap to catch whether
> page is LRU or non-lru movable once the page has been isolated. Because
> LRU pages never can have PAGE_MAPPING_MOVABLE in page->mapping. It is also
> good for just peeking to test non-lru movable pages before more expensive
> checking with lock_page in pfn scanning to select victim.
>
> For guaranteeing non-lru movable page, VM provides PageMovable function.
> Unlike __PageMovable, PageMovable functions validates page->mapping and
> mapping->a_ops->isolate_page under lock_page. The lock_page prevents sudden
> destroying of page->mapping.
>
> Driver using __SetPageMovable should clear the flag via __ClearMovablePage
> under page_lock before the releasing the page."
Right, I get it now.
>>> + if (!((unsigned long)page->mapping & PAGE_MAPPING_FLAGS))
>>> page->mapping = NULL;
>>
>> The two lines above make little sense to me without a comment.
>
> I folded this.
>
> @@ -901,7 +901,12 @@ static int move_to_new_page(struct page *newpage, struct page *page,
> __ClearPageIsolated(page);
> }
>
> - if (!((unsigned long)page->mapping & PAGE_MAPPING_FLAGS))
> + /*
> + * Anonymous and movable page->mapping will be cleard by
> + * free_pages_prepare so don't reset it here for keeping
> + * the type to work PageAnon, for example.
> + */
> + if (!PageMappingFlags(page))
> page->mapping = NULL;
> }
Thanks.
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH v6 00/12] Support non-lru page migration Minchan Kim <minchan@kernel.org> - 2016-05-20 16:30 +0200
[PATCH v6 03/12] mm: balloon: use general non-lru movable page feature Minchan Kim <minchan@kernel.org> - 2016-05-20 16:30 +0200
Re: [PATCH v6 03/12] mm: balloon: use general non-lru movable page feature Vlastimil Babka <vbabka@suse.cz> - 2016-05-30 14:20 +0200
[PATCH v6 12/12] zram: use __GFP_MOVABLE for memory allocation Minchan Kim <minchan@kernel.org> - 2016-05-20 16:30 +0200
[PATCH v6 05/12] zsmalloc: use bit_spin_lock Minchan Kim <minchan@kernel.org> - 2016-05-20 16:30 +0200
[PATCH v6 11/12] zsmalloc: page migration support Minchan Kim <minchan@kernel.org> - 2016-05-20 16:30 +0200
Re: [PATCH v6 11/12] zsmalloc: page migration support Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-05-24 07:30 +0200
Re: [PATCH v6 11/12] zsmalloc: page migration support Minchan Kim <minchan@kernel.org> - 2016-05-24 08:30 +0200
Re: [PATCH v6 11/12] zsmalloc: page migration support Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-05-24 10:10 +0200
Re: [PATCH v6 11/12] zsmalloc: page migration support Minchan Kim <minchan@kernel.org> - 2016-05-24 10:20 +0200
Re: [PATCH v6 11/12] zsmalloc: page migration support Minchan Kim <minchan@kernel.org> - 2016-05-25 07:20 +0200
Re: [PATCH v6 11/12] zsmalloc: page migration support Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2016-05-25 16:30 +0200
Re: [PATCH v6 11/12] zsmalloc: page migration support Minchan Kim <minchan@kernel.org> - 2016-05-26 02:40 +0200
Re: [PATCH v6 11/12] zsmalloc: page migration support Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-05-26 03:00 +0200
Re: [PATCH v6 11/12] zsmalloc: page migration support Minchan Kim <minchan@kernel.org> - 2016-05-26 06:40 +0200
[PATCH v6r2 11/12] zsmalloc: page migration support Minchan Kim <minchan@kernel.org> - 2016-05-27 00:00 +0200
[PATCH v6 08/12] zsmalloc: introduce zspage structure Minchan Kim <minchan@kernel.org> - 2016-05-20 16:30 +0200
[PATCH v6 06/12] zsmalloc: use accessor Minchan Kim <minchan@kernel.org> - 2016-05-20 16:30 +0200
[PATCH v6 02/12] mm: migrate: support non-lru movable page migration Minchan Kim <minchan@kernel.org> - 2016-05-20 16:30 +0200
Re: [PATCH v6 02/12] mm: migrate: support non-lru movable page migration Vlastimil Babka <vbabka@suse.cz> - 2016-05-27 16:30 +0200
Re: [PATCH v6 02/12] mm: migrate: support non-lru movable page migration Minchan Kim <minchan@kernel.org> - 2016-05-30 03:40 +0200
Re: [PATCH v6 02/12] mm: migrate: support non-lru movable page migration Vlastimil Babka <vbabka@suse.cz> - 2016-05-30 11:10 +0200
PATCH v6v2 02/12] mm: migrate: support non-lru movable page migration Minchan Kim <minchan@kernel.org> - 2016-05-30 03:40 +0200
Re: PATCH v6v2 02/12] mm: migrate: support non-lru movable page migration Vlastimil Babka <vbabka@suse.cz> - 2016-05-30 11:40 +0200
Re: PATCH v6v2 02/12] mm: migrate: support non-lru movable page migration Minchan Kim <minchan@kernel.org> - 2016-05-30 18:30 +0200
Re: PATCH v6v2 02/12] mm: migrate: support non-lru movable page migration Vlastimil Babka <vbabka@suse.cz> - 2016-05-31 10:00 +0200
[PATCH v6v3 02/12] mm: migrate: support non-lru movable page migration Minchan Kim <minchan@kernel.org> - 2016-05-31 02:10 +0200
Re: [PATCH v6v3 02/12] mm: migrate: support non-lru movable page migration Vlastimil Babka <vbabka@suse.cz> - 2016-05-31 10:00 +0200
[PATCH v6 10/12] zsmalloc: use freeobj for index Minchan Kim <minchan@kernel.org> - 2016-05-20 16:30 +0200
[PATCH v6 09/12] zsmalloc: separate free_zspage from putback_zspage Minchan Kim <minchan@kernel.org> - 2016-05-20 16:30 +0200
[PATCH v6 04/12] zsmalloc: keep max_object in size_class Minchan Kim <minchan@kernel.org> - 2016-05-20 16:30 +0200
[PATCH v6 01/12] mm: use put_page to free page instead of putback_lru_page Minchan Kim <minchan@kernel.org> - 2016-05-20 16:30 +0200
[PATCH v6 07/12] zsmalloc: factor page chain functionality out Minchan Kim <minchan@kernel.org> - 2016-05-20 16:30 +0200
csiph-web