Re: [PATCH v2 1/5] of: reserved_mem: skip init for regions whose early reservation failed

[Marek Szyprowski <m.szyprowski@samsung.com>]

On 10.09.2026 12:55, Wandun wrote:
> On 9/4/26 16:49, Marek Szyprowski wrote:
>> On 31.08.2026 15:04, Wandun wrote:
>>> On 8/26/26 21:14, Marek Szyprowski wrote:
>>>> On 18.08.2026 11:24, Wandun Chen wrote:
>>>>> From: Wandun Chen <chenwandun@lixiang.com>
>>>>>
>>>>> __reserved_mem_reserve_reg() discards the error from
>>>>> early_init_dt_reserve_memory() and returns 0 unconditionally, so the
>>>>> caller counts the node in total_reserved_mem_cnt and the late scan
>>>>> initializes it without checking whether the early reservation actually
>>>>> succeeded. A region whose reservation failed is then handed to a
>>>>> device assuming the memory is protected.
>>>>>
>>>>> Propagate the error so failed reservations are no longer counted, and
>>>>> record the failed nodes so fdt_scan_reserved_mem_late() can skip them.
>>>>>
>>>>> Recording the failed nodes explicitly is necessary because
>>>>> fdt_scan_reserved_mem_late() rescans the DT independently. It cannot
>>>>> tell from memblock whether early reservation succeeded.
>>>>>
>>>>> The failed-node array is bounded by MAX_RESERVED_REGIONS, the number
>>>>> of static regions is not bounded by it, so on overflow the extra nodes
>>>>> fall back to being initialized, which is the current behavior.
>>>> I'm not very keen on such partial solution. Indeed we have no place to
>>>>
>>>> store the result of the early init call, but we can check if the given
>>>>
>>>> region has been earlier marked in memblock as reserved or no-map in
>>>>
>>>> fdt_scan_reserved_mem_late(). If those attributes don't match the
>>>>
>>>> region can be simply skipped then.
>>> Considering the later patches that reject reservations for overlapping
>>> nodes, checking the memblock state in fdt_scan_reserved_mem_late() may
>>> produce false positives.
>>>
>>> For example, if region A is reserved first and region B is a subset of
>>> A, reserving B will fail because it overlaps with A (in patch 02/03).
>>> However, during fdt_scan_reserved_mem_late(), B will still appear to
>>> be reserved because its range is already covered by A. As a result,
>>> B would be initialized even though its own reservation failed, which
>>> is contrary to the intended behavior.
>> Imho the overlapping reserved regions are some kind of configuration 
>> mismatch and it is enough to detect them. fdt_scan_reserved_mem_late()
>> can first store all regions to dynamic reserved_mem array, then check
>> for overlaps, and only then initialize those, which don't overlap and
>> have proper memblock attributes?
> Thanks a lot for reviewing this series.
>
> I did try this approach, and it looks clean. The overlap check works
> well for regions within /reserved-memory. But there's one case I
> couldn't make it handle, where it seems to still produce a false
> positive, please correct me if I'm missing something.
>
> For example, region A is reserved before /reserved-memory nodes are
> processed. A is not a /reserved-memory node, so it never appears in
> the reserved_mem array. Now region B in /reserved-memory is a subset
> of A. At early reservation, patch 02/03 rejects B due to the overlap.
> But at the late scan, B's range already appears reserved (covered by A),
> so it looks like a successful reservation and gets initialized.
>
> The root issue is that the late scan can only tell whether a region is
> reserved, but it can't tell whether the reservation was made by
> /reserved-memory node itself or by something else. Maybe we still need
> to record during the early reservation stage.
Then maybe it will be easier and cleaner just to add a new flag to 
memblock_flags (see include/linux/memblock.h) and mark each successfully
reserved region with it? There are some spare bits there.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland