Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1409950

Re: PATCH v6v2 02/12] mm: migrate: support non-lru movable page migration

Show all headers | View raw

On 05/30/2016 06:25 PM, Minchan Kim wrote:
>>> --- a/mm/compaction.c
>>> +++ b/mm/compaction.c
>>> @@ -81,6 +81,39 @@ static inline bool migrate_async_suitable(int migratetype)
>>>
>>> #ifdef CONFIG_COMPACTION
>>>
>>> +int PageMovable(struct page *page)
>>> +{
>>> +	struct address_space *mapping;
>>> +
>>> +	VM_BUG_ON_PAGE(!PageLocked(page), page);
>>> +	if (!__PageMovable(page))
>>> +		return 0;
>>> +
>>> +	mapping = page_mapping(page);
>>> +	if (mapping && mapping->a_ops && mapping->a_ops->isolate_page)
>>> +		return 1;
>>> +
>>> +	return 0;
>>> +}
>>> +EXPORT_SYMBOL(PageMovable);
>>> +
>>> +void __SetPageMovable(struct page *page, struct address_space *mapping)
>>> +{
>>> +	VM_BUG_ON_PAGE(!PageLocked(page), page);
>>> +	VM_BUG_ON_PAGE((unsigned long)mapping & PAGE_MAPPING_MOVABLE, page);
>>> +	page->mapping = (void *)((unsigned long)mapping | PAGE_MAPPING_MOVABLE);
>>> +}
>>> +EXPORT_SYMBOL(__SetPageMovable);
>>> +
>>> +void __ClearPageMovable(struct page *page)
>>> +{
>>> +	VM_BUG_ON_PAGE(!PageLocked(page), page);
>>> +	VM_BUG_ON_PAGE(!PageMovable(page), page);
>>> +	page->mapping = (void *)((unsigned long)page->mapping &
>>> +				PAGE_MAPPING_MOVABLE);
>>> +}
>>> +EXPORT_SYMBOL(__ClearPageMovable);
>>
>> The second confusing thing is that the function is named
>> __ClearPageMovable(), but what it really clears is the mapping
>> pointer,
>> which is not at all the opposite of what __SetPageMovable() does.
>>
>> I know it's explained in the documentation, but it also deserves a
>> comment here so it doesn't confuse everyone who looks at it.
>> Even better would be a less confusing name for the function, but I
>> can't offer one right now.
>
> To me, __ClearPageMovable naming is suitable for user POV.
> It effectively makes the page unmovable. The confusion is just caused
> by the implementation and I don't prefer exported API depends on the
> implementation. So I want to add just comment.
>
> I didn't add comment above the function because I don't want to export
> internal implementation to the user. I think they don't need to know it.
>
> index a7df2ae71f2a..d1d2063b4fd9 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -108,6 +108,11 @@ void __ClearPageMovable(struct page *page)
>  {
>         VM_BUG_ON_PAGE(!PageLocked(page), page);
>         VM_BUG_ON_PAGE(!PageMovable(page), page);
> +       /*
> +        * Clear registered address_space val with keeping PAGE_MAPPING_MOVABLE
> +        * flag so that VM can catch up released page by driver after isolation.
> +        * With it, VM migration doesn't try to put it back.
> +        */
>         page->mapping = (void *)((unsigned long)page->mapping &
>                                 PAGE_MAPPING_MOVABLE);

OK, that's fine!

>>
>>> diff --git a/mm/util.c b/mm/util.c
>>> index 917e0e3d0f8e..b756ee36f7f0 100644
>>> --- a/mm/util.c
>>> +++ b/mm/util.c
>>> @@ -399,10 +399,12 @@ struct address_space *page_mapping(struct page *page)
>>> 	}
>>>
>>> 	mapping = page->mapping;
>>
>> I'd probably use READ_ONCE() here to be safe. Not all callers are
>> under page lock?
>
> I don't understand. Yeah, all caller are not under page lock but at least,
> new user of movable pages should call it under page_lock.
> Yeah, I will write the rule down in document.
> In this case, what kinds of problem do you see?

After more thinking, probably none. It wouldn't prevent any extra races. 
Sorry for the noise.

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

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
          Re: [PATCH v6v3 02/12] mm: migrate: support non-lru movable page  migration Minchan Kim <minchan@kernel.org> - 2016-06-01 01:10 +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