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

Reply via email to