Re: [RFC PATCH v8 06/14] dma-pool: Allocate CoCo atomic pools using CoCo shared memory allocator

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] dma-pool: atomic_pool_expand() falsely returns success on failure
--

commit 7de8bc3a8ea9e2bbef83e09d72fa88999ce898b9
Author: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>

dma-pool: Allocate CoCo atomic pools using CoCo shared memory allocator

This commit modifies atomic DMA pools to use the CoCo shared memory allocator.
It prevents allocation fallback below the required shared granule size, and
uses the cc_make_shared() helper to transition the backing allocations in a
sleepable context.

> diff --git a/kernel/dma/pool.c b/kernel/dma/pool.c
> index 70b7f64b17ab5..f059312c15aa5 100644
> --- a/kernel/dma/pool.c
> +++ b/kernel/dma/pool.c
[ ... ]
> @@ -85,6 +85,9 @@ static bool cma_in_zone(gfp_t gfp)
>  static int atomic_pool_expand(struct dma_gen_pool *dma_pool, size_t pool_size,
>  			      gfp_t gfp)
>  {
> +	struct cc_shared_layout layout;
> +	unsigned long attrs = 0;
> +	unsigned int min_order = 0;
>  	unsigned int order;
>  	struct page *page = NULL;
>  	bool leak_pages = false;
> @@ -92,6 +95,17 @@ static int atomic_pool_expand(struct dma_gen_pool *dma_pool, size_t pool_size,
>  	int ret = -ENOMEM;
>  	pgprot_t prot __maybe_unused;
>  
> +	if (dma_pool->cc_shared) {
> +		ret = cc_shared_calc_layout(pool_size, &layout);
> +		if (ret)
> +			goto out;

[Severity: High]
If cc_shared_calc_layout() succeeds, it returns 0 and overwrites the initial
ret = -ENOMEM. Could this cause the function to falsely return 0 (success) on
subsequent failures?

For instance, if page allocation fails below, the function jumps to out and
returns the current value of ret:

    if (!page)
        goto out;

Similarly, if remapping fails later in the function:

    addr = dma_common_contiguous_remap(...)
    if (!addr)
        goto free_page;

In both cases, ret will be 0 instead of -ENOMEM. Would it be better to reset
ret back to -ENOMEM when these failures occur, or use a separate variable
for the cc_shared_calc_layout() return value?

> +		pool_size = layout.shared_size;
> +		min_order = get_order(layout.alignment);
> +		if (min_order > MAX_PAGE_ORDER)
> +			return -E2BIG;
> +		attrs = __DMA_ATTR_ALLOC_CC_SHARED;
> +	}
> +
>  	/* Cannot allocate larger than MAX_PAGE_ORDER */
>  	order = min(get_order(pool_size), MAX_PAGE_ORDER);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260924100529.1398790-1-aneesh.kumar@kernel.org?part=6