[v2,v4l-utils] common: Use posix_memalign for allocating userptr buffers

Message ID 20240622065743.385831-1-fusibrandon13@gmail.com (mailing list archive)
State Superseded
Headers
Series [v2,v4l-utils] common: Use posix_memalign for allocating userptr buffers |

Commit Message

Brandon Cheo Fusi June 22, 2024, 6:57 a.m. UTC
  When dealing with a userptr pointing to a buffer in userspace,
videobuf2 swaps the corresponding physical pages with other pages
so we have a contiguous area of memory for DMA. This only works if
the userptr is page aligned.

The current way of allocating user buffers using malloc only
guarantees alignment up to `alignof(max_align_t)`, which is usually
16. So replace malloc with posix_memalign to ensure the returned
pointer is on a page boundary.

Signed-off-by: Brandon Cheo Fusi <fusibrandon13@gmail.com>
---
 utils/common/v4l-helpers.h | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
  

Comments

Sakari Ailus June 23, 2024, 8:12 a.m. UTC | #1
Hi Brandon,

On Sat, Jun 22, 2024 at 07:57:43AM +0100, Brandon Cheo Fusi wrote:
> When dealing with a userptr pointing to a buffer in userspace,
> videobuf2 swaps the corresponding physical pages with other pages
> so we have a contiguous area of memory for DMA. This only works if
> the userptr is page aligned.
> 
> The current way of allocating user buffers using malloc only
> guarantees alignment up to `alignof(max_align_t)`, which is usually
> 16. So replace malloc with posix_memalign to ensure the returned
> pointer is on a page boundary.
> 
> Signed-off-by: Brandon Cheo Fusi <fusibrandon13@gmail.com>
> ---
>  utils/common/v4l-helpers.h | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/utils/common/v4l-helpers.h b/utils/common/v4l-helpers.h
> index cf0e92d..4104b53 100644
> --- a/utils/common/v4l-helpers.h
> +++ b/utils/common/v4l-helpers.h
> @@ -1656,15 +1656,16 @@ static inline int v4l_queue_alloc_bufs(struct v4l_fd *f,
>  		struct v4l_queue *q, unsigned from)
>  {
>  	unsigned b, p;
> +	void *m;
> +	int ret;
>  
>  	if (q->memory != V4L2_MEMORY_USERPTR)
>  		return 0;
>  	for (b = from; b < v4l_queue_g_buffers(q); b++) {
>  		for (p = 0; p < v4l_queue_g_num_planes(q); p++) {
> -			void *m = malloc(v4l_queue_g_length(q, p));

Please continue declaring m and ret here. They're not used outside the loop
(and should not be).

> -
> -			if (m == NULL)
> -				return errno;
> +			ret = posix_memalign(&m, getpagesize(), v4l_queue_g_length(q, p));

Over 80 characters per line for no apparent reason.

> +			if (ret)
> +				return ret;
>  			v4l_queue_s_userptr(q, b, p, m);
>  		}
>  	}
  

Patch

diff --git a/utils/common/v4l-helpers.h b/utils/common/v4l-helpers.h
index cf0e92d..4104b53 100644
--- a/utils/common/v4l-helpers.h
+++ b/utils/common/v4l-helpers.h
@@ -1656,15 +1656,16 @@  static inline int v4l_queue_alloc_bufs(struct v4l_fd *f,
 		struct v4l_queue *q, unsigned from)
 {
 	unsigned b, p;
+	void *m;
+	int ret;
 
 	if (q->memory != V4L2_MEMORY_USERPTR)
 		return 0;
 	for (b = from; b < v4l_queue_g_buffers(q); b++) {
 		for (p = 0; p < v4l_queue_g_num_planes(q); p++) {
-			void *m = malloc(v4l_queue_g_length(q, p));
-
-			if (m == NULL)
-				return errno;
+			ret = posix_memalign(&m, getpagesize(), v4l_queue_g_length(q, p));
+			if (ret)
+				return ret;
 			v4l_queue_s_userptr(q, b, p, m);
 		}
 	}