Re: [PATCH V11 2/5] mm/memblock: Introduce MEMBLOCK_BOOT flag

[Anshuman Khandual]

On 01/13/2020 01:07 PM, Mike Rapoport wrote:
> On Fri, Jan 10, 2020 at 08:39:12AM +0530, Anshuman Khandual wrote:
>> On arm64 platform boot memory should never be hot removed due to certain
>> platform specific constraints. Hence the platform would like to override
>> earlier added arch call back arch_memory_removable() for this purpose. In
>> order to reject boot memory hot removal request, it needs to first track
>> them at runtime. In the future, there might be other platforms requiring
>> runtime boot memory enumeration. Hence lets expand the existing generic
>> memblock framework for this purpose rather then creating one just for
>> arm64 platforms.
>>
>> This introduces a new memblock flag MEMBLOCK_BOOT along with helpers which
>> can be marked by given platform on all memory regions discovered during
>> boot.
>  
> We already have MEMBLOCK_HOTPLUG to mark hotpluggable region. Can't we use
> it for your use-case?

At present MEMBLOCK_HOTPLUG flag helps in identifying parts of boot memory
as hotpluggable as indicated by the firmware. This information is then used
to avoid those regions during standard memblock_alloc_*() API requests and
later marking them as ZONE_MOVABLE when buddy gets initialized.

Memory hot remove does not check for MEMBLOCK_HOTPLUG flag as a requirement
before initiating the process. We could probably use this flag if generic
hot remove can be changed to check for MEMBLOCK_HOTPLUG as a prerequisite
which will require changes to memblock handling (boot and runtime) on all
existing platforms currently supporting hot remove. But what about handling
the movable boot memory created with movablecore/kernelcore command line,
should generic MM update their memblock regions with MEMBLOCK_HOTPLUG ?

> 
>> Cc: Mike Rapoport <rppt@xxxxxxxxxxxxx>
>> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@xxxxxxx>
>> ---
>>  include/linux/memblock.h | 10 ++++++++++
>>  mm/memblock.c            | 37 +++++++++++++++++++++++++++++++++++++
>>  2 files changed, 47 insertions(+)
>>
>> diff --git a/include/linux/memblock.h b/include/linux/memblock.h
>> index b38bbef..fb04c87 100644
>> --- a/include/linux/memblock.h
>> +++ b/include/linux/memblock.h
>> @@ -31,12 +31,14 @@ extern unsigned long long max_possible_pfn;
>>   * @MEMBLOCK_HOTPLUG: hotpluggable region
>>   * @MEMBLOCK_MIRROR: mirrored region
>>   * @MEMBLOCK_NOMAP: don't add to kernel direct mapping
>> + * @MEMBLOCK_BOOT: memory received from firmware during boot
>>   */
>>  enum memblock_flags {
>>  	MEMBLOCK_NONE		= 0x0,	/* No special request */
>>  	MEMBLOCK_HOTPLUG	= 0x1,	/* hotpluggable region */
>>  	MEMBLOCK_MIRROR		= 0x2,	/* mirrored region */
>>  	MEMBLOCK_NOMAP		= 0x4,	/* don't add to kernel direct mapping */
>> +	MEMBLOCK_BOOT		= 0x8,	/* memory received from firmware during boot */
>>  };
>>  
>>  /**
>> @@ -116,6 +118,8 @@ int memblock_reserve(phys_addr_t base, phys_addr_t size);
>>  void memblock_trim_memory(phys_addr_t align);
>>  bool memblock_overlaps_region(struct memblock_type *type,
>>  			      phys_addr_t base, phys_addr_t size);
>> +int memblock_mark_boot(phys_addr_t base, phys_addr_t size);
>> +int memblock_clear_boot(phys_addr_t base, phys_addr_t size);
>>  int memblock_mark_hotplug(phys_addr_t base, phys_addr_t size);
>>  int memblock_clear_hotplug(phys_addr_t base, phys_addr_t size);
>>  int memblock_mark_mirror(phys_addr_t base, phys_addr_t size);
>> @@ -216,6 +220,11 @@ static inline bool memblock_is_nomap(struct memblock_region *m)
>>  	return m->flags & MEMBLOCK_NOMAP;
>>  }
>>  
>> +static inline bool memblock_is_boot(struct memblock_region *m)
>> +{
>> +	return m->flags & MEMBLOCK_BOOT;
>> +}
>> +
>>  #ifdef CONFIG_HAVE_MEMBLOCK_NODE_MAP
>>  int memblock_search_pfn_nid(unsigned long pfn, unsigned long *start_pfn,
>>  			    unsigned long  *end_pfn);
>> @@ -449,6 +458,7 @@ void memblock_cap_memory_range(phys_addr_t base, phys_addr_t size);
>>  void memblock_mem_limit_remove_map(phys_addr_t limit);
>>  bool memblock_is_memory(phys_addr_t addr);
>>  bool memblock_is_map_memory(phys_addr_t addr);
>> +bool memblock_is_boot_memory(phys_addr_t addr);
>>  bool memblock_is_region_memory(phys_addr_t base, phys_addr_t size);
>>  bool memblock_is_reserved(phys_addr_t addr);
>>  bool memblock_is_region_reserved(phys_addr_t base, phys_addr_t size);
>> diff --git a/mm/memblock.c b/mm/memblock.c
>> index 4bc2c7d..e10207f 100644
>> --- a/mm/memblock.c
>> +++ b/mm/memblock.c
>> @@ -865,6 +865,30 @@ static int __init_memblock memblock_setclr_flag(phys_addr_t base,
>>  }
>>  
>>  /**
>> + * memblock_mark_bootmem - Mark boot memory with flag MEMBLOCK_BOOT.
>> + * @base: the base phys addr of the region
>> + * @size: the size of the region
>> + *
>> + * Return: 0 on success, -errno on failure.
>> + */
>> +int __init_memblock memblock_mark_boot(phys_addr_t base, phys_addr_t size)
>> +{
>> +	return memblock_setclr_flag(base, size, 1, MEMBLOCK_BOOT);
>> +}
>> +
>> +/**
>> + * memblock_clear_bootmem - Clear flag MEMBLOCK_BOOT for a specified region.
>> + * @base: the base phys addr of the region
>> + * @size: the size of the region
>> + *
>> + * Return: 0 on success, -errno on failure.
>> + */
>> +int __init_memblock memblock_clear_boot(phys_addr_t base, phys_addr_t size)
>> +{
>> +	return memblock_setclr_flag(base, size, 0, MEMBLOCK_BOOT);
>> +}
>> +
>> +/**
>>   * memblock_mark_hotplug - Mark hotpluggable memory with flag MEMBLOCK_HOTPLUG.
>>   * @base: the base phys addr of the region
>>   * @size: the size of the region
>> @@ -974,6 +998,10 @@ static bool should_skip_region(struct memblock_region *m, int nid, int flags)
>>  	if ((flags & MEMBLOCK_MIRROR) && !memblock_is_mirror(m))
>>  		return true;
>>  
>> +	/* if we want boot memory skip non-boot memory regions */
>> +	if ((flags & MEMBLOCK_BOOT) && !memblock_is_boot(m))
>> +		return true;
>> +
>>  	/* skip nomap memory unless we were asked for it explicitly */
>>  	if (!(flags & MEMBLOCK_NOMAP) && memblock_is_nomap(m))
>>  		return true;
>> @@ -1785,6 +1813,15 @@ bool __init_memblock memblock_is_map_memory(phys_addr_t addr)
>>  	return !memblock_is_nomap(&memblock.memory.regions[i]);
>>  }
>>  
>> +bool __init_memblock memblock_is_boot_memory(phys_addr_t addr)
>> +{
>> +	int i = memblock_search(&memblock.memory, addr);
>> +
>> +	if (i == -1)
>> +		return false;
>> +	return memblock_is_boot(&memblock.memory.regions[i]);
>> +}
>> +
>>  #ifdef CONFIG_HAVE_MEMBLOCK_NODE_MAP
>>  int __init_memblock memblock_search_pfn_nid(unsigned long pfn,
>>  			 unsigned long *start_pfn, unsigned long *end_pfn)
>> -- 
>> 2.7.4
>>
>