RE: [EXT] Re: [PATCH v2] Supports to use the default CMA when the device-specified CMA memory is not enough.

[Zhai He <zhai.he@nxp.com>]

[-- Attachment #1: Type: text/plain, Size: 5161 bytes --]

> -----Original Message-----
> From: Andrew Morton <akpm@linux-foundation.org>
> Sent: Thursday, June 13, 2024 2:48 AM
> To: Zhai He <zhai.he@nxp.com>
> Cc: sboyd@kernel.org; linux-mm@kvack.org; linux-kernel@vger.kernel.org;
> stable@vger.kernel.org; Zhipeng Wang <zhipeng.wang_1@nxp.com>; Jindong
> Yue <jindong.yue@nxp.com>; Barry Song <baohua@kernel.org>; Christoph
> Hellwig <hch@lst.de>
> Subject: [EXT] Re: [PATCH v2] Supports to use the default CMA when the
> device-specified CMA memory is not enough.
> 
> Caution: This is an external email. Please take care when clicking links
or
> opening attachments. When in doubt, report the message using the 'Report
this
> email' button
> 
> 
> On Wed, 12 Jun 2024 16:12:16 +0800 "zhai.he" <zhai.he@nxp.com> wrote:
> 
> > From: He Zhai <zhai.he@nxp.com>
> 
> (cc Barry & Christoph)
> 
> What was your reason for adding cc:stable to the email headers?  Does this
> address some serious problem?  If so, please fully describe that problem.
> 
Sorry, I don't think this is probably a serious problem, just something I
discovered while developing that I think might exist. 
I will not continue to cc: stable.

> > In the current code logic, if the device-specified CMA memory
> > allocation fails, memory will not be allocated from the default CMA
area.
> > This patch will use the default cma region when the device's specified
> > CMA is not enough.
> >
> > In addition, the log level of allocation failure is changed to debug.
> > Because these logs will be printed when memory allocation from the
> > device specified CMA fails, but if the allocation fails, it will be
> > allocated from the default cma area. It can easily mislead developers'
> > judgment.
> >
> > ...
> >
> > --- a/kernel/dma/contiguous.c
> > +++ b/kernel/dma/contiguous.c
> > @@ -357,8 +357,13 @@ struct page *dma_alloc_contiguous(struct device
> *dev, size_t size, gfp_t gfp)
> >       /* CMA can be used only in the context which permits sleeping */
> >       if (!gfpflags_allow_blocking(gfp))
> >               return NULL;
> > -     if (dev->cma_area)
> > -             return cma_alloc_aligned(dev->cma_area, size, gfp);
> > +     if (dev->cma_area) {
> > +             struct page *page = NULL;
> > +
> > +             page = cma_alloc_aligned(dev->cma_area, size, gfp);
> > +             if (page)
> > +                     return page;
> > +     }
> >       if (size <= PAGE_SIZE)
> >               return NULL;
> 
> The dma_alloc_contiguous() kerneldoc should be updated for this.
> 
> The patch prompts the question "why does the device-specified CMA area
> exist?".  Why not always allocate from the global pool?  If the
device-specified
> area exists to prevent one device from going crazy and consuming too much
> contiguous memory, this patch violates that intent?
>

In our environment, because of Widevine DRM, we enable secure heap. When the
VPU decodes 4K secure video, 
the VPU needs to allocate a large amount of secure contiguous memory. This
will cause us to need to shrink the CMA
in order not to affect other devices that require contiguous memory. So I
specified CMA memory for VPU, But currently, in addition 
to the secure heap and CMA, the remaining continuous memory in our memory
configuration is not enough to support the VPU decoding 
high-resolution code streams, so I added this patch so that when the CMA
specified by the device is not enough, 
allocated in default CMA.

Another reason is that the secure heap requires secure configuration, so the
start address of the secure heap cannot be specified arbitrarily. 
Therefore, in order to reduce the impact of shrinking CMA, I used
multi-segment CMA and assign one of the CMA 
to the VPU. When the VPU decodes the high-resolution stream, if the
specified CMA is not enough, it will continue to be allocated from the
default CMA.

The VPU will not consume a crazy amount of memory, it just requires more
memory when playing high-resolution streams.
Therefore, this patch is mainly to prevent the situation where the global
CMA size cannot be satisfied.

> > @@ -406,6 +411,8 @@ void dma_free_contiguous(struct device *dev, struct
> page *page, size_t size)
> >       if (dev->cma_area) {
> >               if (cma_release(dev->cma_area, page, count))
> >                       return;
> > +             if (cma_release(dma_contiguous_default_area, page,
> count))
> > +                     return;
> >       } else {
> >               /*
> >                * otherwise, page is from either per-numa cma or
> > default cma diff --git a/mm/cma.c b/mm/cma.c index
> > 3e9724716bad..6e12faf1bea7 100644
> > --- a/mm/cma.c
> > +++ b/mm/cma.c
> > @@ -495,8 +495,8 @@ struct page *cma_alloc(struct cma *cma, unsigned
> long count,
> >       }
> >
> >       if (ret && !no_warn) {
> > -             pr_err_ratelimited("%s: %s: alloc failed, req-size: %lu
pages,
> ret: %d\n",
> > -                                __func__, cma->name, count, ret);
> > +             pr_debug("%s: alloc failed, req-size: %lu pages, ret: %d,
try to
> use default cma\n",
> > +                         cma->name, count, ret);
> >               cma_debug_show_areas(cma);
> >       }


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 9790 bytes --]