[PATCH v2 0/2] dma-buf: heaps: Use constant name for CMA heap

[PATCH v2 0/2] dma-buf: heaps: Use constant name for CMA heap

[Jared Kangas]

Hi all,

This patch series is based on a previous discussion around CMA heap
naming. [1] The heap's name depends on the device name, which is
generally "reserved", "linux,cma", or "default-pool", but could be any
arbitrary name given to the default CMA area in the devicetree. For a
consistent userspace interface, the series introduces a constant name
for the CMA heap, and for backwards compatibility, an additional Kconfig
that controls the creation of a legacy-named heap with the same CMA
backing.

The ideas to handle backwards compatibility in [1] are to either use a
symlink or add a heap node with a duplicate minor. However, I assume
that we don't want to create symlinks in /dev from module initcalls, and
attempting to duplicate minors would cause device_create() to fail.
Because of these drawbacks, after brainstorming with Maxime Ripard, I
went with creating a new node in devtmpfs with its own minor. This
admittedly makes it a little unclear that the old and new nodes are
backed by the same heap when both are present. The only approach that I
think would provide total clarity on this in userspace is symlinking,
which seemed like a fairly involved solution for devtmpfs, but if I'm
wrong on this, please let me know.

Changelog:
    v2: Use tabs instead of spaces for large vertical alignment.

Jared Kangas (2):
  dma-buf: heaps: Parameterize heap name in __add_cma_heap()
  dma-buf: heaps: Give default CMA heap a fixed name

 Documentation/userspace-api/dma-buf-heaps.rst | 11 ++++---
 drivers/dma-buf/heaps/Kconfig                 | 10 +++++++
 drivers/dma-buf/heaps/cma_heap.c              | 30 ++++++++++++++-----
 3 files changed, 40 insertions(+), 11 deletions(-)

-- 
2.49.0

[PATCH v2 1/2] dma-buf: heaps: Parameterize heap name in __add_cma_heap()

[Jared Kangas]

Prepare for the introduction of a fixed-name CMA heap by replacing the
unused void pointer parameter in __add_cma_heap() with the heap name.

Signed-off-by: Jared Kangas <jkangas@redhat.com>
---
 drivers/dma-buf/heaps/cma_heap.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c
index 9512d050563a9..e998d8ccd1dc6 100644
--- a/drivers/dma-buf/heaps/cma_heap.c
+++ b/drivers/dma-buf/heaps/cma_heap.c
@@ -366,17 +366,17 @@ static const struct dma_heap_ops cma_heap_ops = {
 	.allocate = cma_heap_allocate,
 };
 
-static int __init __add_cma_heap(struct cma *cma, void *data)
+static int __init __add_cma_heap(struct cma *cma, const char *name)
 {
-	struct cma_heap *cma_heap;
 	struct dma_heap_export_info exp_info;
+	struct cma_heap *cma_heap;
 
 	cma_heap = kzalloc(sizeof(*cma_heap), GFP_KERNEL);
 	if (!cma_heap)
 		return -ENOMEM;
 	cma_heap->cma = cma;
 
-	exp_info.name = cma_get_name(cma);
+	exp_info.name = name;
 	exp_info.ops = &cma_heap_ops;
 	exp_info.priv = cma_heap;
 
@@ -394,12 +394,16 @@ static int __init __add_cma_heap(struct cma *cma, void *data)
 static int __init add_default_cma_heap(void)
 {
 	struct cma *default_cma = dev_get_cma_area(NULL);
-	int ret = 0;
+	int ret;
 
-	if (default_cma)
-		ret = __add_cma_heap(default_cma, NULL);
+	if (!default_cma)
+		return 0;
 
-	return ret;
+	ret = __add_cma_heap(default_cma, cma_get_name(default_cma));
+	if (ret)
+		return ret;
+
+	return 0;
 }
 module_init(add_default_cma_heap);
 MODULE_DESCRIPTION("DMA-BUF CMA Heap");
-- 
2.49.0

[PATCH v2 2/2] dma-buf: heaps: Give default CMA heap a fixed name

[Jared Kangas]

The CMA heap's name in devtmpfs can vary depending on how the heap is
defined. Its name defaults to "reserved", but if a CMA area is defined
in the devicetree, the heap takes on the devicetree node's name, such as
"default-pool" or "linux,cma". To simplify naming, just name it
"default_cma", and keep a legacy node in place backed by the same
underlying structure for backwards compatibility.

Signed-off-by: Jared Kangas <jkangas@redhat.com>
---
 Documentation/userspace-api/dma-buf-heaps.rst | 11 +++++++----
 drivers/dma-buf/heaps/Kconfig                 | 10 ++++++++++
 drivers/dma-buf/heaps/cma_heap.c              | 14 +++++++++++++-
 3 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/Documentation/userspace-api/dma-buf-heaps.rst b/Documentation/userspace-api/dma-buf-heaps.rst
index 535f49047ce64..577de813ba461 100644
--- a/Documentation/userspace-api/dma-buf-heaps.rst
+++ b/Documentation/userspace-api/dma-buf-heaps.rst
@@ -19,7 +19,10 @@ following heaps:
  - The ``cma`` heap allocates physically contiguous, cacheable,
    buffers. Only present if a CMA region is present. Such a region is
    usually created either through the kernel commandline through the
-   `cma` parameter, a memory region Device-Tree node with the
-   `linux,cma-default` property set, or through the `CMA_SIZE_MBYTES` or
-   `CMA_SIZE_PERCENTAGE` Kconfig options. Depending on the platform, it
-   might be called ``reserved``, ``linux,cma``, or ``default-pool``.
+   ``cma`` parameter, a memory region Device-Tree node with the
+   ``linux,cma-default`` property set, or through the ``CMA_SIZE_MBYTES`` or
+   ``CMA_SIZE_PERCENTAGE`` Kconfig options. The heap's name in devtmpfs is
+   ``default_cma``. For backwards compatibility, when the
+   ``DMABUF_HEAPS_CMA_LEGACY`` Kconfig option is set, a duplicate node is
+   created following legacy naming conventions; the legacy name might be
+   ``reserved``, ``linux,cma``, or ``default-pool``.
diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma-buf/heaps/Kconfig
index a5eef06c42264..83f3770fa820a 100644
--- a/drivers/dma-buf/heaps/Kconfig
+++ b/drivers/dma-buf/heaps/Kconfig
@@ -12,3 +12,13 @@ config DMABUF_HEAPS_CMA
 	  Choose this option to enable dma-buf CMA heap. This heap is backed
 	  by the Contiguous Memory Allocator (CMA). If your system has these
 	  regions, you should say Y here.
+
+config DMABUF_HEAPS_CMA_LEGACY
+	bool "DMA-BUF CMA Heap"
+	default y
+	depends on DMABUF_HEAPS_CMA
+	help
+	  Add a duplicate CMA-backed dma-buf heap with legacy naming derived
+	  from the CMA area's devicetree node, or "reserved" if the area is not
+	  defined in the devicetree. This uses the same underlying allocator as
+	  CONFIG_DMABUF_HEAPS_CMA.
diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c
index e998d8ccd1dc6..cd742c961190d 100644
--- a/drivers/dma-buf/heaps/cma_heap.c
+++ b/drivers/dma-buf/heaps/cma_heap.c
@@ -22,6 +22,7 @@
 #include <linux/slab.h>
 #include <linux/vmalloc.h>
 
+#define DEFAULT_CMA_NAME "default_cma"
 
 struct cma_heap {
 	struct dma_heap *heap;
@@ -394,15 +395,26 @@ static int __init __add_cma_heap(struct cma *cma, const char *name)
 static int __init add_default_cma_heap(void)
 {
 	struct cma *default_cma = dev_get_cma_area(NULL);
+	const char *legacy_cma_name;
 	int ret;
 
 	if (!default_cma)
 		return 0;
 
-	ret = __add_cma_heap(default_cma, cma_get_name(default_cma));
+	ret = __add_cma_heap(default_cma, DEFAULT_CMA_NAME);
 	if (ret)
 		return ret;
 
+	legacy_cma_name = cma_get_name(default_cma);
+
+	if (IS_ENABLED(CONFIG_DMABUF_HEAPS_CMA_LEGACY) &&
+	    strcmp(legacy_cma_name, DEFAULT_CMA_NAME)) {
+		ret = __add_cma_heap(default_cma, legacy_cma_name);
+		if (ret)
+			pr_warn("cma_heap: failed to add legacy heap: %pe\n",
+				ERR_PTR(-ret));
+	}
+
 	return 0;
 }
 module_init(add_default_cma_heap);
-- 
2.49.0

Re: [PATCH v2 1/2] dma-buf: heaps: Parameterize heap name in __add_cma_heap()

[John Stultz]

On Tue, Apr 22, 2025 at 12:19 PM Jared Kangas <jkangas@redhat.com> wrote:
>
> Prepare for the introduction of a fixed-name CMA heap by replacing the
> unused void pointer parameter in __add_cma_heap() with the heap name.
>
> Signed-off-by: Jared Kangas <jkangas@redhat.com>

Thanks so much for taking this effort on. Looks good to me!

Acked-by: John Stultz <jstultz@google.com>

Re: [PATCH v2 2/2] dma-buf: heaps: Give default CMA heap a fixed name

[John Stultz]

On Tue, Apr 22, 2025 at 12:19 PM Jared Kangas <jkangas@redhat.com> wrote:
>
> The CMA heap's name in devtmpfs can vary depending on how the heap is
> defined. Its name defaults to "reserved", but if a CMA area is defined
> in the devicetree, the heap takes on the devicetree node's name, such as
> "default-pool" or "linux,cma". To simplify naming, just name it
> "default_cma", and keep a legacy node in place backed by the same
> underlying structure for backwards compatibility.
>
> Signed-off-by: Jared Kangas <jkangas@redhat.com>

Once again, thanks for working out how to improve the standard naming
while keeping compatibility.

I do still hope we can get to the point where other cma regions can be
registered as heaps with unique/purpose-specific names, but I can see
having a standard name for the default region is a nice improvement.

Acked-by: John Stultz <jstultz@google.com>

thanks
-john

Re: [PATCH v2 0/2] dma-buf: heaps: Use constant name for CMA heap

[Sumit Semwal]

Hello Jared,

On Wed, 23 Apr 2025 at 00:49, Jared Kangas <jkangas@redhat.com> wrote:
>
> Hi all,
>
> This patch series is based on a previous discussion around CMA heap
> naming. [1] The heap's name depends on the device name, which is
> generally "reserved", "linux,cma", or "default-pool", but could be any
> arbitrary name given to the default CMA area in the devicetree. For a
> consistent userspace interface, the series introduces a constant name
> for the CMA heap, and for backwards compatibility, an additional Kconfig
> that controls the creation of a legacy-named heap with the same CMA
> backing.
>
> The ideas to handle backwards compatibility in [1] are to either use a
> symlink or add a heap node with a duplicate minor. However, I assume
> that we don't want to create symlinks in /dev from module initcalls, and
> attempting to duplicate minors would cause device_create() to fail.
> Because of these drawbacks, after brainstorming with Maxime Ripard, I
> went with creating a new node in devtmpfs with its own minor. This
> admittedly makes it a little unclear that the old and new nodes are
> backed by the same heap when both are present. The only approach that I
> think would provide total clarity on this in userspace is symlinking,
> which seemed like a fairly involved solution for devtmpfs, but if I'm
> wrong on this, please let me know.

Thanks indeed for this patch; just one minor nit: the link referred to
as [1] here seems to be missing. Could you please add it? This would
make it easier to follow the chain of discussion in posterity.
>
> Changelog:
>     v2: Use tabs instead of spaces for large vertical alignment.
>
> Jared Kangas (2):
>   dma-buf: heaps: Parameterize heap name in __add_cma_heap()
>   dma-buf: heaps: Give default CMA heap a fixed name
>
>  Documentation/userspace-api/dma-buf-heaps.rst | 11 ++++---
>  drivers/dma-buf/heaps/Kconfig                 | 10 +++++++
>  drivers/dma-buf/heaps/cma_heap.c              | 30 ++++++++++++++-----
>  3 files changed, 40 insertions(+), 11 deletions(-)
>
> --
> 2.49.0
>

Best,
Sumit

Re: [PATCH v2 0/2] dma-buf: heaps: Use constant name for CMA heap

[Jared Kangas]

Hi Sumit,

On Wed, Apr 23, 2025 at 08:53:20PM +0530, Sumit Semwal wrote:
> Hello Jared,
> 
> On Wed, 23 Apr 2025 at 00:49, Jared Kangas <jkangas@redhat.com> wrote:
> >
> > Hi all,
> >
> > This patch series is based on a previous discussion around CMA heap
> > naming. [1] The heap's name depends on the device name, which is
> > generally "reserved", "linux,cma", or "default-pool", but could be any
> > arbitrary name given to the default CMA area in the devicetree. For a
> > consistent userspace interface, the series introduces a constant name
> > for the CMA heap, and for backwards compatibility, an additional Kconfig
> > that controls the creation of a legacy-named heap with the same CMA
> > backing.
> >
> > The ideas to handle backwards compatibility in [1] are to either use a
> > symlink or add a heap node with a duplicate minor. However, I assume
> > that we don't want to create symlinks in /dev from module initcalls, and
> > attempting to duplicate minors would cause device_create() to fail.
> > Because of these drawbacks, after brainstorming with Maxime Ripard, I
> > went with creating a new node in devtmpfs with its own minor. This
> > admittedly makes it a little unclear that the old and new nodes are
> > backed by the same heap when both are present. The only approach that I
> > think would provide total clarity on this in userspace is symlinking,
> > which seemed like a fairly involved solution for devtmpfs, but if I'm
> > wrong on this, please let me know.
> 
> Thanks indeed for this patch; just one minor nit: the link referred to
> as [1] here seems to be missing. Could you please add it? This would
> make it easier to follow the chain of discussion in posterity.

My bad, I must have dropped the link while revising the cover letter.
Here's the dropped reference:

[1]: https://lore.kernel.org/all/f6412229-4606-41ad-8c05-7bbba2eb6e08@ti.com/

Thanks to you and John for looking this over,
Jared

> >
> > Changelog:
> >     v2: Use tabs instead of spaces for large vertical alignment.
> >
> > Jared Kangas (2):
> >   dma-buf: heaps: Parameterize heap name in __add_cma_heap()
> >   dma-buf: heaps: Give default CMA heap a fixed name
> >
> >  Documentation/userspace-api/dma-buf-heaps.rst | 11 ++++---
> >  drivers/dma-buf/heaps/Kconfig                 | 10 +++++++
> >  drivers/dma-buf/heaps/cma_heap.c              | 30 ++++++++++++++-----
> >  3 files changed, 40 insertions(+), 11 deletions(-)
> >
> > --
> > 2.49.0
> >
> 
> Best,
> Sumit
> 

Re: [PATCH v2 2/2] dma-buf: heaps: Give default CMA heap a fixed name

[Maxime Ripard]

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

Hi Jared,

Thanks for working on this

On Tue, Apr 22, 2025 at 12:19:39PM -0700, Jared Kangas wrote:
> The CMA heap's name in devtmpfs can vary depending on how the heap is
> defined. Its name defaults to "reserved", but if a CMA area is defined
> in the devicetree, the heap takes on the devicetree node's name, such as
> "default-pool" or "linux,cma". To simplify naming, just name it
> "default_cma", and keep a legacy node in place backed by the same
> underlying structure for backwards compatibility.
> 
> Signed-off-by: Jared Kangas <jkangas@redhat.com>
> ---
>  Documentation/userspace-api/dma-buf-heaps.rst | 11 +++++++----
>  drivers/dma-buf/heaps/Kconfig                 | 10 ++++++++++
>  drivers/dma-buf/heaps/cma_heap.c              | 14 +++++++++++++-
>  3 files changed, 30 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/userspace-api/dma-buf-heaps.rst b/Documentation/userspace-api/dma-buf-heaps.rst
> index 535f49047ce64..577de813ba461 100644
> --- a/Documentation/userspace-api/dma-buf-heaps.rst
> +++ b/Documentation/userspace-api/dma-buf-heaps.rst
> @@ -19,7 +19,10 @@ following heaps:
>   - The ``cma`` heap allocates physically contiguous, cacheable,
>     buffers. Only present if a CMA region is present. Such a region is
>     usually created either through the kernel commandline through the
> -   `cma` parameter, a memory region Device-Tree node with the
> -   `linux,cma-default` property set, or through the `CMA_SIZE_MBYTES` or
> -   `CMA_SIZE_PERCENTAGE` Kconfig options. Depending on the platform, it
> -   might be called ``reserved``, ``linux,cma``, or ``default-pool``.
> +   ``cma`` parameter, a memory region Device-Tree node with the
> +   ``linux,cma-default`` property set, or through the ``CMA_SIZE_MBYTES`` or
> +   ``CMA_SIZE_PERCENTAGE`` Kconfig options. The heap's name in devtmpfs is
> +   ``default_cma``. For backwards compatibility, when the
> +   ``DMABUF_HEAPS_CMA_LEGACY`` Kconfig option is set, a duplicate node is
> +   created following legacy naming conventions; the legacy name might be
> +   ``reserved``, ``linux,cma``, or ``default-pool``.

It looks like, in addition to documenting the new naming, you also
changed all the backticks to double backticks. Why did you do so? It
seems mostly unrelated to that patch, so it would be better in a
separate patch.

> diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma-buf/heaps/Kconfig
> index a5eef06c42264..83f3770fa820a 100644
> --- a/drivers/dma-buf/heaps/Kconfig
> +++ b/drivers/dma-buf/heaps/Kconfig
> @@ -12,3 +12,13 @@ config DMABUF_HEAPS_CMA
>  	  Choose this option to enable dma-buf CMA heap. This heap is backed
>  	  by the Contiguous Memory Allocator (CMA). If your system has these
>  	  regions, you should say Y here.
> +
> +config DMABUF_HEAPS_CMA_LEGACY
> +	bool "DMA-BUF CMA Heap"
> +	default y
> +	depends on DMABUF_HEAPS_CMA
> +	help
> +	  Add a duplicate CMA-backed dma-buf heap with legacy naming derived
> +	  from the CMA area's devicetree node, or "reserved" if the area is not
> +	  defined in the devicetree. This uses the same underlying allocator as
> +	  CONFIG_DMABUF_HEAPS_CMA.
> diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c
> index e998d8ccd1dc6..cd742c961190d 100644
> --- a/drivers/dma-buf/heaps/cma_heap.c
> +++ b/drivers/dma-buf/heaps/cma_heap.c
> @@ -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.

>  struct cma_heap {
>  	struct dma_heap *heap;
> @@ -394,15 +395,26 @@ static int __init __add_cma_heap(struct cma *cma, const char *name)
>  static int __init add_default_cma_heap(void)
>  {
>  	struct cma *default_cma = dev_get_cma_area(NULL);
> +	const char *legacy_cma_name;
>  	int ret;
>  
>  	if (!default_cma)
>  		return 0;
>  
> -	ret = __add_cma_heap(default_cma, cma_get_name(default_cma));
> +	ret = __add_cma_heap(default_cma, DEFAULT_CMA_NAME);
>  	if (ret)
>  		return ret;
>  
> +	legacy_cma_name = cma_get_name(default_cma);
> +
> +	if (IS_ENABLED(CONFIG_DMABUF_HEAPS_CMA_LEGACY) &&
> +	    strcmp(legacy_cma_name, DEFAULT_CMA_NAME)) {
> +		ret = __add_cma_heap(default_cma, legacy_cma_name);
> +		if (ret)
> +			pr_warn("cma_heap: failed to add legacy heap: %pe\n",
> +				ERR_PTR(-ret));
> +	}
> +

It would also simplify this part, since you would always create the legacy heap.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 2/2] dma-buf: heaps: Give default CMA heap a fixed name

[Jared Kangas]

Hi Maxime,

On Thu, Apr 24, 2025 at 10:33:58AM +0200, Maxime Ripard wrote:
> Hi Jared,
> 
> Thanks for working on this
> 
> On Tue, Apr 22, 2025 at 12:19:39PM -0700, Jared Kangas wrote:
> > The CMA heap's name in devtmpfs can vary depending on how the heap is
> > defined. Its name defaults to "reserved", but if a CMA area is defined
> > in the devicetree, the heap takes on the devicetree node's name, such as
> > "default-pool" or "linux,cma". To simplify naming, just name it
> > "default_cma", and keep a legacy node in place backed by the same
> > underlying structure for backwards compatibility.
> > 
> > Signed-off-by: Jared Kangas <jkangas@redhat.com>
> > ---
> >  Documentation/userspace-api/dma-buf-heaps.rst | 11 +++++++----
> >  drivers/dma-buf/heaps/Kconfig                 | 10 ++++++++++
> >  drivers/dma-buf/heaps/cma_heap.c              | 14 +++++++++++++-
> >  3 files changed, 30 insertions(+), 5 deletions(-)
> > 
> > diff --git a/Documentation/userspace-api/dma-buf-heaps.rst b/Documentation/userspace-api/dma-buf-heaps.rst
> > index 535f49047ce64..577de813ba461 100644
> > --- a/Documentation/userspace-api/dma-buf-heaps.rst
> > +++ b/Documentation/userspace-api/dma-buf-heaps.rst
> > @@ -19,7 +19,10 @@ following heaps:
> >   - The ``cma`` heap allocates physically contiguous, cacheable,
> >     buffers. Only present if a CMA region is present. Such a region is
> >     usually created either through the kernel commandline through the
> > -   `cma` parameter, a memory region Device-Tree node with the
> > -   `linux,cma-default` property set, or through the `CMA_SIZE_MBYTES` or
> > -   `CMA_SIZE_PERCENTAGE` Kconfig options. Depending on the platform, it
> > -   might be called ``reserved``, ``linux,cma``, or ``default-pool``.
> > +   ``cma`` parameter, a memory region Device-Tree node with the
> > +   ``linux,cma-default`` property set, or through the ``CMA_SIZE_MBYTES`` or
> > +   ``CMA_SIZE_PERCENTAGE`` Kconfig options. The heap's name in devtmpfs is
> > +   ``default_cma``. For backwards compatibility, when the
> > +   ``DMABUF_HEAPS_CMA_LEGACY`` Kconfig option is set, a duplicate node is
> > +   created following legacy naming conventions; the legacy name might be
> > +   ``reserved``, ``linux,cma``, or ``default-pool``.
> 
> It looks like, in addition to documenting the new naming, you also
> changed all the backticks to double backticks. Why did you do so? It
> seems mostly unrelated to that patch, so it would be better in a
> separate patch.

Ah, I thought that since it was touching the immediate area and it's a
small fix it would be suitable for this patch. Thanks for the catch,
I'll extract this in v3.

As for the rationale: as I understand it, the single backticks here are
semantically incorrect, at least in general reST -- they're mainly used
for links and roles. In this instance, the syntax is interpreted as the
default role:

    https://docutils.sourceforge.io/docs/ref/rst/roles.html

I believe double backticks are what the doc is looking for here, used
for code and rendered as monospaced text. Although there are a number of
places around existing documentation that use single backticks (which
happens to render as italics because the default role is
:title-reference: if not configured in conf.py), a look through doc
history points to double backticks being treated as correct for code,
such as f6314b76d826 ("docs: kbuild/kconfig: reformat/cleanup") or
2f0e2a39bbab ("docs/kbuild/makefiles: unify quoting").

> 
> > diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma-buf/heaps/Kconfig
> > index a5eef06c42264..83f3770fa820a 100644
> > --- a/drivers/dma-buf/heaps/Kconfig
> > +++ b/drivers/dma-buf/heaps/Kconfig
> > @@ -12,3 +12,13 @@ config DMABUF_HEAPS_CMA
> >  	  Choose this option to enable dma-buf CMA heap. This heap is backed
> >  	  by the Contiguous Memory Allocator (CMA). If your system has these
> >  	  regions, you should say Y here.
> > +
> > +config DMABUF_HEAPS_CMA_LEGACY
> > +	bool "DMA-BUF CMA Heap"
> > +	default y
> > +	depends on DMABUF_HEAPS_CMA
> > +	help
> > +	  Add a duplicate CMA-backed dma-buf heap with legacy naming derived
> > +	  from the CMA area's devicetree node, or "reserved" if the area is not
> > +	  defined in the devicetree. This uses the same underlying allocator as
> > +	  CONFIG_DMABUF_HEAPS_CMA.
> > diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c
> > index e998d8ccd1dc6..cd742c961190d 100644
> > --- a/drivers/dma-buf/heaps/cma_heap.c
> > +++ b/drivers/dma-buf/heaps/cma_heap.c
> > @@ -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'm a little uncertain about plain "cma"; John mentioned the possibility
of other CMA regions, suggesting that if we used "cma", we'd need to
rename it again in the future to disambiguate. It doesn't sound
guaranteed that other regions will be added, so I can see starting with
"cma" and adjusting later if needed, but "default_cma" seemed
inoffensive enough because it's consistent with current wording (e.g.,
"linux,cma-default", "default-pool"), and has a lower chance of changing
if other CMA regions/heaps were added. Let me know what you think.

> 
> >  struct cma_heap {
> >  	struct dma_heap *heap;
> > @@ -394,15 +395,26 @@ static int __init __add_cma_heap(struct cma *cma, const char *name)
> >  static int __init add_default_cma_heap(void)
> >  {
> >  	struct cma *default_cma = dev_get_cma_area(NULL);
> > +	const char *legacy_cma_name;
> >  	int ret;
> >  
> >  	if (!default_cma)
> >  		return 0;
> >  
> > -	ret = __add_cma_heap(default_cma, cma_get_name(default_cma));
> > +	ret = __add_cma_heap(default_cma, DEFAULT_CMA_NAME);
> >  	if (ret)
> >  		return ret;
> >  
> > +	legacy_cma_name = cma_get_name(default_cma);
> > +
> > +	if (IS_ENABLED(CONFIG_DMABUF_HEAPS_CMA_LEGACY) &&
> > +	    strcmp(legacy_cma_name, DEFAULT_CMA_NAME)) {
> > +		ret = __add_cma_heap(default_cma, legacy_cma_name);
> > +		if (ret)
> > +			pr_warn("cma_heap: failed to add legacy heap: %pe\n",
> > +				ERR_PTR(-ret));
> > +	}
> > +
> 
> It would also simplify this part, since you would always create the legacy heap.

By "always", do you mean removing the strcmp? I added this to guard
against cases where the devicetree node's name clashed with the default
name, given that the DT name isn't necessarily restricted to one of the
current names in use ("linux,cma" or "default-pool"). It seems like the
strcmp would be relevant regardless of the naming choice, but if this is
overly cautious, I can remove it in v3.

Jared

> 
> Maxime

Re: [PATCH v2 2/2] dma-buf: heaps: Give default CMA heap a fixed name

[John Stultz]

On Thu, Apr 24, 2025 at 1:34 AM Maxime Ripard <mripard@kernel.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.

There can be multiple CMA areas, and out of tree, vendors do reserve
separate areas for specific purposes, exposing multiple CMA dmabuf
heaps.
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.

thanks
-john

Re: [PATCH v2 2/2] dma-buf: heaps: Give default CMA heap a fixed name

[Maxime Ripard]

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

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@kernel.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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]

Re: [PATCH v2 2/2] dma-buf: heaps: Give default CMA heap a fixed name

[John Stultz]

On Thu, Apr 24, 2025 at 11:58 PM Maxime Ripard <mripard@kernel.org> wrote:
> 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@kernel.org> wrote:
> > > 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.

Well, there may be one CMA allocator, but there can be multiple CMA regions.

So in the kernel, cma_alloc() always takes the cma area as an
argument.  And dma_alloc_contiguous() lets you do allocations against
a device, which may reference a specific cma area. Or if the device
doesn't specify a region it will utilize the default region.

> > 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?

So, no, I don't mean carve-out regions.  It's more about dealing with
competition between multiple CMA users. In some cases, where there are
known fixed buffer sizes, say camera buffers, it's much easier to
reserve a separate specific sized region to allocate from so that you
know it will always succeed and you don't need to waste much on safety
margins. Having this added as a separate CMA region makes it a lot
easier to account or reason about, and the kernel can still make
(limited) use of the CMA space when it's idle. Then you don't have to
worry about some other device having a short term cma allocation that
pushes back the alignment for your large allocation, possibly
impacting some other devices larger allocations.

And unlike with just using a carveout, you don't end up just wasting
all that space when it is unused.

So userland may want to allocate contiguous memory, but it may also be
relevant to userland to be able to allocate contiguous memory from a
purpose specific pool.

And while not used in Android, you could imagine having separate
purpose reserved cma heaps with different permissions on the heap
devnodes, allowing less trusted applications to allocate cma from a
small pool without having the potential to DoS the system.

> > 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.

I don't see it as such a problem. It is clear it is cma, it also is
clear conceptually that it is the "default" region that the kernel
uses when devices aren't specific.
But I wouldn't object to cma_default_region/area as a name either, but
I don't see it as particularly improved over cma_default.

To your larger point about policy, I do get the tension that you want
to be able to programmatically derive or evaluate heap names, so that
applications can consistently derive a pathname to get what they want.
But I also think that there is so much variety in both the devices and
uses that there is no way that all use cases and all devices can be
satisfied with such a static or even programmatic mapping. From my
perspective, there just is going to have to be some device specific
glue logic that maps use->heap name. Same reason we have fstab and the
passwd file.  That said, I think advocating for naming conventions is
definitely useful, but I'm wary of trying to enforce too specific a
schema on the names as the incompleteness theorem will bite us.

thanks
-john

Re: [PATCH v2 2/2] dma-buf: heaps: Give default CMA heap a fixed name

[Maxime Ripard]

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

Hi John,

On Fri, Apr 25, 2025 at 12:39:40PM -0700, John Stultz wrote:
> On Thu, Apr 24, 2025 at 11:58 PM Maxime Ripard <mripard@kernel.org> wrote:
> > 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@kernel.org> wrote:
> > > > 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.
> 
> Well, there may be one CMA allocator, but there can be multiple CMA regions.
> 
> So in the kernel, cma_alloc() always takes the cma area as an
> argument.  And dma_alloc_contiguous() lets you do allocations against
> a device, which may reference a specific cma area. Or if the device
> doesn't specify a region it will utilize the default region.
> 
> > > 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?
> 
> So, no, I don't mean carve-out regions.  It's more about dealing with
> competition between multiple CMA users. In some cases, where there are
> known fixed buffer sizes, say camera buffers, it's much easier to
> reserve a separate specific sized region to allocate from so that you
> know it will always succeed and you don't need to waste much on safety
> margins. Having this added as a separate CMA region makes it a lot
> easier to account or reason about, and the kernel can still make
> (limited) use of the CMA space when it's idle. Then you don't have to
> worry about some other device having a short term cma allocation that
> pushes back the alignment for your large allocation, possibly
> impacting some other devices larger allocations.
> 
> And unlike with just using a carveout, you don't end up just wasting
> all that space when it is unused.

The way I see it, it's an implementation detail and is abstracted away
from userspace. That's what I meant by carved-out I guess: a region
dedicated to a (set of) devices(s) that the rest of the system won't
use from userspace point of view.

> So userland may want to allocate contiguous memory, but it may also be
> relevant to userland to be able to allocate contiguous memory from a
> purpose specific pool.
> 
> And while not used in Android, you could imagine having separate
> purpose reserved cma heaps with different permissions on the heap
> devnodes, allowing less trusted applications to allocate cma from a
> small pool without having the potential to DoS the system.

Yeah... I don't think it's the right approach for that. If Android
doesn't use it, and if it's the only Linux distro with 1 app / 1 user
policy, then the only permissions we'll effectively have is one for the
whole use, trusted and untrusted apps alike.

cgroups look like a much better path forward, and wouldn't require
multiple heaps.

Anyway... It's not really important at this point I guess.

> > > 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.
> 
> I don't see it as such a problem. It is clear it is cma, it also is
> clear conceptually that it is the "default" region that the kernel
> uses when devices aren't specific.
> But I wouldn't object to cma_default_region/area as a name either, but
> I don't see it as particularly improved over cma_default.
> 
> To your larger point about policy, I do get the tension that you want
> to be able to programmatically derive or evaluate heap names, so that
> applications can consistently derive a pathname to get what they want.

We've discussed it in the past, I don't really want to. But it was clear
from the last discussion that you (plural) wanted to infer heap
semantics from the names. I'm ok with that, but then if we want to make
it work we need to have well defined names.

And it's actually what I really want to discuss here: we've discussed at
length how bad the heaps name are (and not only here), but I don't think
we have any documented policy on what makes a good name?

For example, I'm not sure exposing the allocator name is a good idea:
it's an implementation detail and for all userspace cares about, we
could change it every release if it provided the same kind of buffers.

Taking your camera buffers example before, then we could also expose a
memory region id, and let the platform figure it out, or use the usecase
as the name.

But if we don't document that, how can we possibly expect everyone
including downstream to come up with perfect names every time. And FTR,
I'm willing to write that doc down once the discussion settles.

> But I also think that there is so much variety in both the devices and
> uses that there is no way that all use cases and all devices can be
> satisfied with such a static or even programmatic mapping. From my
> perspective, there just is going to have to be some device specific
> glue logic that maps use->heap name. Same reason we have fstab and the
> passwd file.

fstab and passwd can be generated at (first) boot time / install. fstab
is also being somewhat less important with the auto-partition discovery.
How would you generate that configuration file at boot?

I'm not really asking this as a theoretical question. Being able to
auto-discover which heap a driver/device would allocate from is central
for the cgroup work I mentioned earlier.

And I'm really not sure how distros or applications developpers are
supposed to keep up with the raw volume of devices that go out every
year, each and every one of them having different heap names, etc.
Possibly different from one version of the firmware to another.

> That said, I think advocating for naming conventions is definitely
> useful, but I'm wary of trying to enforce too specific a schema on the
> names as the incompleteness theorem will bite us.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]

Re: [PATCH v2 2/2] dma-buf: heaps: Give default CMA heap a fixed name

[John Stultz]

On Mon, Apr 28, 2025 at 7:52 AM Maxime Ripard <mripard@kernel.org> wrote:
> On Fri, Apr 25, 2025 at 12:39:40PM -0700, John Stultz wrote:
> > To your larger point about policy, I do get the tension that you want
> > to be able to programmatically derive or evaluate heap names, so that
> > applications can consistently derive a pathname to get what they want.
>
> We've discussed it in the past, I don't really want to. But it was clear
> from the last discussion that you (plural) wanted to infer heap
> semantics from the names. I'm ok with that, but then if we want to make
> it work we need to have well defined names.

So my name keeps on getting attached to that, but I don't think I was
involved in the LPC conversation when that got decided.

> And it's actually what I really want to discuss here: we've discussed at
> length how bad the heaps name are (and not only here), but I don't think
> we have any documented policy on what makes a good name?

I very much think having a policy/guidance for better names is a good goal.

I just want to make sure it doesn't become a strict policy that lead
folks to make mistaken assumptions about a static solution being
viable in userland (like folks nostalgicly using "eth0" or a fixed
network device name in scripts expecting it to work on a different
system)

> For example, I'm not sure exposing the allocator name is a good idea:
> it's an implementation detail and for all userspace cares about, we
> could change it every release if it provided the same kind of buffers.

That is a fair point.

> Taking your camera buffers example before, then we could also expose a
> memory region id, and let the platform figure it out, or use the usecase
> as the name.
>
> But if we don't document that, how can we possibly expect everyone
> including downstream to come up with perfect names every time. And FTR,
> I'm willing to write that doc down once the discussion settles.

So again, yeah, I very much support having better guidance on the names.

I think the number of device constraints and device combinations makes
a raw enumeration of things difficult.

This is why the per-device use->heap mapping "glue" seems necessary to me.

And, I do get that this runs into a similar problem with enumerating
and defining "uses" (which boil down to a combination of
devices-in-a-pipeline and access use patterns), but for Andorid it has
so far been manageable.

Personally, I think the best idea I've heard so far to resolve this
from userland was Christian's suggestion that devices expose links to
compatible heaps, and then userland without a use->heap mapping could
for the set of devices they plan to use in a pipeline, figure out the
common heap name and use that to allocate.

However, that pushes the problem down a bit, requiring drivers
(instead of userland) to know what heaps they can work with and what
the names might be (which again, your goal for standardizing the heap
names isn't a bad thing!). Though, this approach also runs into
trouble as it opens a question of: should it only encode strict
constraint satisfaction, or something more subtle, as while something
might work with multiple heaps, its possible it won't be performant
enough unless it picks a specific one on device A or a different one
on device B.  And getting that sort of device-specific details
embedded into a driver isn't great either.

> > But I also think that there is so much variety in both the devices and
> > uses that there is no way that all use cases and all devices can be
> > satisfied with such a static or even programmatic mapping. From my
> > perspective, there just is going to have to be some device specific
> > glue logic that maps use->heap name. Same reason we have fstab and the
> > passwd file.
>
> fstab and passwd can be generated at (first) boot time / install. fstab
> is also being somewhat less important with the auto-partition discovery.
> How would you generate that configuration file at boot?
>
> I'm not really asking this as a theoretical question. Being able to
> auto-discover which heap a driver/device would allocate from is central
> for the cgroup work I mentioned earlier.
>
> And I'm really not sure how distros or applications developpers are
> supposed to keep up with the raw volume of devices that go out every
> year, each and every one of them having different heap names, etc.
> Possibly different from one version of the firmware to another.

For generic distros, I don't have a good answer here. Historically the
focus has always been on single device usage, so having the driver do
the allocation was fine, and if you were using multiple devices you
could just copy the memory between the driver allocated buffers.  But
as we've moved to disaggregated IP blocks and device pipelines, all
those potential copies wreck performance and power.   I'm not sure
generic distros have the concept of a device pipeline very well
abstracted (obviously mesa and the wayland/X have had to deal with it,
and the video and camera side is dealing with it more and more).
Maybe a more established notion of use -> pipeline/device collections,
is needed as a starting point? Then using Christian's suggestion, one
could at least enumerate  use -> heap that would be functional. And
maybe device makers could then supplement explicit optimized mapping
overrides for their device?

I just think leaving individual applications (or even individual
frameworks like mesa) to embed assumptions about heap names ->
functionality is going to be a problematic approach.

thanks
-john

Re: [PATCH v2 2/2] dma-buf: heaps: Give default CMA heap a fixed name

[Maxime Ripard]

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

On Tue, Apr 29, 2025 at 09:25:00AM -0700, John Stultz wrote:
> On Mon, Apr 28, 2025 at 7:52 AM Maxime Ripard <mripard@kernel.org> wrote:
> > On Fri, Apr 25, 2025 at 12:39:40PM -0700, John Stultz wrote:
> > > To your larger point about policy, I do get the tension that you want
> > > to be able to programmatically derive or evaluate heap names, so that
> > > applications can consistently derive a pathname to get what they want.
> >
> > We've discussed it in the past, I don't really want to. But it was clear
> > from the last discussion that you (plural) wanted to infer heap
> > semantics from the names. I'm ok with that, but then if we want to make
> > it work we need to have well defined names.
> 
> So my name keeps on getting attached to that, but I don't think I was
> involved in the LPC conversation when that got decided.

Sorry then :/

That's what I recalled, but I guess the pastries got the best of me :)

Does that mean that you disagree with this point though? or just that
you agree but still wanted to point out you were not part of that
discussion?

> > And it's actually what I really want to discuss here: we've discussed at
> > length how bad the heaps name are (and not only here), but I don't think
> > we have any documented policy on what makes a good name?
> 
> I very much think having a policy/guidance for better names is a good goal.
> 
> I just want to make sure it doesn't become a strict policy that lead
> folks to make mistaken assumptions about a static solution being
> viable in userland (like folks nostalgicly using "eth0" or a fixed
> network device name in scripts expecting it to work on a different
> system)

I think that's one of the point where the "derive the buffer attributes"
from the name interact badly though. In your example, eth0 wouldn't have
had any non-discoverable guarantees. So it can have any name you want,
it doesn't matter, you can always discover it through some other mean,
and go from there.

If we say the name is how you can associate a heap and the kind of
buffers you get, then we can't just use another heap name just like
that. We could get buffers with a totally different semantics.

I mean, it would probably work with Android, but for any other
distribution, even if we came up with a gralloc-like solution, as soon
as you start updating the kernel and whatever is using the heaps
separately, it's game over. And pretty much all non-Android distros do?

> > For example, I'm not sure exposing the allocator name is a good idea:
> > it's an implementation detail and for all userspace cares about, we
> > could change it every release if it provided the same kind of buffers.
> 
> That is a fair point.
> 
> > Taking your camera buffers example before, then we could also expose a
> > memory region id, and let the platform figure it out, or use the usecase
> > as the name.
> >
> > But if we don't document that, how can we possibly expect everyone
> > including downstream to come up with perfect names every time. And FTR,
> > I'm willing to write that doc down once the discussion settles.
> 
> So again, yeah, I very much support having better guidance on the names.
> 
> I think the number of device constraints and device combinations makes
> a raw enumeration of things difficult.
> 
> This is why the per-device use->heap mapping "glue" seems necessary to me.
> 
> And, I do get that this runs into a similar problem with enumerating
> and defining "uses" (which boil down to a combination of
> devices-in-a-pipeline and access use patterns), but for Andorid it has
> so far been manageable.
> 
> Personally, I think the best idea I've heard so far to resolve this
> from userland was Christian's suggestion that devices expose links to
> compatible heaps, and then userland without a use->heap mapping could
> for the set of devices they plan to use in a pipeline, figure out the
> common heap name and use that to allocate.

I plan to work on that, but also, it covers only what the driver cares
about, ie, buffer location, etc. It doesn't really cover what userspace
might care about, like whether the buffer is cachable or not. Both would
work for any driver, but userspace will have to prefer one over the
other if it plans to do CPU accesses.

So we'd still need some (arguably more limited) enumeration on the
userspace side.

> However, that pushes the problem down a bit, requiring drivers
> (instead of userland) to know what heaps they can work with and what
> the names might be (which again, your goal for standardizing the heap
> names isn't a bad thing!). Though, this approach also runs into
> trouble as it opens a question of: should it only encode strict
> constraint satisfaction, or something more subtle, as while something
> might work with multiple heaps, its possible it won't be performant
> enough unless it picks a specific one on device A or a different one
> on device B.  And getting that sort of device-specific details
> embedded into a driver isn't great either.

Yeah :/

> > > But I also think that there is so much variety in both the devices and
> > > uses that there is no way that all use cases and all devices can be
> > > satisfied with such a static or even programmatic mapping. From my
> > > perspective, there just is going to have to be some device specific
> > > glue logic that maps use->heap name. Same reason we have fstab and the
> > > passwd file.
> >
> > fstab and passwd can be generated at (first) boot time / install. fstab
> > is also being somewhat less important with the auto-partition discovery.
> > How would you generate that configuration file at boot?
> >
> > I'm not really asking this as a theoretical question. Being able to
> > auto-discover which heap a driver/device would allocate from is central
> > for the cgroup work I mentioned earlier.
> >
> > And I'm really not sure how distros or applications developpers are
> > supposed to keep up with the raw volume of devices that go out every
> > year, each and every one of them having different heap names, etc.
> > Possibly different from one version of the firmware to another.
> 
> For generic distros, I don't have a good answer here. Historically the
> focus has always been on single device usage, so having the driver do
> the allocation was fine, and if you were using multiple devices you
> could just copy the memory between the driver allocated buffers.  But
> as we've moved to disaggregated IP blocks and device pipelines, all
> those potential copies wreck performance and power.   I'm not sure
> generic distros have the concept of a device pipeline very well
> abstracted (obviously mesa and the wayland/X have had to deal with it,
> and the video and camera side is dealing with it more and more).
> Maybe a more established notion of use -> pipeline/device collections,
> is needed as a starting point? Then using Christian's suggestion, one
> could at least enumerate  use -> heap that would be functional. And
> maybe device makers could then supplement explicit optimized mapping
> overrides for their device?
> 
> I just think leaving individual applications (or even individual
> frameworks like mesa) to embed assumptions about heap names ->
> functionality is going to be a problematic approach.

I totally agree on the conclusion, but I still don't see how having a
central component in charge of that will make things better. It just
won't scale to the thousands of devices out there.

And that's great improvements for the future, but heaps have use-cases
today: the CMA heap is the only way to get a physically contiguous
cacheable buffer in userspace at the moment for example.

libcamera uses it for its software ISP implementation for example.

So, while working on improving things in the future is a reasonable
goal, we also need to improve things for the current users right now.
And there's definitely users for it outside of Android.

Which brings us back to the question: What would be a good name? Do we
want to expose a platform specific region name, possibly with a suffix
or prefix to define whether it's cached or not?

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]

Re: [PATCH v2 2/2] dma-buf: heaps: Give default CMA heap a fixed name

[Maxime Ripard]

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

Hi Jared,

On Thu, Apr 24, 2025 at 09:11:24AM -0700, Jared Kangas wrote:
> > >  struct cma_heap {
> > >  	struct dma_heap *heap;
> > > @@ -394,15 +395,26 @@ static int __init __add_cma_heap(struct cma *cma, const char *name)
> > >  static int __init add_default_cma_heap(void)
> > >  {
> > >  	struct cma *default_cma = dev_get_cma_area(NULL);
> > > +	const char *legacy_cma_name;
> > >  	int ret;
> > >  
> > >  	if (!default_cma)
> > >  		return 0;
> > >  
> > > -	ret = __add_cma_heap(default_cma, cma_get_name(default_cma));
> > > +	ret = __add_cma_heap(default_cma, DEFAULT_CMA_NAME);
> > >  	if (ret)
> > >  		return ret;
> > >  
> > > +	legacy_cma_name = cma_get_name(default_cma);
> > > +
> > > +	if (IS_ENABLED(CONFIG_DMABUF_HEAPS_CMA_LEGACY) &&
> > > +	    strcmp(legacy_cma_name, DEFAULT_CMA_NAME)) {
> > > +		ret = __add_cma_heap(default_cma, legacy_cma_name);
> > > +		if (ret)
> > > +			pr_warn("cma_heap: failed to add legacy heap: %pe\n",
> > > +				ERR_PTR(-ret));
> > > +	}
> > > +
> > 
> > It would also simplify this part, since you would always create the legacy heap.
> 
> By "always", do you mean removing the strcmp? I added this to guard
> against cases where the devicetree node's name clashed with the default
> name, given that the DT name isn't necessarily restricted to one of the
> current names in use ("linux,cma" or "default-pool"). It seems like the
> strcmp would be relevant regardless of the naming choice, but if this is
> overly cautious, I can remove it in v3.

That's not overly cautious, that's something I overlooked :)

You're totally right that we should check for that. We should probably
add a more specific error message in that case though

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]