Re: [PATCH v3 0/5] of: reserved_mem: several fixes about reserved memory

[Wandun <chenwandun1@gmail.com>]

On 9/22/26 16:48, Mike Rapoport wrote:
> On Sun, Sep 20, 2026 at 05:28:47PM +0800, Wandun Chen wrote:
>> From: Wandun Chen <chenwandun@lixiang.com>
>>
>> This series fixes several error-handling issues in the reserved-memory
>> initialization paths.
>>
>> The first two patches fix cleanup of no-map regions after driver
>> initialization failure.
>>
>> Static reserved-memory nodes are reserved during the early DT scan but
>> initialized later. The third patch tags regions whose early reservation
>> succeeded, so the late scan can skip the nodes whose early reservation
>> failed.
>>
>> The last two patches reject overlapping static regions. Without
>> these checks, overlapping nodes can be initialized over the same
>> physical memory, result in data corrupt.
>>
>> Sashiko reported these issues in [1] [2] [3].
>>
>> [1] https://sashiko.dev/#/message/20260814090305.4C8741F00A3D%40smtp.kernel.org
>> [2] https://sashiko.dev/#/message/20260814084718.29C341F000E9%40smtp.kernel.org
>> [3] https://sashiko.dev/#/message/20260806100605.2C2C01F000E9%40smtp.kernel.org
>>
>> v2 --> v3:
>> 1. Rework the mechanism that checks in the late scan whether the early
>>    reservation succeeded (patches 3-5, suggested by Marek, thanks).
>>
>>    Patch 3 adds a new memblock flag MEMBLOCK_RSRV_RMEM, which is set when
>>    the early reservation of a static region succeeds and checked in the
>>    late scan.
> 
> Can we keep this local to of_reserved_mem please?

Probably not. I do not see a way to keep this entirely local to
of_reserved_mem while handling the issue robustly.

I previously implemented an approach in of_reserved_mem that records
static reserved-memory nodes whose early reservation failed in a local
array [1]. However, the early scan runs before paging_init(), so the array
cannot be dynamically expanded. If the number of failed nodes exceeds
the array size, some failures cannot be recorded and the issue remains,
and that is why Marek said "partial solution", although in practice
having that many failed nodes is unlikely.

To handle this robustly, the late scan needs a way to determine whether
the corresponding early reservation actually succeeded. Current approach
uses memblock to retain that state.

[1] https://lore.kernel.org/lkml/20260818092420.2859026-1-chenwandun1@gmail.com/


Best regards,
Wandun

>  
>>    Patches 4 and 5 are reworked to reject regions that overlap or are
>>    contained by an existing reservation. The code makes a little different
>>    from what was acked in v2, so the Acked-by tags for these patches are not
>>    carried over.
>>
>> 2. Reorder the patches: the two cleanup fixes in v2 now come first.
>>    In v3, the first patch now introduces the 'dynamic' distinction in
>>    fdt_init_reserved_mem_node(), which the following patche 3 build on,
>>    so the series reads more fluently.
>>
>> v1 --> v2:
>> 1. Rework failed-node tracking in patch 1: do not track zero-sized nodes,
>>    and keep a reserved_mem slot when tracking overflows.
>> 2. Reject static reserved regions overlapping existing no-map regions.
>> 3. Keep MEMBLOCK_NOMAP flag for static no-map regions when init failure.
>>
>>
>> Wandun Chen (5):
>>   of: reserved_mem: release dynamically allocated no-map region on init
>>     failure
>>   of: reserved_mem: retain static no-map memory on init failure
>>   of: reserved_mem: skip init for regions whose early reservation failed
>>   of: reserved_mem: reject static regions overlapping no-map memory
>>   of: reserved_mem: reject static mapped regions overlapping existing
>>     reservations
>>
>>  drivers/of/of_reserved_mem.c | 86 +++++++++++++++++++++++++-------
>>  include/linux/memblock.h     |  7 +++
>>  mm/memblock.c                | 95 ++++++++++++++++++++++++++++++++++++
>>  3 files changed, 170 insertions(+), 18 deletions(-)
>>
>> -- 
>> 2.43.0
>>
>