Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
From:   David Hildenbrand <david@redhat.com>
Mime-Version: 1.0 (1.0)
Subject: Re: [PATCH V11 2/5] mm/memblock: Introduce MEMBLOCK_BOOT flag
Date:   Mon, 13 Jan 2020 09:57:09 +0100
Message-Id: <12BCAD36-D99C-4AC0-B466-06E1A02DDD72@redhat.com>
References: <08a2f82a-3201-055a-316a-a2f11c7ff7a5@arm.com>
Cc:     Mike Rapoport <rppt@linux.ibm.com>, mark.rutland@arm.com,
        david@redhat.com, catalin.marinas@arm.com, linux-mm@kvack.org,
        arunks@codeaurora.org, cpandya@codeaurora.org, will@kernel.org,
        ira.weiny@intel.com, steven.price@arm.com,
        valentin.schneider@arm.com, suzuki.poulose@arm.com,
        robin.murphy@arm.com, broonie@kernel.org, cai@lca.pw,
        ard.biesheuvel@arm.com, dan.j.williams@intel.com,
        linux-arm-kernel@lists.infradead.org, osalvador@suse.de,
        steve.capper@arm.com, logang@deltatee.com,
        linux-kernel@vger.kernel.org, akpm@linux-foundation.org,
        mgorman@techsingularity.net
In-Reply-To: <08a2f82a-3201-055a-316a-a2f11c7ff7a5@arm.com>
To:     Anshuman Khandual <anshuman.khandual@arm.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


> Am 13.01.2020 um 09:41 schrieb Anshuman Khandual <anshuman.khandual@arm.co=
m>:
>=20
> =EF=BB=BF
>=20
>> 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. I=
n
>>> 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.
>>>=20
>>> This introduces a new memblock flag MEMBLOCK_BOOT along with helpers whi=
ch
>>> can be marked by given platform on all memory regions discovered during
>>> boot.
>>=20
>> We already have MEMBLOCK_HOTPLUG to mark hotpluggable region. Can't we us=
e
>> it for your use-case?
>=20
> At present MEMBLOCK_HOTPLUG flag helps in identifying parts of boot memory=

> as hotpluggable as indicated by the firmware. This information is then use=
d
> to avoid those regions during standard memblock_alloc_*() API requests and=

> later marking them as ZONE_MOVABLE when buddy gets initialized.
>=20
> Memory hot remove does not check for MEMBLOCK_HOTPLUG flag as a requiremen=
t
> 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 handlin=
g
> the movable boot memory created with movablecore/kernelcore command line,
> should generic MM update their memblock regions with MEMBLOCK_HOTPLUG ?

As I said in my other mail, just disallow offlining of the affected (boot) m=
emory blocks using a memory notifier and you should be good to go. No change=
s in memory unplug code required.

>=20
>>=20
>>> Cc: Mike Rapoport <rppt@linux.ibm.com>
>>> Cc: Andrew Morton <akpm@linux-foundation.org>
>>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>>> ---
>>> include/linux/memblock.h | 10 ++++++++++
>>> mm/memblock.c            | 37 +++++++++++++++++++++++++++++++++++++
>>> 2 files changed, 47 insertions(+)
>>>=20
>>> 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        =3D 0x0,    /* No special request */
>>>    MEMBLOCK_HOTPLUG    =3D 0x1,    /* hotpluggable region */
>>>    MEMBLOCK_MIRROR        =3D 0x2,    /* mirrored region */
>>>    MEMBLOCK_NOMAP        =3D 0x4,    /* don't add to kernel direct mappi=
ng */
>>> +    MEMBLOCK_BOOT        =3D 0x8,    /* memory received from firmware d=
uring boot */
>>> };
>>>=20
>>> /**
>>> @@ -116,6 +118,8 @@ int memblock_reserve(phys_addr_t base, phys_addr_t s=
ize);
>>> 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 membloc=
k_region *m)
>>>    return m->flags & MEMBLOCK_NOMAP;
>>> }
>>>=20
>>> +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, phy=
s_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(phy=
s_addr_t base,
>>> }
>>>=20
>>> /**
>>> + * 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 si=
ze)
>>> +{
>>> +    return memblock_setclr_flag(base, size, 1, MEMBLOCK_BOOT);
>>> +}
>>> +
>>> +/**
>>> + * memblock_clear_bootmem - Clear flag MEMBLOCK_BOOT for a specified re=
gion.
>>> + * @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 s=
ize)
>>> +{
>>> +    return memblock_setclr_flag(base, size, 0, MEMBLOCK_BOOT);
>>> +}
>>> +
>>> +/**
>>>  * memblock_mark_hotplug - Mark hotpluggable memory with flag MEMBLOCK_H=
OTPLUG.
>>>  * @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_regi=
on *m, int nid, int flags)
>>>    if ((flags & MEMBLOCK_MIRROR) && !memblock_is_mirror(m))
>>>        return true;
>>>=20
>>> +    /* 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]);
>>> }
>>>=20
>>> +bool __init_memblock memblock_is_boot_memory(phys_addr_t addr)
>>> +{
>>> +    int i =3D memblock_search(&memblock.memory, addr);
>>> +
>>> +    if (i =3D=3D -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)
>>> --=20
>>> 2.7.4
>>>=20
>>=20
>=20