linux-kernel - Re: [PATCH] mm/hotplug: Reorder memblock_[free|remove]() calls in try_remove

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 24 Sep 2019 09:42:31 +0530
From:   Anshuman Khandual <anshuman.khandual@....com>
To:     David Hildenbrand <david@...hat.com>,
        Michal Hocko <mhocko@...nel.org>
Cc:     linux-mm@...ck.org, akpm@...ux-foundation.org,
        linux-kernel@...r.kernel.org, Oscar Salvador <osalvador@...e.de>,
        Pavel Tatashin <pasha.tatashin@...een.com>,
        Dan Williams <dan.j.williams@...el.com>
Subject: Re: [PATCH] mm/hotplug: Reorder memblock_[free|remove]() calls in
 try_remove_memory()



On 09/23/2019 04:24 PM, David Hildenbrand wrote:
> On 23.09.19 12:52, Michal Hocko wrote:
>> On Mon 16-09-19 11:17:37, Anshuman Khandual wrote:
>>> In add_memory_resource() the memory range to be hot added first gets into
>>> the memblock via memblock_add() before arch_add_memory() is called on it.
>>> Reverse sequence should be followed during memory hot removal which already
>>> is being followed in add_memory_resource() error path. This now ensures
>>> required re-order between memblock_[free|remove]() and arch_remove_memory()
>>> during memory hot-remove.
>>
>> This changelog is not really easy to follow. First of all please make
>> sure to explain whether there is any actual problem to solve or this is
>> an aesthetic matter. Please think of people reading this changelog in
>> few years and scratching their heads what you were thinking back then...
>>
> 
> I think it would make sense to just draft the current call sequence in
> the add and the removal path (instead of describing it) - then it
> becomes obvious why this is a cosmetic change.

Does this look okay ?

mm/hotplug: Reorder memblock_[free|remove]() calls in try_remove_memory()

Currently during memory hot add procedure, memory gets into memblock before
calling arch_add_memory() which creates it's linear mapping.

add_memory_resource() {
        ..................
        memblock_add_node()
        ..................
        arch_add_memory()
        ..................
}

But during memory hot remove procedure, removal from memblock happens first
before it's linear mapping gets teared down with arch_remove_memory() which
is not coherent. Resource removal should happen in reverse order as they
were added.

try_remove_memory() {
        ..................
        memblock_free()
        memblock_remove()
        ..................
        arch_remove_memory()
        ..................
}

This changes the sequence of resource removal including memblock and linear
mapping tear down during memory hot remove which will now be the reverse
order in which they were added during memory hot add. The changed removal
order looks like the following.

try_remove_memory() {
        ..................
        arch_remove_memory()
        ..................
        memblock_free()
        memblock_remove()
        ..................
}