On Thu, May 14, 2026 at 8:01 AM Aneesh Kumar K.V
<[email protected]> wrote:
>
> Mostafa Saleh <[email protected]> writes:
>
> ...
>
> >> struct page *dma_alloc_from_pool(struct device *dev, size_t size,
> >> - void **cpu_addr, gfp_t gfp,
> >> + void **cpu_addr, gfp_t gfp, unsigned long attrs,
> >> bool (*phys_addr_ok)(struct device *, phys_addr_t, size_t))
> >> {
> >> - struct gen_pool *pool = NULL;
> >> + struct dma_gen_pool *dma_pool = NULL;
> >> struct page *page;
> >> bool pool_found = false;
> >>
> >> - while ((pool = dma_guess_pool(pool, gfp))) {
> >> + while ((dma_pool = dma_guess_pool(dma_pool, gfp))) {
> >> +
> >> + if (dma_pool->unencrypted != !!(attrs & DMA_ATTR_CC_SHARED))
> >> + continue;
> >> +
> >
> > nit: If we fail to find a matching pool, a slightly misleading message
> > is printed as pool_found = false
> >
>
> The message printed is
>
> WARN(1, "Failed to get suitable pool for %s\n", dev_name(dev));
>
> That is correct, isn’t it? The kernel failed to find a pool with the
> correct encryption attribute. For example, the request was for an
> encrypted allocation from the pool, but no encrypted pool was available.
>
Sure, I’d prefer a clearer print in that case, especially since that’s new code:
“Only {encrypted/decrypted} pool found for a {encrypted/decrypted} alloction”
But no strong opinion.
Thanks,
Mostafa
> >
> >> pool_found = true;
> >> - page = __dma_alloc_from_pool(dev, size, pool, cpu_addr,
> >> + page = __dma_alloc_from_pool(dev, size, dma_pool->pool,
> >> cpu_addr,
> >> phys_addr_ok);
> >> if (page)
> >> return page;
> >> @@ -296,12 +345,14 @@ struct page *dma_alloc_from_pool(struct device *dev,
> >> size_t size,
>
> -aneesh