[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250425-savvy-chubby-alpaca-0196e3@houat>
Date: Fri, 25 Apr 2025 08:58:35 +0200
From: Maxime Ripard <mripard@...nel.org>
To: John Stultz <jstultz@...gle.com>
Cc: Jared Kangas <jkangas@...hat.com>, sumit.semwal@...aro.org,
benjamin.gaignard@...labora.com, Brian.Starkey@....com, tjmercier@...gle.com,
christian.koenig@....com, linux-media@...r.kernel.org, dri-devel@...ts.freedesktop.org,
linaro-mm-sig@...ts.linaro.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/2] dma-buf: heaps: Give default CMA heap a fixed name
On Thu, Apr 24, 2025 at 05:13:47PM -0700, John Stultz wrote:
> On Thu, Apr 24, 2025 at 1:34 AM Maxime Ripard <mripard@...nel.org> wrote:
> > On Tue, Apr 22, 2025 at 12:19:39PM -0700, Jared Kangas wrote:
> > > @@ -22,6 +22,7 @@
> > > #include <linux/slab.h>
> > > #include <linux/vmalloc.h>
> > >
> > > +#define DEFAULT_CMA_NAME "default_cma"
> >
> > I appreciate this is kind of bikeshed-color territory, but I think "cma"
> > would be a better option here. There's nothing "default" about it.
>
> I disagree. It very much is "default" as it's returning the
> dma_contiguous_default_area.
My main concern here is that it's "default" as opposed to what, exactly?
We have a single CMA allocator. We could have multiple buffer
attributes, but then "cached_cma" would make more sense to me if we
expect to have uncached CMA allocations at some point.
> There can be multiple CMA areas, and out of tree, vendors do reserve
> separate areas for specific purposes, exposing multiple CMA dmabuf
> heaps.
By "CMA areas", I guess you mean carved-out memory regions? If so, how
is it relevant to userspace if we use CMA or any other implementation to
expose a carved-out region, and thus that we carry that implemenattion
detail in the name?
> There have been patches to expose multiple CMA heaps, but with no
> upstream drivers using those purpose specific regions, we haven't
> taken them yet.
> I do hope as the drivers that utilize these purpose focused heaps go
> upstream, we can add that logic, so I think being specific that this
> is default CMA is a good idea.
If heaps names are supposed to carry the region it exposes, then it
should be default_cma_region/area. If heap names are supposed to expose
the allocator (but I don't think it's a good idea), it should be cma. If
they are meant to carry all that plus some policy,
cached_default_cma_region should be used.
Either way, default_cma seems to me either too specific or not specific
enough. And we should really document what the policy for those heaps
are supposed to be.
Maxime
Download attachment "signature.asc" of type "application/pgp-signature" (274 bytes)
Powered by blists - more mailing lists