Re: [PATCH v5 2/8] of: reserved_mem: add dumpable flag to opt-in vmcore

On 8/6/26 17:47, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Incomplete implementation and write-only dead code: the `dumpable` flag is introduced and set, but the promised kdump filtering and overlap checks are missing.
> --
> 
> --- Patch [2]: [PATCH v5 2/8] of: reserved_mem: add dumpable flag to opt-in vmcore ---
> commit 873defcf6278254b5c39d8d3500f986e38a79da0
> Author: Wandun Chen <[email protected]>
> 
> of: reserved_mem: add dumpable flag to opt-in vmcore
> 
> This commit adds a dumpable flag to struct reserved_mem to track whether
> a reserved area should be included in the kdump vmcore. It sets the flag
> for CMA regions to true in rmem_cma_setup().
> 
>> diff --git a/include/linux/of_reserved_mem.h b/include/linux/of_reserved_mem.h
>> index e8b20b29fa68c..55a67cee41ea0 100644
>> --- a/include/linux/of_reserved_mem.h
>> +++ b/include/linux/of_reserved_mem.h
>> @@ -15,6 +15,7 @@ struct reserved_mem {
>>  	phys_addr_t			base;
>>  	phys_addr_t			size;
>>  	void				*priv;
>> +	bool				dumpable;
> 
> [Severity: Medium]
> Is the logic that checks this flag missing from this patch?
> 
> The commit message states that non-dumpable regions are later marked
> MEMBLOCK_NODUMP, and that the flag is used to check for overlap with
> /memreserve/ entries. However, this flag appears to be write-only,
> as there are no corresponding checks or calls to memblock_mark_nodump()
> implemented in this commit.

Not correct. The checking of dumpable flag is in patch 03 and patch 04.

> 
>>  };
> 
> [ ... ]
> 
>> diff --git a/kernel/dma/contiguous.c b/kernel/dma/contiguous.c
>> index f754079a287d6..63ff134dadd42 100644
>> --- a/kernel/dma/contiguous.c
>> +++ b/kernel/dma/contiguous.c
>> @@ -565,6 +565,7 @@ static int __init rmem_cma_setup(unsigned long node, struct reserved_mem *rmem)
>>  		dma_contiguous_default_area = cma;
>>  
>>  	rmem->priv = cma;
>> +	rmem->dumpable = true;
> 
> [Severity: Medium]
> Does setting this flag have any effect?
> 
> Since the promised kdump filtering and overlap checks are missing from the
> code, this assignment currently acts as dead code. Were those checks meant
> to be included in this submission?

Not dead code, the checking of dumpable flag is in patch 03 and patch 04.>