[PATCH v2 01/13] udmabuf: sort headers, drop uapi/ path prefix

[PATCH v2 01/13] udmabuf: sort headers, drop uapi/ path prefix

[Gerd Hoffmann]

Reported-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/dma-buf/udmabuf.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
index 2e8502250a..155050c741 100644
--- a/drivers/dma-buf/udmabuf.c
+++ b/drivers/dma-buf/udmabuf.c
@@ -1,17 +1,16 @@
 // SPDX-License-Identifier: GPL-2.0
-#include <linux/init.h>
-#include <linux/module.h>
+#include <linux/cred.h>
 #include <linux/device.h>
-#include <linux/kernel.h>
-#include <linux/slab.h>
-#include <linux/miscdevice.h>
 #include <linux/dma-buf.h>
 #include <linux/highmem.h>
-#include <linux/cred.h>
-#include <linux/shmem_fs.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
 #include <linux/memfd.h>
-
-#include <uapi/linux/udmabuf.h>
+#include <linux/miscdevice.h>
+#include <linux/module.h>
+#include <linux/shmem_fs.h>
+#include <linux/slab.h>
+#include <linux/udmabuf.h>
 
 struct udmabuf {
 	u32 pagecount;
-- 
2.9.3

[PATCH v2 02/13] udmabuf: improve map_udmabuf error handling

[Gerd Hoffmann]

Reported-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/dma-buf/udmabuf.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
index 155050c741..0d03367c57 100644
--- a/drivers/dma-buf/udmabuf.c
+++ b/drivers/dma-buf/udmabuf.c
@@ -51,25 +51,24 @@ static struct sg_table *map_udmabuf(struct dma_buf_attachment *at,
 {
 	struct udmabuf *ubuf = at->dmabuf->priv;
 	struct sg_table *sg;
+	int ret;
 
 	sg = kzalloc(sizeof(*sg), GFP_KERNEL);
 	if (!sg)
-		goto err1;
-	if (sg_alloc_table_from_pages(sg, ubuf->pages, ubuf->pagecount,
-				      0, ubuf->pagecount << PAGE_SHIFT,
-				      GFP_KERNEL) < 0)
-		goto err2;
+		return ERR_PTR(-ENOMEM);
+	ret = sg_alloc_table_from_pages(sg, ubuf->pages, ubuf->pagecount,
+					0, ubuf->pagecount << PAGE_SHIFT,
+					GFP_KERNEL);
+	if (ret < 0)
+		goto err;
 	if (!dma_map_sg(at->dev, sg->sgl, sg->nents, direction))
-		goto err3;
-
+		goto err;
 	return sg;
 
-err3:
+err:
 	sg_free_table(sg);
-err2:
 	kfree(sg);
-err1:
-	return ERR_PTR(-ENOMEM);
+	return ERR_PTR(ret);
 }
 
 static void unmap_udmabuf(struct dma_buf_attachment *at,
-- 
2.9.3

[PATCH v2 03/13] udmabuf: use pgoff_t for pagecount

[Gerd Hoffmann]

Reported-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/dma-buf/udmabuf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
index 0d03367c57..d99a9b59d3 100644
--- a/drivers/dma-buf/udmabuf.c
+++ b/drivers/dma-buf/udmabuf.c
@@ -13,7 +13,7 @@
 #include <linux/udmabuf.h>
 
 struct udmabuf {
-	u32 pagecount;
+	pgoff_t pagecount;
 	struct page **pages;
 };
 
-- 
2.9.3

[PATCH v2 04/13] udmabuf: constify udmabuf_ops

[Gerd Hoffmann]

Reported-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/dma-buf/udmabuf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
index d99a9b59d3..e3560e840d 100644
--- a/drivers/dma-buf/udmabuf.c
+++ b/drivers/dma-buf/udmabuf.c
@@ -104,7 +104,7 @@ static void kunmap_udmabuf(struct dma_buf *buf, unsigned long page_num,
 	kunmap(vaddr);
 }
 
-static struct dma_buf_ops udmabuf_ops = {
+static const struct dma_buf_ops udmabuf_ops = {
 	.map_dma_buf	  = map_udmabuf,
 	.unmap_dma_buf	  = unmap_udmabuf,
 	.release	  = release_udmabuf,
-- 
2.9.3

[PATCH v2 05/13] udmabuf: constify udmabuf_create args

[Gerd Hoffmann]

Reported-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/dma-buf/udmabuf.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
index e3560e840d..4167da8141 100644
--- a/drivers/dma-buf/udmabuf.c
+++ b/drivers/dma-buf/udmabuf.c
@@ -116,8 +116,8 @@ static const struct dma_buf_ops udmabuf_ops = {
 #define SEALS_WANTED (F_SEAL_SHRINK)
 #define SEALS_DENIED (F_SEAL_WRITE)
 
-static long udmabuf_create(struct udmabuf_create_list *head,
-			   struct udmabuf_create_item *list)
+static long udmabuf_create(const struct udmabuf_create_list *head,
+			   const const struct udmabuf_create_item *list)
 {
 	DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
 	struct file *memfd = NULL;
-- 
2.9.3

Re: [PATCH v2 05/13] udmabuf: constify udmabuf_create args

[Laurent Pinchart]

Hi Gerd,

Thank you for the patch.

On Tuesday, 11 September 2018 16:42:08 EEST Gerd Hoffmann wrote:
> Reported-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/dma-buf/udmabuf.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
> index e3560e840d..4167da8141 100644
> --- a/drivers/dma-buf/udmabuf.c
> +++ b/drivers/dma-buf/udmabuf.c
> @@ -116,8 +116,8 @@ static const struct dma_buf_ops udmabuf_ops = {
>  #define SEALS_WANTED (F_SEAL_SHRINK)
>  #define SEALS_DENIED (F_SEAL_WRITE)
> 
> -static long udmabuf_create(struct udmabuf_create_list *head,
> -			   struct udmabuf_create_item *list)
> +static long udmabuf_create(const struct udmabuf_create_list *head,
> +			   const const struct udmabuf_create_item *list)

Even if you really want to make it const, a single const should suffice :-)

>  {
>  	DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
>  	struct file *memfd = NULL;


-- 
Regards,

Laurent Pinchart

[PATCH v2 06/13] udmabuf: add MEMFD_CREATE dependency

[Gerd Hoffmann]

udmabuf builds without it, but if userspace can not create memfd
handles in the first place it is rather pointless to include it,
except for test builds.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/dma-buf/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/dma-buf/Kconfig b/drivers/dma-buf/Kconfig
index 338129eb12..2e5a0faa2c 100644
--- a/drivers/dma-buf/Kconfig
+++ b/drivers/dma-buf/Kconfig
@@ -34,6 +34,7 @@ config UDMABUF
 	bool "userspace dmabuf misc driver"
 	default n
 	depends on DMA_SHARED_BUFFER
+	depends on MEMFD_CREATE || COMPILE_TEST
 	help
 	  A driver to let userspace turn memfd regions into dma-bufs.
 	  Qemu can use this to create host dmabufs for guest framebuffers.
-- 
2.9.3

[PATCH v2 07/13] udmabuf: rework limits

[Gerd Hoffmann]

Create variable for the list length limit.  Serves as documentation,
also allows to make it a module parameter if needed.

Also add a total size limit.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/dma-buf/udmabuf.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
index 4167da8141..a01fcf6a66 100644
--- a/drivers/dma-buf/udmabuf.c
+++ b/drivers/dma-buf/udmabuf.c
@@ -12,6 +12,9 @@
 #include <linux/slab.h>
 #include <linux/udmabuf.h>
 
+static const u32    list_limit = 1024;  /* udmabuf_create_list->count limit */
+static const size_t size_limit_mb = 64; /* total dmabuf size, in megabytes  */
+
 struct udmabuf {
 	pgoff_t pagecount;
 	struct page **pages;
@@ -123,7 +126,7 @@ static long udmabuf_create(const struct udmabuf_create_list *head,
 	struct file *memfd = NULL;
 	struct udmabuf *ubuf;
 	struct dma_buf *buf;
-	pgoff_t pgoff, pgcnt, pgidx, pgbuf;
+	pgoff_t pgoff, pgcnt, pgidx, pgbuf, pglimit;
 	struct page *page;
 	int seals, ret = -EINVAL;
 	u32 i, flags;
@@ -132,12 +135,15 @@ static long udmabuf_create(const struct udmabuf_create_list *head,
 	if (!ubuf)
 		return -ENOMEM;
 
+	pglimit = (size_limit_mb * 1024 * 1024) >> PAGE_SHIFT;
 	for (i = 0; i < head->count; i++) {
 		if (!IS_ALIGNED(list[i].offset, PAGE_SIZE))
 			goto err_free_ubuf;
 		if (!IS_ALIGNED(list[i].size, PAGE_SIZE))
 			goto err_free_ubuf;
 		ubuf->pagecount += list[i].size >> PAGE_SHIFT;
+		if (ubuf->pagecount > pglimit)
+			goto err_free_ubuf;
 	}
 	ubuf->pages = kmalloc_array(ubuf->pagecount, sizeof(struct page *),
 				    GFP_KERNEL);
@@ -227,7 +233,7 @@ static long udmabuf_ioctl_create_list(struct file *filp, unsigned long arg)
 
 	if (copy_from_user(&head, (void __user *)arg, sizeof(head)))
 		return -EFAULT;
-	if (head.count > 1024)
+	if (head.count > list_limit)
 		return -EINVAL;
 	lsize = sizeof(struct udmabuf_create_item) * head.count;
 	list = memdup_user((void __user *)(arg + sizeof(head)), lsize);
-- 
2.9.3

[PATCH v2 08/13] udmabuf: improve udmabuf_create error handling

[Gerd Hoffmann]

Reported-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/dma-buf/udmabuf.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
index a01fcf6a66..e821361566 100644
--- a/drivers/dma-buf/udmabuf.c
+++ b/drivers/dma-buf/udmabuf.c
@@ -126,7 +126,7 @@ static long udmabuf_create(const struct udmabuf_create_list *head,
 	struct file *memfd = NULL;
 	struct udmabuf *ubuf;
 	struct dma_buf *buf;
-	pgoff_t pgoff, pgcnt, pgidx, pgbuf, pglimit;
+	pgoff_t pgoff, pgcnt, pgidx, pgbuf = 0, pglimit;
 	struct page *page;
 	int seals, ret = -EINVAL;
 	u32 i, flags;
@@ -138,32 +138,32 @@ static long udmabuf_create(const struct udmabuf_create_list *head,
 	pglimit = (size_limit_mb * 1024 * 1024) >> PAGE_SHIFT;
 	for (i = 0; i < head->count; i++) {
 		if (!IS_ALIGNED(list[i].offset, PAGE_SIZE))
-			goto err_free_ubuf;
+			goto err;
 		if (!IS_ALIGNED(list[i].size, PAGE_SIZE))
-			goto err_free_ubuf;
+			goto err;
 		ubuf->pagecount += list[i].size >> PAGE_SHIFT;
 		if (ubuf->pagecount > pglimit)
-			goto err_free_ubuf;
+			goto err;
 	}
 	ubuf->pages = kmalloc_array(ubuf->pagecount, sizeof(struct page *),
 				    GFP_KERNEL);
 	if (!ubuf->pages) {
 		ret = -ENOMEM;
-		goto err_free_ubuf;
+		goto err;
 	}
 
 	pgbuf = 0;
 	for (i = 0; i < head->count; i++) {
 		memfd = fget(list[i].memfd);
 		if (!memfd)
-			goto err_put_pages;
+			goto err;
 		if (!shmem_mapping(file_inode(memfd)->i_mapping))
-			goto err_put_pages;
+			goto err;
 		seals = memfd_fcntl(memfd, F_GET_SEALS, 0);
 		if (seals == -EINVAL ||
 		    (seals & SEALS_WANTED) != SEALS_WANTED ||
 		    (seals & SEALS_DENIED) != 0)
-			goto err_put_pages;
+			goto err;
 		pgoff = list[i].offset >> PAGE_SHIFT;
 		pgcnt = list[i].size   >> PAGE_SHIFT;
 		for (pgidx = 0; pgidx < pgcnt; pgidx++) {
@@ -171,13 +171,13 @@ static long udmabuf_create(const struct udmabuf_create_list *head,
 				file_inode(memfd)->i_mapping, pgoff + pgidx);
 			if (IS_ERR(page)) {
 				ret = PTR_ERR(page);
-				goto err_put_pages;
+				goto err;
 			}
 			ubuf->pages[pgbuf++] = page;
 		}
 		fput(memfd);
+		memfd = NULL;
 	}
-	memfd = NULL;
 
 	exp_info.ops  = &udmabuf_ops;
 	exp_info.size = ubuf->pagecount << PAGE_SHIFT;
@@ -186,7 +186,7 @@ static long udmabuf_create(const struct udmabuf_create_list *head,
 	buf = dma_buf_export(&exp_info);
 	if (IS_ERR(buf)) {
 		ret = PTR_ERR(buf);
-		goto err_put_pages;
+		goto err;
 	}
 
 	flags = 0;
@@ -194,10 +194,9 @@ static long udmabuf_create(const struct udmabuf_create_list *head,
 		flags |= O_CLOEXEC;
 	return dma_buf_fd(buf, flags);
 
-err_put_pages:
+err:
 	while (pgbuf > 0)
 		put_page(ubuf->pages[--pgbuf]);
-err_free_ubuf:
 	if (memfd)
 		fput(memfd);
 	kfree(ubuf->pages);
-- 
2.9.3

[PATCH v2 09/13] udmabuf: use EBADFD in case we didn't got a memfd

[Gerd Hoffmann]

Reported-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/dma-buf/udmabuf.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
index e821361566..652707f993 100644
--- a/drivers/dma-buf/udmabuf.c
+++ b/drivers/dma-buf/udmabuf.c
@@ -154,14 +154,17 @@ static long udmabuf_create(const struct udmabuf_create_list *head,
 
 	pgbuf = 0;
 	for (i = 0; i < head->count; i++) {
+		ret = -EBADFD;
 		memfd = fget(list[i].memfd);
 		if (!memfd)
 			goto err;
 		if (!shmem_mapping(file_inode(memfd)->i_mapping))
 			goto err;
 		seals = memfd_fcntl(memfd, F_GET_SEALS, 0);
-		if (seals == -EINVAL ||
-		    (seals & SEALS_WANTED) != SEALS_WANTED ||
+		if (seals == -EINVAL)
+			goto err;
+		ret = -EINVAL;
+		if ((seals & SEALS_WANTED) != SEALS_WANTED ||
 		    (seals & SEALS_DENIED) != 0)
 			goto err;
 		pgoff = list[i].offset >> PAGE_SHIFT;
-- 
2.9.3

[PATCH v2 10/13] udmabuf: use ENOTTY for invalid ioctls

[Gerd Hoffmann]

Reported-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/dma-buf/udmabuf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
index 652707f993..b637a8be6d 100644
--- a/drivers/dma-buf/udmabuf.c
+++ b/drivers/dma-buf/udmabuf.c
@@ -260,7 +260,7 @@ static long udmabuf_ioctl(struct file *filp, unsigned int ioctl,
 		ret = udmabuf_ioctl_create_list(filp, arg);
 		break;
 	default:
-		ret = -EINVAL;
+		ret = -ENOTTY;
 		break;
 	}
 	return ret;
-- 
2.9.3

[PATCH v2 11/13] udmabuf: drop WARN_ON() check.

[Gerd Hoffmann]

Reported-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/dma-buf/udmabuf.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
index b637a8be6d..7a4fd2194d 100644
--- a/drivers/dma-buf/udmabuf.c
+++ b/drivers/dma-buf/udmabuf.c
@@ -25,9 +25,6 @@ static int udmabuf_vm_fault(struct vm_fault *vmf)
 	struct vm_area_struct *vma = vmf->vma;
 	struct udmabuf *ubuf = vma->vm_private_data;
 
-	if (WARN_ON(vmf->pgoff >= ubuf->pagecount))
-		return VM_FAULT_SIGBUS;
-
 	vmf->page = ubuf->pages[vmf->pgoff];
 	get_page(vmf->page);
 	return 0;
-- 
2.9.3

Re: [PATCH v2 11/13] udmabuf: drop WARN_ON() check.

[Laurent Pinchart]

Hi Gerd,

Thank you for the patch.

On Tuesday, 11 September 2018 16:42:14 EEST Gerd Hoffmann wrote:

Still no commit message ? :-)

> Reported-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/dma-buf/udmabuf.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
> index b637a8be6d..7a4fd2194d 100644
> --- a/drivers/dma-buf/udmabuf.c
> +++ b/drivers/dma-buf/udmabuf.c
> @@ -25,9 +25,6 @@ static int udmabuf_vm_fault(struct vm_fault *vmf)
>  	struct vm_area_struct *vma = vmf->vma;
>  	struct udmabuf *ubuf = vma->vm_private_data;
> 
> -	if (WARN_ON(vmf->pgoff >= ubuf->pagecount))
> -		return VM_FAULT_SIGBUS;
> -
>  	vmf->page = ubuf->pages[vmf->pgoff];
>  	get_page(vmf->page);
>  	return 0;


-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 11/13] udmabuf: drop WARN_ON() check.

[Gerd Hoffmann]

On Tue, Sep 11, 2018 at 06:07:10PM +0300, Laurent Pinchart wrote:
> Hi Gerd,
> 
> Thank you for the patch.
> 
> On Tuesday, 11 September 2018 16:42:14 EEST Gerd Hoffmann wrote:
> 
> Still no commit message ? :-)

Well, there isn't much to explain about that one ...

cheers,
  Gerd

[PATCH v2 12/13] udmabuf: use sizeof(variable) instead of sizeof(type)

[Gerd Hoffmann]

Reported-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 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 7a4fd2194d..92af9b5300 100644
--- a/drivers/dma-buf/udmabuf.c
+++ b/drivers/dma-buf/udmabuf.c
@@ -128,7 +128,7 @@ static long udmabuf_create(const struct udmabuf_create_list *head,
 	int seals, ret = -EINVAL;
 	u32 i, flags;
 
-	ubuf = kzalloc(sizeof(struct udmabuf), GFP_KERNEL);
+	ubuf = kzalloc(sizeof(*ubuf), GFP_KERNEL);
 	if (!ubuf)
 		return -ENOMEM;
 
@@ -142,7 +142,7 @@ static long udmabuf_create(const struct udmabuf_create_list *head,
 		if (ubuf->pagecount > pglimit)
 			goto err;
 	}
-	ubuf->pages = kmalloc_array(ubuf->pagecount, sizeof(struct page *),
+	ubuf->pages = kmalloc_array(ubuf->pagecount, sizeof(*ubuf->pages),
 				    GFP_KERNEL);
 	if (!ubuf->pages) {
 		ret = -ENOMEM;
@@ -211,7 +211,7 @@ static long udmabuf_ioctl_create(struct file *filp, unsigned long arg)
 	struct udmabuf_create_item list;
 
 	if (copy_from_user(&create, (void __user *)arg,
-			   sizeof(struct udmabuf_create)))
+			   sizeof(create)))
 		return -EFAULT;
 
 	head.flags  = create.flags;
-- 
2.9.3

Re: [PATCH v2 12/13] udmabuf: use sizeof(variable) instead of sizeof(type)

[Laurent Pinchart]

Hi Gerd,

Thank you for the patch.

On Tuesday, 11 September 2018 16:42:15 EEST Gerd Hoffmann wrote:
> Reported-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  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 7a4fd2194d..92af9b5300 100644
> --- a/drivers/dma-buf/udmabuf.c
> +++ b/drivers/dma-buf/udmabuf.c
> @@ -128,7 +128,7 @@ static long udmabuf_create(const struct
> udmabuf_create_list *head, int seals, ret = -EINVAL;
>  	u32 i, flags;
> 
> -	ubuf = kzalloc(sizeof(struct udmabuf), GFP_KERNEL);
> +	ubuf = kzalloc(sizeof(*ubuf), GFP_KERNEL);
>  	if (!ubuf)
>  		return -ENOMEM;
> 
> @@ -142,7 +142,7 @@ static long udmabuf_create(const struct
> udmabuf_create_list *head, if (ubuf->pagecount > pglimit)
>  			goto err;
>  	}
> -	ubuf->pages = kmalloc_array(ubuf->pagecount, sizeof(struct page *),
> +	ubuf->pages = kmalloc_array(ubuf->pagecount, sizeof(*ubuf->pages),
>  				    GFP_KERNEL);
>  	if (!ubuf->pages) {
>  		ret = -ENOMEM;
> @@ -211,7 +211,7 @@ static long udmabuf_ioctl_create(struct file *filp,
> unsigned long arg) struct udmabuf_create_item list;
> 
>  	if (copy_from_user(&create, (void __user *)arg,
> -			   sizeof(struct udmabuf_create)))
> +			   sizeof(create)))
>  		return -EFAULT;
> 
>  	head.flags  = create.flags;

-- 
Regards,

Laurent Pinchart

[PATCH v2 13/13] udmabuf: add documentation

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/uapi/linux/udmabuf.h         | 50 +++++++++++++++++++++++++++++++++---
 Documentation/driver-api/dma-buf.rst |  8 ++++++
 2 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/include/uapi/linux/udmabuf.h b/include/uapi/linux/udmabuf.h
index 46b6532ed8..f30b37cb5c 100644
--- a/include/uapi/linux/udmabuf.h
+++ b/include/uapi/linux/udmabuf.h
@@ -5,8 +5,38 @@
 #include <linux/types.h>
 #include <linux/ioctl.h>
 
+/**
+ * DOC: udmabuf
+ *
+ * udmabuf is a device driver which allows userspace create dmabufs.
+ * The memory used for these dmabufs must be backed by memfd.  The
+ * memfd must have F_SEAL_SHRINK and it must not have F_SEAL_WRITE.
+ *
+ * The driver has two ioctls, one to create a dmabuf from a single
+ * memory block and one to create a dmabuf from a list of memory
+ * blocks.
+ *
+ * UDMABUF_CREATE - _IOW('u', 0x42, udmabuf_create)
+ *
+ * UDMABUF_CREATE_LIST - _IOW('u', 0x43, udmabuf_create_list)
+ */
+
+#define UDMABUF_CREATE       _IOW('u', 0x42, struct udmabuf_create)
+#define UDMABUF_CREATE_LIST  _IOW('u', 0x43, struct udmabuf_create_list)
+
 #define UDMABUF_FLAGS_CLOEXEC	0x01
 
+/**
+ * struct udmabuf_create - create a dmabuf from a single memory block.
+ *
+ * @memfd: The file handle.
+ * @offset: Start of the buffer (from memfd start).
+ * Must be page aligned.
+ * @size: Size of the buffer.  Must be rounded to page size.
+ *
+ * flags:
+ * UDMABUF_FLAGS_CLOEXEC: set CLOEXEC flag for the dmabuf.
+ */
 struct udmabuf_create {
 	__u32 memfd;
 	__u32 flags;
@@ -14,6 +44,14 @@ struct udmabuf_create {
 	__u64 size;
 };
 
+/**
+ * struct udmabuf_create_item - one memory block list item.
+ *
+ * @memfd: The file handle.
+ * @offset: Start of the buffer (from memfd start).
+ * Must be page aligned.
+ * @size: Size of the buffer.  Must be rounded to page size.
+ */
 struct udmabuf_create_item {
 	__u32 memfd;
 	__u32 __pad;
@@ -21,13 +59,19 @@ struct udmabuf_create_item {
 	__u64 size;
 };
 
+/**
+ * struct udmabuf_create_list - create a dmabuf from a memory block list.
+ *
+ * @count: The number of list elements.
+ * @list: The memory block list
+ *
+ * flags:
+ * UDMABUF_FLAGS_CLOEXEC: set CLOEXEC flag for the dmabuf.
+ */
 struct udmabuf_create_list {
 	__u32 flags;
 	__u32 count;
 	struct udmabuf_create_item list[];
 };
 
-#define UDMABUF_CREATE       _IOW('u', 0x42, struct udmabuf_create)
-#define UDMABUF_CREATE_LIST  _IOW('u', 0x43, struct udmabuf_create_list)
-
 #endif /* _UAPI_LINUX_UDMABUF_H */
diff --git a/Documentation/driver-api/dma-buf.rst b/Documentation/driver-api/dma-buf.rst
index b541e97c7a..1f62c30a14 100644
--- a/Documentation/driver-api/dma-buf.rst
+++ b/Documentation/driver-api/dma-buf.rst
@@ -166,3 +166,11 @@ DMA Fence uABI/Sync File
 .. kernel-doc:: include/linux/sync_file.h
    :internal:
 
+Userspace DMA Buffer driver
+~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+.. kernel-doc:: include/uapi/linux/udmabuf.h
+   :doc: udmabuf
+
+.. kernel-doc:: include/uapi/linux/udmabuf.h
+   :internal:
-- 
2.9.3

Re: [PATCH v2 13/13] udmabuf: add documentation

[Randy Dunlap]

On 9/11/18 6:42 AM, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  include/uapi/linux/udmabuf.h         | 50 +++++++++++++++++++++++++++++++++---
>  Documentation/driver-api/dma-buf.rst |  8 ++++++
>  2 files changed, 55 insertions(+), 3 deletions(-)
> 
> diff --git a/include/uapi/linux/udmabuf.h b/include/uapi/linux/udmabuf.h
> index 46b6532ed8..f30b37cb5c 100644
> --- a/include/uapi/linux/udmabuf.h
> +++ b/include/uapi/linux/udmabuf.h
> @@ -5,8 +5,38 @@
>  #include <linux/types.h>
>  #include <linux/ioctl.h>
>  
> +/**
> + * DOC: udmabuf
> + *
> + * udmabuf is a device driver which allows userspace create dmabufs.

                                                        to create

> + * The memory used for these dmabufs must be backed by memfd.  The
> + * memfd must have F_SEAL_SHRINK and it must not have F_SEAL_WRITE.
> + *
> + * The driver has two ioctls, one to create a dmabuf from a single
> + * memory block and one to create a dmabuf from a list of memory
> + * blocks.
> + *
> + * UDMABUF_CREATE - _IOW('u', 0x42, udmabuf_create)
> + *
> + * UDMABUF_CREATE_LIST - _IOW('u', 0x43, udmabuf_create_list)
> + */
> +
> +#define UDMABUF_CREATE       _IOW('u', 0x42, struct udmabuf_create)
> +#define UDMABUF_CREATE_LIST  _IOW('u', 0x43, struct udmabuf_create_list)
> +
>  #define UDMABUF_FLAGS_CLOEXEC	0x01
>  
> +/**
> + * struct udmabuf_create - create a dmabuf from a single memory block.
> + *
> + * @memfd: The file handle.
> + * @offset: Start of the buffer (from memfd start).
> + * Must be page aligned.
> + * @size: Size of the buffer.  Must be rounded to page size.

      @flags: ???

> + *
> + * flags:
> + * UDMABUF_FLAGS_CLOEXEC: set CLOEXEC flag for the dmabuf.
> + */
>  struct udmabuf_create {
>  	__u32 memfd;
>  	__u32 flags;
> @@ -14,6 +44,14 @@ struct udmabuf_create {
>  	__u64 size;
>  };
>  
> +/**
> + * struct udmabuf_create_item - one memory block list item.
> + *
> + * @memfd: The file handle.
> + * @offset: Start of the buffer (from memfd start).
> + * Must be page aligned.
> + * @size: Size of the buffer.  Must be rounded to page size.
> + */
>  struct udmabuf_create_item {
>  	__u32 memfd;
>  	__u32 __pad;
> @@ -21,13 +59,19 @@ struct udmabuf_create_item {
>  	__u64 size;
>  };
>  
> +/**
> + * struct udmabuf_create_list - create a dmabuf from a memory block list.
> + *
> + * @count: The number of list elements.
> + * @list: The memory block list
> + *
> + * flags:

      @flags:

> + * UDMABUF_FLAGS_CLOEXEC: set CLOEXEC flag for the dmabuf.
> + */
>  struct udmabuf_create_list {
>  	__u32 flags;
>  	__u32 count;
>  	struct udmabuf_create_item list[];
>  };


thanks.
-- 
~Randy