Re: [PATCH -v2 0/9] mm: make movable onlining suck less

[Vlastimil Babka]

On 04/10/2017 06:02 PM, Michal Hocko wrote:
> On Mon 10-04-17 16:27:49, Igor Mammedov wrote:
> [...]
>> #issue3:
>> removable flag flipped to non-removable state
>>
>> // before series at commit ef0b577b6:
>> memory32:offline removable: 0  zones: Normal Movable
>> memory33:offline removable: 0  zones: Normal Movable
>> memory34:offline removable: 0  zones: Normal Movable
>> memory35:offline removable: 0  zones: Normal Movable
> 
> did you mean _after_ the series because the bellow looks like
> the original behavior (at least valid_zones).
>  
>> // after series at commit 6a010434
>> memory32:offline removable: 1  zones: Normal
>> memory33:offline removable: 1  zones: Normal
>> memory34:offline removable: 1  zones: Normal
>> memory35:offline removable: 1  zones: Normal Movable
>>
>> also looking at #issue1 removable flag state doesn't
>> seem to be consistent between state changes but maybe that's
>> been broken before
> 
> Well, the file has a very questionable semantic. It doesn't provide
> a stable information. Anyway put that aside.
> is_pageblock_removable_nolock relies on having zone association
> which we do not have yet if the memblock is offline. So we need
> the following. I will queue this as a preparatory patch.
> ---
> From 4f3ebc02f4d552d3fe114787ca8a38cc68702208 Mon Sep 17 00:00:00 2001
> From: Michal Hocko <mhocko@xxxxxxxx>
> Date: Mon, 10 Apr 2017 17:59:03 +0200
> Subject: [PATCH] mm, memory_hotplug: consider offline memblocks removable
> 
> is_pageblock_removable_nolock relies on having zone association to
> examine all the page blocks to check whether they are movable or free.
> This is just wasting of cycles when the memblock is offline. Later patch
> in the series will also change the time when the page is associated with
> a zone so we let's bail out early if the memblock is offline.
> 
> Reported-by: Igor Mammedov <imammedo@xxxxxxxxxx>
> Signed-off-by: Michal Hocko <mhocko@xxxxxxxx>

Acked-by: Vlastimil Babka <vbabka@xxxxxxx>

> ---
>  drivers/base/memory.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
> index 9677b6b711b0..0c29ec5598ea 100644
> --- a/drivers/base/memory.c
> +++ b/drivers/base/memory.c
> @@ -128,6 +128,9 @@ static ssize_t show_mem_removable(struct device *dev,
>  	int ret = 1;
>  	struct memory_block *mem = to_memory_block(dev);
>  
> +	if (mem->stat != MEM_ONLINE)
> +		goto out;
> +
>  	for (i = 0; i < sections_per_block; i++) {
>  		if (!present_section_nr(mem->start_section_nr + i))
>  			continue;
> @@ -135,6 +138,7 @@ static ssize_t show_mem_removable(struct device *dev,
>  		ret &= is_mem_section_removable(pfn, PAGES_PER_SECTION);
>  	}
>  
> +out:
>  	return sprintf(buf, "%d\n", ret);
>  }
>  
>