2022-09-12 17:24:25

by Nate Drude

[permalink] [raw]
Subject: [PATCH] dma-contiguous: add optional cma_name for cma= kernel parameter

When cma is defined in the device tree, the device tree node
name is used as the cma name. In the following example, the cma
will be named 'linux,cma':

linux,cma {
compatible = "shared-dma-pool";
reusable;
size = <0 0x3c000000>;
alloc-ranges = <0 0x40000000 0 0xC0000000>;
linux,cma-default;
};

And a device /dev/dma_heap/linux,cma is created.

However, when cma is provided by command line, a default
name of 'reserved' is used, and the device path changes to
/dev/dma_heap/reserved.

This is problematic because some user space applications,
like gstreamer plugins, are expecting /dev/dma_heap/linux,cma.

This parameter allows overriding the default 'reserved' name.

Signed-off-by: Nate Drude <[email protected]>
---
.../admin-guide/kernel-parameters.txt | 7 +++++++
kernel/dma/contiguous.c | 21 ++++++++++++++++++-
2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 51397a320f5e..975ec862d071 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -644,6 +644,13 @@
altogether. For more information, see
kernel/dma/contiguous.c

+ cma_name= Override the cma heap name
+ Format: <string>
+ When passing the cma kernel parameter, the default
+ cma name is 'reserved'. This parameter allows it to
+ be overriden to align with the device tree name,
+ like 'linux,cma'.
+
cma_pernuma=nn[MG]
[ARM64,KNL,CMA]
Sets the size of kernel per-numa memory area for
diff --git a/kernel/dma/contiguous.c b/kernel/dma/contiguous.c
index 3d63d91cba5c..e89819ec183e 100644
--- a/kernel/dma/contiguous.c
+++ b/kernel/dma/contiguous.c
@@ -74,6 +74,7 @@ static const phys_addr_t size_bytes __initconst =
static phys_addr_t size_cmdline __initdata = -1;
static phys_addr_t base_cmdline __initdata;
static phys_addr_t limit_cmdline __initdata;
+static char name_cmdline[CMA_MAX_NAME] = "reserved";

static int __init early_cma(char *p)
{
@@ -96,6 +97,24 @@ static int __init early_cma(char *p)
}
early_param("cma", early_cma);

+static int __init early_cma_name(char *p)
+{
+ if (!p) {
+ pr_err("Config string not provided\n");
+ return -EINVAL;
+ }
+
+ if (!strlen(p)) {
+ pr_err("cma_name must have at least one character\n");
+ return -EINVAL;
+ }
+
+ snprintf(name_cmdline, CMA_MAX_NAME, p);
+
+ return 0;
+}
+early_param("cma_name", early_cma_name);
+
#ifdef CONFIG_DMA_PERNUMA_CMA

static struct cma *dma_contiguous_pernuma_area[MAX_NUMNODES];
@@ -231,7 +250,7 @@ int __init dma_contiguous_reserve_area(phys_addr_t size, phys_addr_t base,
int ret;

ret = cma_declare_contiguous(base, size, limit, 0, 0, fixed,
- "reserved", res_cma);
+ name_cmdline, res_cma);
if (ret)
return ret;

--
2.37.3


2022-09-13 11:24:39

by Robin Murphy

[permalink] [raw]
Subject: Re: [PATCH] dma-contiguous: add optional cma_name for cma= kernel parameter

On 2022-09-12 17:38, Nate Drude wrote:
> When cma is defined in the device tree, the device tree node
> name is used as the cma name. In the following example, the cma
> will be named 'linux,cma':
>
> linux,cma {
> compatible = "shared-dma-pool";
> reusable;
> size = <0 0x3c000000>;
> alloc-ranges = <0 0x40000000 0 0xC0000000>;
> linux,cma-default;
> };
>
> And a device /dev/dma_heap/linux,cma is created.
>
> However, when cma is provided by command line, a default
> name of 'reserved' is used, and the device path changes to
> /dev/dma_heap/reserved.

If userspace expects the CMA heap driver to expose a consistent name for
CMA heaps, shouldn't it be the CMA heap driver's responsibility to
expose a consistent name for CMA heaps? Tinkering with the core CMA code
doesn't feel like the right approach.

Furthermore, given that DT reserved-memory nodes carrying the
"linux-cma-default" property equally can (and do) have different names
as well, that or fixing userspace really are the only robust options.

Thanks,
Robin.

> This is problematic because some user space applications,
> like gstreamer plugins, are expecting /dev/dma_heap/linux,cma.
>
> This parameter allows overriding the default 'reserved' name.
>
> Signed-off-by: Nate Drude <[email protected]>
> ---
> .../admin-guide/kernel-parameters.txt | 7 +++++++
> kernel/dma/contiguous.c | 21 ++++++++++++++++++-
> 2 files changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 51397a320f5e..975ec862d071 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -644,6 +644,13 @@
> altogether. For more information, see
> kernel/dma/contiguous.c
>
> + cma_name= Override the cma heap name
> + Format: <string>
> + When passing the cma kernel parameter, the default
> + cma name is 'reserved'. This parameter allows it to
> + be overriden to align with the device tree name,
> + like 'linux,cma'.
> +
> cma_pernuma=nn[MG]
> [ARM64,KNL,CMA]
> Sets the size of kernel per-numa memory area for
> diff --git a/kernel/dma/contiguous.c b/kernel/dma/contiguous.c
> index 3d63d91cba5c..e89819ec183e 100644
> --- a/kernel/dma/contiguous.c
> +++ b/kernel/dma/contiguous.c
> @@ -74,6 +74,7 @@ static const phys_addr_t size_bytes __initconst =
> static phys_addr_t size_cmdline __initdata = -1;
> static phys_addr_t base_cmdline __initdata;
> static phys_addr_t limit_cmdline __initdata;
> +static char name_cmdline[CMA_MAX_NAME] = "reserved";
>
> static int __init early_cma(char *p)
> {
> @@ -96,6 +97,24 @@ static int __init early_cma(char *p)
> }
> early_param("cma", early_cma);
>
> +static int __init early_cma_name(char *p)
> +{
> + if (!p) {
> + pr_err("Config string not provided\n");
> + return -EINVAL;
> + }
> +
> + if (!strlen(p)) {
> + pr_err("cma_name must have at least one character\n");
> + return -EINVAL;
> + }
> +
> + snprintf(name_cmdline, CMA_MAX_NAME, p);
> +
> + return 0;
> +}
> +early_param("cma_name", early_cma_name);
> +
> #ifdef CONFIG_DMA_PERNUMA_CMA
>
> static struct cma *dma_contiguous_pernuma_area[MAX_NUMNODES];
> @@ -231,7 +250,7 @@ int __init dma_contiguous_reserve_area(phys_addr_t size, phys_addr_t base,
> int ret;
>
> ret = cma_declare_contiguous(base, size, limit, 0, 0, fixed,
> - "reserved", res_cma);
> + name_cmdline, res_cma);
> if (ret)
> return ret;
>

2022-09-23 14:55:46

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH] dma-contiguous: add optional cma_name for cma= kernel parameter

On Tue, Sep 13, 2022 at 11:45:17AM +0100, Robin Murphy wrote:
> If userspace expects the CMA heap driver to expose a consistent name for
> CMA heaps, shouldn't it be the CMA heap driver's responsibility to expose a
> consistent name for CMA heaps? Tinkering with the core CMA code doesn't
> feel like the right approach.

Agreed. In fact I think exposing this name in a uapi seems like a really
bad idea that is asking for a lot of trouble.