RE: [PATCH v1] dma-buf/udmabuf: Disable the size limit by default

[Kasireddy, Vivek]

Hi Robert,

> Subject: [PATCH v1] dma-buf/udmabuf: Disable the size limit by default
> 
> As udmabuf increasingly enjoys popularity - being used in projects like
> libcamera, Gstreamer, Mesa, KWin and Weston - users more frequently
> encounter cases where the current default size limit of 64MB is too low.
> Examples include allocating video buffers at a 8K resolution - and even 4K
> is affected when using non-subsampled video formats and high bit depths.
> 
> In its current form the size limit for individual buffers does not seem to
> provide any additional level of protection - such as limiting the amount of
> memory a process can pin - as the later can just allocate multiple buffers.
> If additional guardrails are desired, they would likely require some kind
> accounting not limited to individual buffers.
> 
> Therefor let's disable the size limit by default. Use the special value
> of zero to do so, which prevously could be used to effectively disable the
> interface. Using other means, such as file permissions, appears to be a
> much better fit for that purpose.
> 
> Signed-off-by: Robert Mader <robert.mader@xxxxxxxxxxxxx>
> 
> ---
> 
> Please let me know if changing the meaning of the parameter value of zero
> is considered a breaking change / not acceptable. In that case INT_MAX
> might be a better option.
Yeah, I think using INT_MAX might be better.

Thanks,
Vivek

> 
> See
> https://lore.kernel.org/dri-devel/20260711144814.8205-1-
> robert.mader@xxxxxxxxxxxxx/
> for a previous attempt to make the value configurable via kconfig - and
> in particular
> https://lore.kernel.org/dri-devel/6764ca6f-b4d8-4baa-9d27-
> 2ca867ac2d41@xxxxxxx/
> for the suggestion and discussion to remove the default limit.
> ---
>  drivers/dma-buf/udmabuf.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
> index bced421c0d65..3509b690d8e2 100644
> --- a/drivers/dma-buf/udmabuf.c
> +++ b/drivers/dma-buf/udmabuf.c
> @@ -20,9 +20,9 @@ static int list_limit = 1024;
>  module_param(list_limit, int, 0644);
>  MODULE_PARM_DESC(list_limit, "udmabuf_create_list->count limit.
> Default is 1024.");
> 
> -static int size_limit_mb = 64;
> +static int size_limit_mb = 0;
>  module_param(size_limit_mb, int, 0644);
> -MODULE_PARM_DESC(size_limit_mb, "Max size of a dmabuf, in
> megabytes. Default is 64.");
> +MODULE_PARM_DESC(size_limit_mb, "Max size of a dmabuf, in
> megabytes. Setting 0 disables the limit. Default is 0.");
> 
>  struct udmabuf {
>  	pgoff_t pagecount;
> @@ -373,7 +373,7 @@ static long udmabuf_create(struct miscdevice
> *device,
> 
>  		subpgcnt = list[i].size >> PAGE_SHIFT;
>  		pgcnt += subpgcnt;
> -		if (pgcnt > pglimit)
> +		if (pglimit && pglimit < pgcnt)
>  			goto err_noinit;
> 
>  		max_nr_folios = max_t(unsigned long, subpgcnt,
> max_nr_folios);
> --
> 2.55.0