* [PATCH] dma-buf: remove restriction of IOCTL:DMA_BUF_SET_NAME
@ 2021-10-09 2:47 ` guangming.cao
0 siblings, 0 replies; 35+ messages in thread
From: guangming.cao @ 2021-10-09 2:47 UTC (permalink / raw)
To: Sumit Semwal, Christian König, Matthias Brugger,
open list:DMA BUFFER SHARING FRAMEWORK,
open list:DMA BUFFER SHARING FRAMEWORK,
moderated list:DMA BUFFER SHARING FRAMEWORK, open list,
moderated list:ARM/Mediatek SoC support,
moderated list:ARM/Mediatek SoC support
Cc: wsd_upstream, Guangming Cao
From: Guangming Cao <Guangming.Cao@mediatek.com>
If dma-buf don't want userspace users to touch the dmabuf buffer,
it seems we should add this restriction into dma_buf_ops.mmap,
not in this IOCTL:DMA_BUF_SET_NAME.
With this restriction, we can only know the kernel users of the dmabuf
by attachments.
However, for many userspace users, such as userpsace users of dma_heap,
they also need to mark the usage of dma-buf, and they don't care about
who attached to this dmabuf, and seems it's no meaning to waitting for
IOCTL:DMA_BUF_SET_NAME rather than mmap.
Signed-off-by: Guangming Cao <Guangming.Cao@mediatek.com>
---
drivers/dma-buf/dma-buf.c | 14 ++------------
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 511fe0d217a0..afbd0a226639 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -325,10 +325,8 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll)
/**
* dma_buf_set_name - Set a name to a specific dma_buf to track the usage.
- * The name of the dma-buf buffer can only be set when the dma-buf is not
- * attached to any devices. It could theoritically support changing the
- * name of the dma-buf if the same piece of memory is used for multiple
- * purpose between different devices.
+ * It could theoritically support changing the name of the dma-buf if the same
+ * piece of memory is used for multiple purpose between different devices.
*
* @dmabuf: [in] dmabuf buffer that will be renamed.
* @buf: [in] A piece of userspace memory that contains the name of
@@ -346,19 +344,11 @@ static long dma_buf_set_name(struct dma_buf *dmabuf, const char __user *buf)
if (IS_ERR(name))
return PTR_ERR(name);
- dma_resv_lock(dmabuf->resv, NULL);
- if (!list_empty(&dmabuf->attachments)) {
- ret = -EBUSY;
- kfree(name);
- goto out_unlock;
- }
spin_lock(&dmabuf->name_lock);
kfree(dmabuf->name);
dmabuf->name = name;
spin_unlock(&dmabuf->name_lock);
-out_unlock:
- dma_resv_unlock(dmabuf->resv);
return ret;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH] dma-buf: remove restriction of IOCTL:DMA_BUF_SET_NAME
@ 2021-10-09 2:47 ` guangming.cao
0 siblings, 0 replies; 35+ messages in thread
From: guangming.cao @ 2021-10-09 2:47 UTC (permalink / raw)
To: Sumit Semwal, Christian König, Matthias Brugger,
open list:DMA BUFFER SHARING FRAMEWORK,
open list:DMA BUFFER SHARING FRAMEWORK,
moderated list:DMA BUFFER SHARING FRAMEWORK, open list,
moderated list:ARM/Mediatek SoC support,
moderated list:ARM/Mediatek SoC support
Cc: wsd_upstream, Guangming Cao
From: Guangming Cao <Guangming.Cao@mediatek.com>
If dma-buf don't want userspace users to touch the dmabuf buffer,
it seems we should add this restriction into dma_buf_ops.mmap,
not in this IOCTL:DMA_BUF_SET_NAME.
With this restriction, we can only know the kernel users of the dmabuf
by attachments.
However, for many userspace users, such as userpsace users of dma_heap,
they also need to mark the usage of dma-buf, and they don't care about
who attached to this dmabuf, and seems it's no meaning to waitting for
IOCTL:DMA_BUF_SET_NAME rather than mmap.
Signed-off-by: Guangming Cao <Guangming.Cao@mediatek.com>
---
drivers/dma-buf/dma-buf.c | 14 ++------------
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 511fe0d217a0..afbd0a226639 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -325,10 +325,8 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll)
/**
* dma_buf_set_name - Set a name to a specific dma_buf to track the usage.
- * The name of the dma-buf buffer can only be set when the dma-buf is not
- * attached to any devices. It could theoritically support changing the
- * name of the dma-buf if the same piece of memory is used for multiple
- * purpose between different devices.
+ * It could theoritically support changing the name of the dma-buf if the same
+ * piece of memory is used for multiple purpose between different devices.
*
* @dmabuf: [in] dmabuf buffer that will be renamed.
* @buf: [in] A piece of userspace memory that contains the name of
@@ -346,19 +344,11 @@ static long dma_buf_set_name(struct dma_buf *dmabuf, const char __user *buf)
if (IS_ERR(name))
return PTR_ERR(name);
- dma_resv_lock(dmabuf->resv, NULL);
- if (!list_empty(&dmabuf->attachments)) {
- ret = -EBUSY;
- kfree(name);
- goto out_unlock;
- }
spin_lock(&dmabuf->name_lock);
kfree(dmabuf->name);
dmabuf->name = name;
spin_unlock(&dmabuf->name_lock);
-out_unlock:
- dma_resv_unlock(dmabuf->resv);
return ret;
}
--
2.17.1
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH] dma-buf: remove restriction of IOCTL:DMA_BUF_SET_NAME
@ 2021-10-09 2:47 ` guangming.cao
0 siblings, 0 replies; 35+ messages in thread
From: guangming.cao @ 2021-10-09 2:47 UTC (permalink / raw)
To: Sumit Semwal, Christian König, Matthias Brugger,
open list:DMA BUFFER SHARING FRAMEWORK,
open list:DMA BUFFER SHARING FRAMEWORK,
moderated list:DMA BUFFER SHARING FRAMEWORK, open list,
moderated list:ARM/Mediatek SoC support,
moderated list:ARM/Mediatek SoC support
Cc: wsd_upstream, Guangming Cao
From: Guangming Cao <Guangming.Cao@mediatek.com>
If dma-buf don't want userspace users to touch the dmabuf buffer,
it seems we should add this restriction into dma_buf_ops.mmap,
not in this IOCTL:DMA_BUF_SET_NAME.
With this restriction, we can only know the kernel users of the dmabuf
by attachments.
However, for many userspace users, such as userpsace users of dma_heap,
they also need to mark the usage of dma-buf, and they don't care about
who attached to this dmabuf, and seems it's no meaning to waitting for
IOCTL:DMA_BUF_SET_NAME rather than mmap.
Signed-off-by: Guangming Cao <Guangming.Cao@mediatek.com>
---
drivers/dma-buf/dma-buf.c | 14 ++------------
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 511fe0d217a0..afbd0a226639 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -325,10 +325,8 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll)
/**
* dma_buf_set_name - Set a name to a specific dma_buf to track the usage.
- * The name of the dma-buf buffer can only be set when the dma-buf is not
- * attached to any devices. It could theoritically support changing the
- * name of the dma-buf if the same piece of memory is used for multiple
- * purpose between different devices.
+ * It could theoritically support changing the name of the dma-buf if the same
+ * piece of memory is used for multiple purpose between different devices.
*
* @dmabuf: [in] dmabuf buffer that will be renamed.
* @buf: [in] A piece of userspace memory that contains the name of
@@ -346,19 +344,11 @@ static long dma_buf_set_name(struct dma_buf *dmabuf, const char __user *buf)
if (IS_ERR(name))
return PTR_ERR(name);
- dma_resv_lock(dmabuf->resv, NULL);
- if (!list_empty(&dmabuf->attachments)) {
- ret = -EBUSY;
- kfree(name);
- goto out_unlock;
- }
spin_lock(&dmabuf->name_lock);
kfree(dmabuf->name);
dmabuf->name = name;
spin_unlock(&dmabuf->name_lock);
-out_unlock:
- dma_resv_unlock(dmabuf->resv);
return ret;
}
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH] dma-buf: remove restriction of IOCTL:DMA_BUF_SET_NAME
2021-10-09 2:47 ` guangming.cao
(?)
@ 2021-10-09 4:41 ` Randy Dunlap
-1 siblings, 0 replies; 35+ messages in thread
From: Randy Dunlap @ 2021-10-09 4:41 UTC (permalink / raw)
To: guangming.cao, Sumit Semwal, Christian König,
Matthias Brugger, open list:DMA BUFFER SHARING FRAMEWORK,
open list:DMA BUFFER SHARING FRAMEWORK,
moderated list:DMA BUFFER SHARING FRAMEWORK, open list,
moderated list:ARM/Mediatek SoC support,
moderated list:ARM/Mediatek SoC support
Cc: wsd_upstream
On 10/8/21 7:47 PM, guangming.cao@mediatek.com wrote:
> From: Guangming Cao <Guangming.Cao@mediatek.com>
>
> If dma-buf don't want userspace users to touch the dmabuf buffer,
> it seems we should add this restriction into dma_buf_ops.mmap,
> not in this IOCTL:DMA_BUF_SET_NAME.
>
> With this restriction, we can only know the kernel users of the dmabuf
> by attachments.
> However, for many userspace users, such as userpsace users of dma_heap,
> they also need to mark the usage of dma-buf, and they don't care about
> who attached to this dmabuf, and seems it's no meaning to waitting for
to be waiting for
> IOCTL:DMA_BUF_SET_NAME rather than mmap.
>
> Signed-off-by: Guangming Cao <Guangming.Cao@mediatek.com>
> ---
> drivers/dma-buf/dma-buf.c | 14 ++------------
> 1 file changed, 2 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 511fe0d217a0..afbd0a226639 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -325,10 +325,8 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll)
>
> /**
> * dma_buf_set_name - Set a name to a specific dma_buf to track the usage.
> - * The name of the dma-buf buffer can only be set when the dma-buf is not
> - * attached to any devices. It could theoritically support changing the
> - * name of the dma-buf if the same piece of memory is used for multiple
> - * purpose between different devices.
> + * It could theoritically support changing the name of the dma-buf if the same
theoretically
(yes, it was incorrect before this change.)
> + * piece of memory is used for multiple purpose between different devices.
> *
> * @dmabuf: [in] dmabuf buffer that will be renamed.
> * @buf: [in] A piece of userspace memory that contains the name of
> @@ -346,19 +344,11 @@ static long dma_buf_set_name(struct dma_buf *dmabuf, const char __user *buf)
> if (IS_ERR(name))
> return PTR_ERR(name);
>
> - dma_resv_lock(dmabuf->resv, NULL);
> - if (!list_empty(&dmabuf->attachments)) {
> - ret = -EBUSY;
> - kfree(name);
> - goto out_unlock;
> - }
> spin_lock(&dmabuf->name_lock);
> kfree(dmabuf->name);
> dmabuf->name = name;
> spin_unlock(&dmabuf->name_lock);
>
> -out_unlock:
> - dma_resv_unlock(dmabuf->resv);
> return ret;
> }
>
>
--
~Randy
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH] dma-buf: remove restriction of IOCTL:DMA_BUF_SET_NAME
@ 2021-10-09 4:41 ` Randy Dunlap
0 siblings, 0 replies; 35+ messages in thread
From: Randy Dunlap @ 2021-10-09 4:41 UTC (permalink / raw)
To: guangming.cao, Sumit Semwal, Christian König,
Matthias Brugger, open list:DMA BUFFER SHARING FRAMEWORK,
open list:DMA BUFFER SHARING FRAMEWORK,
moderated list:DMA BUFFER SHARING FRAMEWORK, open list,
moderated list:ARM/Mediatek SoC support,
moderated list:ARM/Mediatek SoC support
Cc: wsd_upstream
On 10/8/21 7:47 PM, guangming.cao@mediatek.com wrote:
> From: Guangming Cao <Guangming.Cao@mediatek.com>
>
> If dma-buf don't want userspace users to touch the dmabuf buffer,
> it seems we should add this restriction into dma_buf_ops.mmap,
> not in this IOCTL:DMA_BUF_SET_NAME.
>
> With this restriction, we can only know the kernel users of the dmabuf
> by attachments.
> However, for many userspace users, such as userpsace users of dma_heap,
> they also need to mark the usage of dma-buf, and they don't care about
> who attached to this dmabuf, and seems it's no meaning to waitting for
to be waiting for
> IOCTL:DMA_BUF_SET_NAME rather than mmap.
>
> Signed-off-by: Guangming Cao <Guangming.Cao@mediatek.com>
> ---
> drivers/dma-buf/dma-buf.c | 14 ++------------
> 1 file changed, 2 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 511fe0d217a0..afbd0a226639 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -325,10 +325,8 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll)
>
> /**
> * dma_buf_set_name - Set a name to a specific dma_buf to track the usage.
> - * The name of the dma-buf buffer can only be set when the dma-buf is not
> - * attached to any devices. It could theoritically support changing the
> - * name of the dma-buf if the same piece of memory is used for multiple
> - * purpose between different devices.
> + * It could theoritically support changing the name of the dma-buf if the same
theoretically
(yes, it was incorrect before this change.)
> + * piece of memory is used for multiple purpose between different devices.
> *
> * @dmabuf: [in] dmabuf buffer that will be renamed.
> * @buf: [in] A piece of userspace memory that contains the name of
> @@ -346,19 +344,11 @@ static long dma_buf_set_name(struct dma_buf *dmabuf, const char __user *buf)
> if (IS_ERR(name))
> return PTR_ERR(name);
>
> - dma_resv_lock(dmabuf->resv, NULL);
> - if (!list_empty(&dmabuf->attachments)) {
> - ret = -EBUSY;
> - kfree(name);
> - goto out_unlock;
> - }
> spin_lock(&dmabuf->name_lock);
> kfree(dmabuf->name);
> dmabuf->name = name;
> spin_unlock(&dmabuf->name_lock);
>
> -out_unlock:
> - dma_resv_unlock(dmabuf->resv);
> return ret;
> }
>
>
--
~Randy
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH] dma-buf: remove restriction of IOCTL:DMA_BUF_SET_NAME
@ 2021-10-09 4:41 ` Randy Dunlap
0 siblings, 0 replies; 35+ messages in thread
From: Randy Dunlap @ 2021-10-09 4:41 UTC (permalink / raw)
To: guangming.cao, Sumit Semwal, Christian König,
Matthias Brugger, open list:DMA BUFFER SHARING FRAMEWORK,
open list:DMA BUFFER SHARING FRAMEWORK,
moderated list:DMA BUFFER SHARING FRAMEWORK, open list,
moderated list:ARM/Mediatek SoC support,
moderated list:ARM/Mediatek SoC support
Cc: wsd_upstream
On 10/8/21 7:47 PM, guangming.cao@mediatek.com wrote:
> From: Guangming Cao <Guangming.Cao@mediatek.com>
>
> If dma-buf don't want userspace users to touch the dmabuf buffer,
> it seems we should add this restriction into dma_buf_ops.mmap,
> not in this IOCTL:DMA_BUF_SET_NAME.
>
> With this restriction, we can only know the kernel users of the dmabuf
> by attachments.
> However, for many userspace users, such as userpsace users of dma_heap,
> they also need to mark the usage of dma-buf, and they don't care about
> who attached to this dmabuf, and seems it's no meaning to waitting for
to be waiting for
> IOCTL:DMA_BUF_SET_NAME rather than mmap.
>
> Signed-off-by: Guangming Cao <Guangming.Cao@mediatek.com>
> ---
> drivers/dma-buf/dma-buf.c | 14 ++------------
> 1 file changed, 2 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 511fe0d217a0..afbd0a226639 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -325,10 +325,8 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll)
>
> /**
> * dma_buf_set_name - Set a name to a specific dma_buf to track the usage.
> - * The name of the dma-buf buffer can only be set when the dma-buf is not
> - * attached to any devices. It could theoritically support changing the
> - * name of the dma-buf if the same piece of memory is used for multiple
> - * purpose between different devices.
> + * It could theoritically support changing the name of the dma-buf if the same
theoretically
(yes, it was incorrect before this change.)
> + * piece of memory is used for multiple purpose between different devices.
> *
> * @dmabuf: [in] dmabuf buffer that will be renamed.
> * @buf: [in] A piece of userspace memory that contains the name of
> @@ -346,19 +344,11 @@ static long dma_buf_set_name(struct dma_buf *dmabuf, const char __user *buf)
> if (IS_ERR(name))
> return PTR_ERR(name);
>
> - dma_resv_lock(dmabuf->resv, NULL);
> - if (!list_empty(&dmabuf->attachments)) {
> - ret = -EBUSY;
> - kfree(name);
> - goto out_unlock;
> - }
> spin_lock(&dmabuf->name_lock);
> kfree(dmabuf->name);
> dmabuf->name = name;
> spin_unlock(&dmabuf->name_lock);
>
> -out_unlock:
> - dma_resv_unlock(dmabuf->resv);
> return ret;
> }
>
>
--
~Randy
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v2] dma-buf: remove restriction of IOCTL:DMA_BUF_SET_NAME
2021-10-09 4:41 ` Randy Dunlap
(?)
@ 2021-10-09 5:55 ` guangming.cao
-1 siblings, 0 replies; 35+ messages in thread
From: guangming.cao @ 2021-10-09 5:55 UTC (permalink / raw)
To: rdunlap
Cc: christian.koenig, dri-devel, guangming.cao, linaro-mm-sig,
linux-arm-kernel, linux-kernel, linux-media, linux-mediatek,
matthias.bgg, sumit.semwal, wsd_upstream, Guangming Cao
From: Guangming Cao <Guangming.Cao@mediatek.com>
If dma-buf don't want userspace users to touch the dmabuf buffer,
it seems we should add this restriction into dma_buf_ops.mmap,
not in this IOCTL:DMA_BUF_SET_NAME.
With this restriction, we can only know the kernel users of the dmabuf
by attachments.
However, for many userspace users, such as userpsace users of dma_heap,
they also need to mark the usage of dma-buf, and they don't care about
who attached to this dmabuf, and seems it's no meaning to be waiting for
IOCTL:DMA_BUF_SET_NAME rather than mmap.
Signed-off-by: Guangming Cao <Guangming.Cao@mediatek.com>
---
drivers/dma-buf/dma-buf.c | 14 ++------------
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 511fe0d217a0..db2f4efdec32 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -325,10 +325,8 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll)
/**
* dma_buf_set_name - Set a name to a specific dma_buf to track the usage.
- * The name of the dma-buf buffer can only be set when the dma-buf is not
- * attached to any devices. It could theoritically support changing the
- * name of the dma-buf if the same piece of memory is used for multiple
- * purpose between different devices.
+ * It could theoretically support changing the name of the dma-buf if the same
+ * piece of memory is used for multiple purpose between different devices.
*
* @dmabuf: [in] dmabuf buffer that will be renamed.
* @buf: [in] A piece of userspace memory that contains the name of
@@ -346,19 +344,11 @@ static long dma_buf_set_name(struct dma_buf *dmabuf, const char __user *buf)
if (IS_ERR(name))
return PTR_ERR(name);
- dma_resv_lock(dmabuf->resv, NULL);
- if (!list_empty(&dmabuf->attachments)) {
- ret = -EBUSY;
- kfree(name);
- goto out_unlock;
- }
spin_lock(&dmabuf->name_lock);
kfree(dmabuf->name);
dmabuf->name = name;
spin_unlock(&dmabuf->name_lock);
-out_unlock:
- dma_resv_unlock(dmabuf->resv);
return ret;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v2] dma-buf: remove restriction of IOCTL:DMA_BUF_SET_NAME
@ 2021-10-09 5:55 ` guangming.cao
0 siblings, 0 replies; 35+ messages in thread
From: guangming.cao @ 2021-10-09 5:55 UTC (permalink / raw)
To: rdunlap
Cc: christian.koenig, dri-devel, guangming.cao, linaro-mm-sig,
linux-arm-kernel, linux-kernel, linux-media, linux-mediatek,
matthias.bgg, sumit.semwal, wsd_upstream, Guangming Cao
From: Guangming Cao <Guangming.Cao@mediatek.com>
If dma-buf don't want userspace users to touch the dmabuf buffer,
it seems we should add this restriction into dma_buf_ops.mmap,
not in this IOCTL:DMA_BUF_SET_NAME.
With this restriction, we can only know the kernel users of the dmabuf
by attachments.
However, for many userspace users, such as userpsace users of dma_heap,
they also need to mark the usage of dma-buf, and they don't care about
who attached to this dmabuf, and seems it's no meaning to be waiting for
IOCTL:DMA_BUF_SET_NAME rather than mmap.
Signed-off-by: Guangming Cao <Guangming.Cao@mediatek.com>
---
drivers/dma-buf/dma-buf.c | 14 ++------------
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 511fe0d217a0..db2f4efdec32 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -325,10 +325,8 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll)
/**
* dma_buf_set_name - Set a name to a specific dma_buf to track the usage.
- * The name of the dma-buf buffer can only be set when the dma-buf is not
- * attached to any devices. It could theoritically support changing the
- * name of the dma-buf if the same piece of memory is used for multiple
- * purpose between different devices.
+ * It could theoretically support changing the name of the dma-buf if the same
+ * piece of memory is used for multiple purpose between different devices.
*
* @dmabuf: [in] dmabuf buffer that will be renamed.
* @buf: [in] A piece of userspace memory that contains the name of
@@ -346,19 +344,11 @@ static long dma_buf_set_name(struct dma_buf *dmabuf, const char __user *buf)
if (IS_ERR(name))
return PTR_ERR(name);
- dma_resv_lock(dmabuf->resv, NULL);
- if (!list_empty(&dmabuf->attachments)) {
- ret = -EBUSY;
- kfree(name);
- goto out_unlock;
- }
spin_lock(&dmabuf->name_lock);
kfree(dmabuf->name);
dmabuf->name = name;
spin_unlock(&dmabuf->name_lock);
-out_unlock:
- dma_resv_unlock(dmabuf->resv);
return ret;
}
--
2.17.1
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v2] dma-buf: remove restriction of IOCTL:DMA_BUF_SET_NAME
@ 2021-10-09 5:55 ` guangming.cao
0 siblings, 0 replies; 35+ messages in thread
From: guangming.cao @ 2021-10-09 5:55 UTC (permalink / raw)
To: rdunlap
Cc: christian.koenig, dri-devel, guangming.cao, linaro-mm-sig,
linux-arm-kernel, linux-kernel, linux-media, linux-mediatek,
matthias.bgg, sumit.semwal, wsd_upstream, Guangming Cao
From: Guangming Cao <Guangming.Cao@mediatek.com>
If dma-buf don't want userspace users to touch the dmabuf buffer,
it seems we should add this restriction into dma_buf_ops.mmap,
not in this IOCTL:DMA_BUF_SET_NAME.
With this restriction, we can only know the kernel users of the dmabuf
by attachments.
However, for many userspace users, such as userpsace users of dma_heap,
they also need to mark the usage of dma-buf, and they don't care about
who attached to this dmabuf, and seems it's no meaning to be waiting for
IOCTL:DMA_BUF_SET_NAME rather than mmap.
Signed-off-by: Guangming Cao <Guangming.Cao@mediatek.com>
---
drivers/dma-buf/dma-buf.c | 14 ++------------
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 511fe0d217a0..db2f4efdec32 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -325,10 +325,8 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll)
/**
* dma_buf_set_name - Set a name to a specific dma_buf to track the usage.
- * The name of the dma-buf buffer can only be set when the dma-buf is not
- * attached to any devices. It could theoritically support changing the
- * name of the dma-buf if the same piece of memory is used for multiple
- * purpose between different devices.
+ * It could theoretically support changing the name of the dma-buf if the same
+ * piece of memory is used for multiple purpose between different devices.
*
* @dmabuf: [in] dmabuf buffer that will be renamed.
* @buf: [in] A piece of userspace memory that contains the name of
@@ -346,19 +344,11 @@ static long dma_buf_set_name(struct dma_buf *dmabuf, const char __user *buf)
if (IS_ERR(name))
return PTR_ERR(name);
- dma_resv_lock(dmabuf->resv, NULL);
- if (!list_empty(&dmabuf->attachments)) {
- ret = -EBUSY;
- kfree(name);
- goto out_unlock;
- }
spin_lock(&dmabuf->name_lock);
kfree(dmabuf->name);
dmabuf->name = name;
spin_unlock(&dmabuf->name_lock);
-out_unlock:
- dma_resv_unlock(dmabuf->resv);
return ret;
}
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH v2] dma-buf: remove restriction of IOCTL:DMA_BUF_SET_NAME
2021-10-09 5:55 ` guangming.cao
(?)
@ 2021-10-11 8:20 ` Christian König
-1 siblings, 0 replies; 35+ messages in thread
From: Christian König @ 2021-10-11 8:20 UTC (permalink / raw)
To: guangming.cao, rdunlap
Cc: dri-devel, linaro-mm-sig, linux-arm-kernel, linux-kernel,
linux-media, linux-mediatek, matthias.bgg, sumit.semwal,
wsd_upstream
Am 09.10.21 um 07:55 schrieb guangming.cao@mediatek.com:
> From: Guangming Cao <Guangming.Cao@mediatek.com>
>
> If dma-buf don't want userspace users to touch the dmabuf buffer,
> it seems we should add this restriction into dma_buf_ops.mmap,
> not in this IOCTL:DMA_BUF_SET_NAME.
>
> With this restriction, we can only know the kernel users of the dmabuf
> by attachments.
> However, for many userspace users, such as userpsace users of dma_heap,
> they also need to mark the usage of dma-buf, and they don't care about
> who attached to this dmabuf, and seems it's no meaning to be waiting for
> IOCTL:DMA_BUF_SET_NAME rather than mmap.
Sounds valid to me, but I have no idea why this restriction was added in
the first place.
Can you double check the git history and maybe identify when that was
added? Mentioning this change in the commit message then might make
things a bit easier to understand.
Thanks,
Christian.
>
> Signed-off-by: Guangming Cao <Guangming.Cao@mediatek.com>
> ---
> drivers/dma-buf/dma-buf.c | 14 ++------------
> 1 file changed, 2 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 511fe0d217a0..db2f4efdec32 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -325,10 +325,8 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll)
>
> /**
> * dma_buf_set_name - Set a name to a specific dma_buf to track the usage.
> - * The name of the dma-buf buffer can only be set when the dma-buf is not
> - * attached to any devices. It could theoritically support changing the
> - * name of the dma-buf if the same piece of memory is used for multiple
> - * purpose between different devices.
> + * It could theoretically support changing the name of the dma-buf if the same
> + * piece of memory is used for multiple purpose between different devices.
> *
> * @dmabuf: [in] dmabuf buffer that will be renamed.
> * @buf: [in] A piece of userspace memory that contains the name of
> @@ -346,19 +344,11 @@ static long dma_buf_set_name(struct dma_buf *dmabuf, const char __user *buf)
> if (IS_ERR(name))
> return PTR_ERR(name);
>
> - dma_resv_lock(dmabuf->resv, NULL);
> - if (!list_empty(&dmabuf->attachments)) {
> - ret = -EBUSY;
> - kfree(name);
> - goto out_unlock;
> - }
> spin_lock(&dmabuf->name_lock);
> kfree(dmabuf->name);
> dmabuf->name = name;
> spin_unlock(&dmabuf->name_lock);
>
> -out_unlock:
> - dma_resv_unlock(dmabuf->resv);
> return ret;
> }
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2] dma-buf: remove restriction of IOCTL:DMA_BUF_SET_NAME
@ 2021-10-11 8:20 ` Christian König
0 siblings, 0 replies; 35+ messages in thread
From: Christian König @ 2021-10-11 8:20 UTC (permalink / raw)
To: guangming.cao, rdunlap
Cc: dri-devel, linaro-mm-sig, linux-arm-kernel, linux-kernel,
linux-media, linux-mediatek, matthias.bgg, sumit.semwal,
wsd_upstream
Am 09.10.21 um 07:55 schrieb guangming.cao@mediatek.com:
> From: Guangming Cao <Guangming.Cao@mediatek.com>
>
> If dma-buf don't want userspace users to touch the dmabuf buffer,
> it seems we should add this restriction into dma_buf_ops.mmap,
> not in this IOCTL:DMA_BUF_SET_NAME.
>
> With this restriction, we can only know the kernel users of the dmabuf
> by attachments.
> However, for many userspace users, such as userpsace users of dma_heap,
> they also need to mark the usage of dma-buf, and they don't care about
> who attached to this dmabuf, and seems it's no meaning to be waiting for
> IOCTL:DMA_BUF_SET_NAME rather than mmap.
Sounds valid to me, but I have no idea why this restriction was added in
the first place.
Can you double check the git history and maybe identify when that was
added? Mentioning this change in the commit message then might make
things a bit easier to understand.
Thanks,
Christian.
>
> Signed-off-by: Guangming Cao <Guangming.Cao@mediatek.com>
> ---
> drivers/dma-buf/dma-buf.c | 14 ++------------
> 1 file changed, 2 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 511fe0d217a0..db2f4efdec32 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -325,10 +325,8 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll)
>
> /**
> * dma_buf_set_name - Set a name to a specific dma_buf to track the usage.
> - * The name of the dma-buf buffer can only be set when the dma-buf is not
> - * attached to any devices. It could theoritically support changing the
> - * name of the dma-buf if the same piece of memory is used for multiple
> - * purpose between different devices.
> + * It could theoretically support changing the name of the dma-buf if the same
> + * piece of memory is used for multiple purpose between different devices.
> *
> * @dmabuf: [in] dmabuf buffer that will be renamed.
> * @buf: [in] A piece of userspace memory that contains the name of
> @@ -346,19 +344,11 @@ static long dma_buf_set_name(struct dma_buf *dmabuf, const char __user *buf)
> if (IS_ERR(name))
> return PTR_ERR(name);
>
> - dma_resv_lock(dmabuf->resv, NULL);
> - if (!list_empty(&dmabuf->attachments)) {
> - ret = -EBUSY;
> - kfree(name);
> - goto out_unlock;
> - }
> spin_lock(&dmabuf->name_lock);
> kfree(dmabuf->name);
> dmabuf->name = name;
> spin_unlock(&dmabuf->name_lock);
>
> -out_unlock:
> - dma_resv_unlock(dmabuf->resv);
> return ret;
> }
>
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2] dma-buf: remove restriction of IOCTL:DMA_BUF_SET_NAME
@ 2021-10-11 8:20 ` Christian König
0 siblings, 0 replies; 35+ messages in thread
From: Christian König @ 2021-10-11 8:20 UTC (permalink / raw)
To: guangming.cao, rdunlap
Cc: dri-devel, linaro-mm-sig, linux-arm-kernel, linux-kernel,
linux-media, linux-mediatek, matthias.bgg, sumit.semwal,
wsd_upstream
Am 09.10.21 um 07:55 schrieb guangming.cao@mediatek.com:
> From: Guangming Cao <Guangming.Cao@mediatek.com>
>
> If dma-buf don't want userspace users to touch the dmabuf buffer,
> it seems we should add this restriction into dma_buf_ops.mmap,
> not in this IOCTL:DMA_BUF_SET_NAME.
>
> With this restriction, we can only know the kernel users of the dmabuf
> by attachments.
> However, for many userspace users, such as userpsace users of dma_heap,
> they also need to mark the usage of dma-buf, and they don't care about
> who attached to this dmabuf, and seems it's no meaning to be waiting for
> IOCTL:DMA_BUF_SET_NAME rather than mmap.
Sounds valid to me, but I have no idea why this restriction was added in
the first place.
Can you double check the git history and maybe identify when that was
added? Mentioning this change in the commit message then might make
things a bit easier to understand.
Thanks,
Christian.
>
> Signed-off-by: Guangming Cao <Guangming.Cao@mediatek.com>
> ---
> drivers/dma-buf/dma-buf.c | 14 ++------------
> 1 file changed, 2 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 511fe0d217a0..db2f4efdec32 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -325,10 +325,8 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll)
>
> /**
> * dma_buf_set_name - Set a name to a specific dma_buf to track the usage.
> - * The name of the dma-buf buffer can only be set when the dma-buf is not
> - * attached to any devices. It could theoritically support changing the
> - * name of the dma-buf if the same piece of memory is used for multiple
> - * purpose between different devices.
> + * It could theoretically support changing the name of the dma-buf if the same
> + * piece of memory is used for multiple purpose between different devices.
> *
> * @dmabuf: [in] dmabuf buffer that will be renamed.
> * @buf: [in] A piece of userspace memory that contains the name of
> @@ -346,19 +344,11 @@ static long dma_buf_set_name(struct dma_buf *dmabuf, const char __user *buf)
> if (IS_ERR(name))
> return PTR_ERR(name);
>
> - dma_resv_lock(dmabuf->resv, NULL);
> - if (!list_empty(&dmabuf->attachments)) {
> - ret = -EBUSY;
> - kfree(name);
> - goto out_unlock;
> - }
> spin_lock(&dmabuf->name_lock);
> kfree(dmabuf->name);
> dmabuf->name = name;
> spin_unlock(&dmabuf->name_lock);
>
> -out_unlock:
> - dma_resv_unlock(dmabuf->resv);
> return ret;
> }
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2] dma-buf: remove restriction of IOCTL:DMA_BUF_SET_NAME
2021-10-11 8:20 ` Christian König
(?)
@ 2021-10-12 8:41 ` guangming.cao
-1 siblings, 0 replies; 35+ messages in thread
From: guangming.cao @ 2021-10-12 8:41 UTC (permalink / raw)
To: christian.koenig
Cc: dri-devel, guangming.cao, linaro-mm-sig, linux-arm-kernel,
linux-kernel, linux-media, linux-mediatek, matthias.bgg, rdunlap,
sumit.semwal, wsd_upstream, Guangming Cao
From: Guangming Cao <Guangming.Cao@mediatek.com>
> Am 09.10.21 um 07:55 schrieb guangming.cao@mediatek.com:
> From: Guangming Cao <Guangming.Cao@mediatek.com>
> >
> > If dma-buf don't want userspace users to touch the dmabuf buffer,
> > it seems we should add this restriction into dma_buf_ops.mmap,
> > not in this IOCTL:DMA_BUF_SET_NAME.
> >
> > With this restriction, we can only know the kernel users of the dmabuf
> > by attachments.
> > However, for many userspace users, such as userpsace users of dma_heap,
> > they also need to mark the usage of dma-buf, and they don't care about
> > who attached to this dmabuf, and seems it's no meaning to be waiting for
> > IOCTL:DMA_BUF_SET_NAME rather than mmap.
>
> Sounds valid to me, but I have no idea why this restriction was added in
> the first place.
>
> Can you double check the git history and maybe identify when that was
> added? Mentioning this change in the commit message then might make
> things a bit easier to understand.
>
> Thanks,
> Christian.
It was add in this patch: https://patchwork.freedesktop.org/patch/310349/.
However, there is no illustration about it.
I guess it wants users to set_name when no attachments on the dmabuf,
for case with attachments, we can find owner by device in attachments.
But just I said in commit message, this is might not a good idea.
Do you have any idea?
>
> >
> > Signed-off-by: Guangming Cao <Guangming.Cao@mediatek.com>
> > ---
> > drivers/dma-buf/dma-buf.c | 14 ++------------
> > 1 file changed, 2 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> > index 511fe0d217a0..db2f4efdec32 100644
> > --- a/drivers/dma-buf/dma-buf.c
> > +++ b/drivers/dma-buf/dma-buf.c
> > @@ -325,10 +325,8 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll)
> >
> > /**
> > * dma_buf_set_name - Set a name to a specific dma_buf to track the usage.
> > - * The name of the dma-buf buffer can only be set when the dma-buf is not
> > - * attached to any devices. It could theoritically support changing the
> > - * name of the dma-buf if the same piece of memory is used for multiple
> > - * purpose between different devices.
> > + * It could theoretically support changing the name of the dma-buf if the same
> > + * piece of memory is used for multiple purpose between different devices.
> > *
> > * @dmabuf: [in] dmabuf buffer that will be renamed.
> > * @buf: [in] A piece of userspace memory that contains the name of
> > @@ -346,19 +344,11 @@ static long dma_buf_set_name(struct dma_buf *dmabuf, const char __user *buf)
> > if (IS_ERR(name))
> > return PTR_ERR(name);
> >
> > - dma_resv_lock(dmabuf->resv, NULL);
> > - if (!list_empty(&dmabuf->attachments)) {
> > - ret = -EBUSY;
> > - kfree(name);
> > - goto out_unlock;
> > - }
> > spin_lock(&dmabuf->name_lock);
> > kfree(dmabuf->name);
> > dmabuf->name = name;
> > spin_unlock(&dmabuf->name_lock);
> >
> > -out_unlock:
> > - dma_resv_unlock(dmabuf->resv);
> > return ret;
> > }
> >
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2] dma-buf: remove restriction of IOCTL:DMA_BUF_SET_NAME
@ 2021-10-12 8:41 ` guangming.cao
0 siblings, 0 replies; 35+ messages in thread
From: guangming.cao @ 2021-10-12 8:41 UTC (permalink / raw)
To: christian.koenig
Cc: dri-devel, guangming.cao, linaro-mm-sig, linux-arm-kernel,
linux-kernel, linux-media, linux-mediatek, matthias.bgg, rdunlap,
sumit.semwal, wsd_upstream, Guangming Cao
From: Guangming Cao <Guangming.Cao@mediatek.com>
> Am 09.10.21 um 07:55 schrieb guangming.cao@mediatek.com:
> From: Guangming Cao <Guangming.Cao@mediatek.com>
> >
> > If dma-buf don't want userspace users to touch the dmabuf buffer,
> > it seems we should add this restriction into dma_buf_ops.mmap,
> > not in this IOCTL:DMA_BUF_SET_NAME.
> >
> > With this restriction, we can only know the kernel users of the dmabuf
> > by attachments.
> > However, for many userspace users, such as userpsace users of dma_heap,
> > they also need to mark the usage of dma-buf, and they don't care about
> > who attached to this dmabuf, and seems it's no meaning to be waiting for
> > IOCTL:DMA_BUF_SET_NAME rather than mmap.
>
> Sounds valid to me, but I have no idea why this restriction was added in
> the first place.
>
> Can you double check the git history and maybe identify when that was
> added? Mentioning this change in the commit message then might make
> things a bit easier to understand.
>
> Thanks,
> Christian.
It was add in this patch: https://patchwork.freedesktop.org/patch/310349/.
However, there is no illustration about it.
I guess it wants users to set_name when no attachments on the dmabuf,
for case with attachments, we can find owner by device in attachments.
But just I said in commit message, this is might not a good idea.
Do you have any idea?
>
> >
> > Signed-off-by: Guangming Cao <Guangming.Cao@mediatek.com>
> > ---
> > drivers/dma-buf/dma-buf.c | 14 ++------------
> > 1 file changed, 2 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> > index 511fe0d217a0..db2f4efdec32 100644
> > --- a/drivers/dma-buf/dma-buf.c
> > +++ b/drivers/dma-buf/dma-buf.c
> > @@ -325,10 +325,8 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll)
> >
> > /**
> > * dma_buf_set_name - Set a name to a specific dma_buf to track the usage.
> > - * The name of the dma-buf buffer can only be set when the dma-buf is not
> > - * attached to any devices. It could theoritically support changing the
> > - * name of the dma-buf if the same piece of memory is used for multiple
> > - * purpose between different devices.
> > + * It could theoretically support changing the name of the dma-buf if the same
> > + * piece of memory is used for multiple purpose between different devices.
> > *
> > * @dmabuf: [in] dmabuf buffer that will be renamed.
> > * @buf: [in] A piece of userspace memory that contains the name of
> > @@ -346,19 +344,11 @@ static long dma_buf_set_name(struct dma_buf *dmabuf, const char __user *buf)
> > if (IS_ERR(name))
> > return PTR_ERR(name);
> >
> > - dma_resv_lock(dmabuf->resv, NULL);
> > - if (!list_empty(&dmabuf->attachments)) {
> > - ret = -EBUSY;
> > - kfree(name);
> > - goto out_unlock;
> > - }
> > spin_lock(&dmabuf->name_lock);
> > kfree(dmabuf->name);
> > dmabuf->name = name;
> > spin_unlock(&dmabuf->name_lock);
> >
> > -out_unlock:
> > - dma_resv_unlock(dmabuf->resv);
> > return ret;
> > }
> >
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2] dma-buf: remove restriction of IOCTL:DMA_BUF_SET_NAME
@ 2021-10-12 8:41 ` guangming.cao
0 siblings, 0 replies; 35+ messages in thread
From: guangming.cao @ 2021-10-12 8:41 UTC (permalink / raw)
To: christian.koenig
Cc: dri-devel, guangming.cao, linaro-mm-sig, linux-arm-kernel,
linux-kernel, linux-media, linux-mediatek, matthias.bgg, rdunlap,
sumit.semwal, wsd_upstream, Guangming Cao
From: Guangming Cao <Guangming.Cao@mediatek.com>
> Am 09.10.21 um 07:55 schrieb guangming.cao@mediatek.com:
> From: Guangming Cao <Guangming.Cao@mediatek.com>
> >
> > If dma-buf don't want userspace users to touch the dmabuf buffer,
> > it seems we should add this restriction into dma_buf_ops.mmap,
> > not in this IOCTL:DMA_BUF_SET_NAME.
> >
> > With this restriction, we can only know the kernel users of the dmabuf
> > by attachments.
> > However, for many userspace users, such as userpsace users of dma_heap,
> > they also need to mark the usage of dma-buf, and they don't care about
> > who attached to this dmabuf, and seems it's no meaning to be waiting for
> > IOCTL:DMA_BUF_SET_NAME rather than mmap.
>
> Sounds valid to me, but I have no idea why this restriction was added in
> the first place.
>
> Can you double check the git history and maybe identify when that was
> added? Mentioning this change in the commit message then might make
> things a bit easier to understand.
>
> Thanks,
> Christian.
It was add in this patch: https://patchwork.freedesktop.org/patch/310349/.
However, there is no illustration about it.
I guess it wants users to set_name when no attachments on the dmabuf,
for case with attachments, we can find owner by device in attachments.
But just I said in commit message, this is might not a good idea.
Do you have any idea?
>
> >
> > Signed-off-by: Guangming Cao <Guangming.Cao@mediatek.com>
> > ---
> > drivers/dma-buf/dma-buf.c | 14 ++------------
> > 1 file changed, 2 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> > index 511fe0d217a0..db2f4efdec32 100644
> > --- a/drivers/dma-buf/dma-buf.c
> > +++ b/drivers/dma-buf/dma-buf.c
> > @@ -325,10 +325,8 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll)
> >
> > /**
> > * dma_buf_set_name - Set a name to a specific dma_buf to track the usage.
> > - * The name of the dma-buf buffer can only be set when the dma-buf is not
> > - * attached to any devices. It could theoritically support changing the
> > - * name of the dma-buf if the same piece of memory is used for multiple
> > - * purpose between different devices.
> > + * It could theoretically support changing the name of the dma-buf if the same
> > + * piece of memory is used for multiple purpose between different devices.
> > *
> > * @dmabuf: [in] dmabuf buffer that will be renamed.
> > * @buf: [in] A piece of userspace memory that contains the name of
> > @@ -346,19 +344,11 @@ static long dma_buf_set_name(struct dma_buf *dmabuf, const char __user *buf)
> > if (IS_ERR(name))
> > return PTR_ERR(name);
> >
> > - dma_resv_lock(dmabuf->resv, NULL);
> > - if (!list_empty(&dmabuf->attachments)) {
> > - ret = -EBUSY;
> > - kfree(name);
> > - goto out_unlock;
> > - }
> > spin_lock(&dmabuf->name_lock);
> > kfree(dmabuf->name);
> > dmabuf->name = name;
> > spin_unlock(&dmabuf->name_lock);
> >
> > -out_unlock:
> > - dma_resv_unlock(dmabuf->resv);
> > return ret;
> > }
> >
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2] dma-buf: remove restriction of IOCTL:DMA_BUF_SET_NAME
2021-10-12 8:41 ` guangming.cao
(?)
(?)
@ 2021-10-12 23:56 ` Sumit Semwal
2021-10-13 12:20 ` Christian König
-1 siblings, 1 reply; 35+ messages in thread
From: Sumit Semwal @ 2021-10-12 23:56 UTC (permalink / raw)
To: guangming.cao
Cc: Christian Koenig, DRI mailing list, Linaro MM SIG,
linux-arm-kernel, LKML, open list:DMA BUFFER SHARING FRAMEWORK,
linux-mediatek, matthias.bgg, Randy Dunlap, wsd_upstream
[-- Attachment #1: Type: text/plain, Size: 3723 bytes --]
Hello Guangming, Christian,
On Tue, 12 Oct 2021, 14:09 , <guangming.cao@mediatek.com> wrote:
> From: Guangming Cao <Guangming.Cao@mediatek.com>
>
> > Am 09.10.21 um 07:55 schrieb guangming.cao@mediatek.com:
> > From: Guangming Cao <Guangming.Cao@mediatek.com>
> > >
> > > If dma-buf don't want userspace users to touch the dmabuf buffer,
> > > it seems we should add this restriction into dma_buf_ops.mmap,
> > > not in this IOCTL:DMA_BUF_SET_NAME.
> > >
> > > With this restriction, we can only know the kernel users of the dmabuf
> > > by attachments.
> > > However, for many userspace users, such as userpsace users of dma_heap,
> > > they also need to mark the usage of dma-buf, and they don't care about
> > > who attached to this dmabuf, and seems it's no meaning to be waiting
> for
> > > IOCTL:DMA_BUF_SET_NAME rather than mmap.
> >
> > Sounds valid to me, but I have no idea why this restriction was added in
> > the first place.
> >
> > Can you double check the git history and maybe identify when that was
> > added? Mentioning this change in the commit message then might make
> > things a bit easier to understand.
> >
> > Thanks,
> > Christian.
> It was add in this patch: https://patchwork.freedesktop.org/patch/310349/.
> However, there is no illustration about it.
> I guess it wants users to set_name when no attachments on the dmabuf,
> for case with attachments, we can find owner by device in attachments.
> But just I said in commit message, this is might not a good idea.
>
> Do you have any idea?
>
For the original series, the idea was that allowing name change mid-use
could confuse the users about the dma-buf. However, the rest of the series
also makes sure each dma-buf have a unique inode, and any accounting should
probably use that, without relying on the name as much.
So I don't have an objection to this change.
Best,
Sumit.
> >
> > >
> > > Signed-off-by: Guangming Cao <Guangming.Cao@mediatek.com>
> > > ---
> > > drivers/dma-buf/dma-buf.c | 14 ++------------
> > > 1 file changed, 2 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> > > index 511fe0d217a0..db2f4efdec32 100644
> > > --- a/drivers/dma-buf/dma-buf.c
> > > +++ b/drivers/dma-buf/dma-buf.c
> > > @@ -325,10 +325,8 @@ static __poll_t dma_buf_poll(struct file *file,
> poll_table *poll)
> > >
> > > /**
> > > * dma_buf_set_name - Set a name to a specific dma_buf to track the
> usage.
> > > - * The name of the dma-buf buffer can only be set when the dma-buf is
> not
> > > - * attached to any devices. It could theoritically support changing
> the
> > > - * name of the dma-buf if the same piece of memory is used for
> multiple
> > > - * purpose between different devices.
> > > + * It could theoretically support changing the name of the dma-buf if
> the same
> > > + * piece of memory is used for multiple purpose between different
> devices.
> > > *
> > > * @dmabuf: [in] dmabuf buffer that will be renamed.
> > > * @buf: [in] A piece of userspace memory that contains the
> name of
> > > @@ -346,19 +344,11 @@ static long dma_buf_set_name(struct dma_buf
> *dmabuf, const char __user *buf)
> > > if (IS_ERR(name))
> > > return PTR_ERR(name);
> > >
> > > - dma_resv_lock(dmabuf->resv, NULL);
> > > - if (!list_empty(&dmabuf->attachments)) {
> > > - ret = -EBUSY;
> > > - kfree(name);
> > > - goto out_unlock;
> > > - }
> > > spin_lock(&dmabuf->name_lock);
> > > kfree(dmabuf->name);
> > > dmabuf->name = name;
> > > spin_unlock(&dmabuf->name_lock);
> > >
> > > -out_unlock:
> > > - dma_resv_unlock(dmabuf->resv);
> > > return ret;
> > > }
> > >
>
[-- Attachment #2: Type: text/html, Size: 5719 bytes --]
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2] dma-buf: remove restriction of IOCTL:DMA_BUF_SET_NAME
2021-10-12 23:56 ` Sumit Semwal
@ 2021-10-13 12:20 ` Christian König
0 siblings, 0 replies; 35+ messages in thread
From: Christian König @ 2021-10-13 12:20 UTC (permalink / raw)
To: Sumit Semwal, guangming.cao
Cc: DRI mailing list, Linaro MM SIG, linux-arm-kernel, LKML,
open list:DMA BUFFER SHARING FRAMEWORK, linux-mediatek,
matthias.bgg, Randy Dunlap, wsd_upstream
[-- Attachment #1: Type: text/plain, Size: 5037 bytes --]
Am 13.10.21 um 01:56 schrieb Sumit Semwal:
> Hello Guangming, Christian,
>
>
>
> On Tue, 12 Oct 2021, 14:09 , <guangming.cao@mediatek.com
> <mailto:guangming.cao@mediatek.com>> wrote:
>
> From: Guangming Cao <Guangming.Cao@mediatek.com
> <mailto:Guangming.Cao@mediatek.com>>
>
> > Am 09.10.21 um 07:55 schrieb guangming.cao@mediatek.com
> <mailto:guangming.cao@mediatek.com>:
> > From: Guangming Cao <Guangming.Cao@mediatek.com
> <mailto:Guangming.Cao@mediatek.com>>
> > >
> > > If dma-buf don't want userspace users to touch the dmabuf buffer,
> > > it seems we should add this restriction into dma_buf_ops.mmap,
> > > not in this IOCTL:DMA_BUF_SET_NAME.
> > >
> > > With this restriction, we can only know the kernel users of
> the dmabuf
> > > by attachments.
> > > However, for many userspace users, such as userpsace users of
> dma_heap,
> > > they also need to mark the usage of dma-buf, and they don't
> care about
> > > who attached to this dmabuf, and seems it's no meaning to be
> waiting for
> > > IOCTL:DMA_BUF_SET_NAME rather than mmap.
> >
> > Sounds valid to me, but I have no idea why this restriction was
> added in
> > the first place.
> >
> > Can you double check the git history and maybe identify when
> that was
> > added? Mentioning this change in the commit message then might make
> > things a bit easier to understand.
> >
> > Thanks,
> > Christian.
> It was add in this patch:
> https://patchwork.freedesktop.org/patch/310349/
> <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.freedesktop.org%2Fpatch%2F310349%2F&data=04%7C01%7Cchristian.koenig%40amd.com%7C4149923e2b0646de82ce08d98ddbf2c2%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637696798278342557%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=N49RVF4s%2BGQ4D%2Ft1MOwRsCnslFnwobSB3G86pvP9m7A%3D&reserved=0>.
> However, there is no illustration about it.
> I guess it wants users to set_name when no attachments on the dmabuf,
> for case with attachments, we can find owner by device in attachments.
> But just I said in commit message, this is might not a good idea.
>
> Do you have any idea?
>
>
> For the original series, the idea was that allowing name change
> mid-use could confuse the users about the dma-buf. However, the rest
> of the series also makes sure each dma-buf have a unique inode, and
> any accounting should probably use that, without relying on the name
> as much.
> So I don't have an objection to this change.
I suggest to add that explanation and the original commit id into the
commit message.
With that changed the patch has my rb as well.
Regards,
Christian.
>
> Best,
> Sumit.
>
> >
> > >
> > > Signed-off-by: Guangming Cao <Guangming.Cao@mediatek.com
> <mailto:Guangming.Cao@mediatek.com>>
> > > ---
> > > drivers/dma-buf/dma-buf.c | 14 ++------------
> > > 1 file changed, 2 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> > > index 511fe0d217a0..db2f4efdec32 100644
> > > --- a/drivers/dma-buf/dma-buf.c
> > > +++ b/drivers/dma-buf/dma-buf.c
> > > @@ -325,10 +325,8 @@ static __poll_t dma_buf_poll(struct file
> *file, poll_table *poll)
> > >
> > > /**
> > > * dma_buf_set_name - Set a name to a specific dma_buf to
> track the usage.
> > > - * The name of the dma-buf buffer can only be set when the
> dma-buf is not
> > > - * attached to any devices. It could theoritically support
> changing the
> > > - * name of the dma-buf if the same piece of memory is used
> for multiple
> > > - * purpose between different devices.
> > > + * It could theoretically support changing the name of the
> dma-buf if the same
> > > + * piece of memory is used for multiple purpose between
> different devices.
> > > *
> > > * @dmabuf: [in] dmabuf buffer that will be renamed.
> > > * @buf: [in] A piece of userspace memory that
> contains the name of
> > > @@ -346,19 +344,11 @@ static long dma_buf_set_name(struct
> dma_buf *dmabuf, const char __user *buf)
> > > if (IS_ERR(name))
> > > return PTR_ERR(name);
> > >
> > > - dma_resv_lock(dmabuf->resv, NULL);
> > > - if (!list_empty(&dmabuf->attachments)) {
> > > - ret = -EBUSY;
> > > - kfree(name);
> > > - goto out_unlock;
> > > - }
> > > spin_lock(&dmabuf->name_lock);
> > > kfree(dmabuf->name);
> > > dmabuf->name = name;
> > > spin_unlock(&dmabuf->name_lock);
> > >
> > > -out_unlock:
> > > - dma_resv_unlock(dmabuf->resv);
> > > return ret;
> > > }
> > >
>
[-- Attachment #2: Type: text/html, Size: 9308 bytes --]
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v3] dma-buf: remove restriction of IOCTL:DMA_BUF_SET_NAME
2021-10-12 8:41 ` guangming.cao
(?)
@ 2021-10-14 10:25 ` guangming.cao
-1 siblings, 0 replies; 35+ messages in thread
From: guangming.cao @ 2021-10-14 10:25 UTC (permalink / raw)
To: christian.koenig, sumit.semwal
Cc: dri-devel, guangming.cao, linaro-mm-sig, linux-arm-kernel,
linux-kernel, linux-media, linux-mediatek, matthias.bgg, rdunlap,
wsd_upstream, Guangming Cao
From: Guangming Cao <Guangming.Cao@mediatek.com>
In this patch(https://patchwork.freedesktop.org/patch/310349),
it add a new IOCTL to support dma-buf user to set debug name.
But it also added a limitation of this IOCTL, it needs the
attachments of dmabuf should be empty, otherwise it will fail.
For the original series, the idea was that allowing name change
mid-use could confuse the users about the dma-buf.
However, the rest of the series also makes sure each dma-buf have a unique
inode(https://patchwork.freedesktop.org/patch/310387/), and any accounting
should probably use that, without relying on the name as much.
So, removing this restriction will let dma-buf userspace users to use it
more comfortably and without any side effect.
Signed-off-by: Guangming Cao <Guangming.Cao@mediatek.com>
---
drivers/dma-buf/dma-buf.c | 17 +++--------------
1 file changed, 3 insertions(+), 14 deletions(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 511fe0d217a0..5fbb3a2068a3 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -325,10 +325,8 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll)
/**
* dma_buf_set_name - Set a name to a specific dma_buf to track the usage.
- * The name of the dma-buf buffer can only be set when the dma-buf is not
- * attached to any devices. It could theoritically support changing the
- * name of the dma-buf if the same piece of memory is used for multiple
- * purpose between different devices.
+ * It could support changing the name of the dma-buf if the same
+ * piece of memory is used for multiple purpose between different devices.
*
* @dmabuf: [in] dmabuf buffer that will be renamed.
* @buf: [in] A piece of userspace memory that contains the name of
@@ -341,25 +339,16 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll)
static long dma_buf_set_name(struct dma_buf *dmabuf, const char __user *buf)
{
char *name = strndup_user(buf, DMA_BUF_NAME_LEN);
- long ret = 0;
if (IS_ERR(name))
return PTR_ERR(name);
- dma_resv_lock(dmabuf->resv, NULL);
- if (!list_empty(&dmabuf->attachments)) {
- ret = -EBUSY;
- kfree(name);
- goto out_unlock;
- }
spin_lock(&dmabuf->name_lock);
kfree(dmabuf->name);
dmabuf->name = name;
spin_unlock(&dmabuf->name_lock);
-out_unlock:
- dma_resv_unlock(dmabuf->resv);
- return ret;
+ return 0;
}
static long dma_buf_ioctl(struct file *file,
--
2.17.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v3] dma-buf: remove restriction of IOCTL:DMA_BUF_SET_NAME
@ 2021-10-14 10:25 ` guangming.cao
0 siblings, 0 replies; 35+ messages in thread
From: guangming.cao @ 2021-10-14 10:25 UTC (permalink / raw)
To: christian.koenig, sumit.semwal
Cc: dri-devel, guangming.cao, linaro-mm-sig, linux-arm-kernel,
linux-kernel, linux-media, linux-mediatek, matthias.bgg, rdunlap,
wsd_upstream, Guangming Cao
From: Guangming Cao <Guangming.Cao@mediatek.com>
In this patch(https://patchwork.freedesktop.org/patch/310349),
it add a new IOCTL to support dma-buf user to set debug name.
But it also added a limitation of this IOCTL, it needs the
attachments of dmabuf should be empty, otherwise it will fail.
For the original series, the idea was that allowing name change
mid-use could confuse the users about the dma-buf.
However, the rest of the series also makes sure each dma-buf have a unique
inode(https://patchwork.freedesktop.org/patch/310387/), and any accounting
should probably use that, without relying on the name as much.
So, removing this restriction will let dma-buf userspace users to use it
more comfortably and without any side effect.
Signed-off-by: Guangming Cao <Guangming.Cao@mediatek.com>
---
drivers/dma-buf/dma-buf.c | 17 +++--------------
1 file changed, 3 insertions(+), 14 deletions(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 511fe0d217a0..5fbb3a2068a3 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -325,10 +325,8 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll)
/**
* dma_buf_set_name - Set a name to a specific dma_buf to track the usage.
- * The name of the dma-buf buffer can only be set when the dma-buf is not
- * attached to any devices. It could theoritically support changing the
- * name of the dma-buf if the same piece of memory is used for multiple
- * purpose between different devices.
+ * It could support changing the name of the dma-buf if the same
+ * piece of memory is used for multiple purpose between different devices.
*
* @dmabuf: [in] dmabuf buffer that will be renamed.
* @buf: [in] A piece of userspace memory that contains the name of
@@ -341,25 +339,16 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll)
static long dma_buf_set_name(struct dma_buf *dmabuf, const char __user *buf)
{
char *name = strndup_user(buf, DMA_BUF_NAME_LEN);
- long ret = 0;
if (IS_ERR(name))
return PTR_ERR(name);
- dma_resv_lock(dmabuf->resv, NULL);
- if (!list_empty(&dmabuf->attachments)) {
- ret = -EBUSY;
- kfree(name);
- goto out_unlock;
- }
spin_lock(&dmabuf->name_lock);
kfree(dmabuf->name);
dmabuf->name = name;
spin_unlock(&dmabuf->name_lock);
-out_unlock:
- dma_resv_unlock(dmabuf->resv);
- return ret;
+ return 0;
}
static long dma_buf_ioctl(struct file *file,
--
2.17.1
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v3] dma-buf: remove restriction of IOCTL:DMA_BUF_SET_NAME
@ 2021-10-14 10:25 ` guangming.cao
0 siblings, 0 replies; 35+ messages in thread
From: guangming.cao @ 2021-10-14 10:25 UTC (permalink / raw)
To: christian.koenig, sumit.semwal
Cc: dri-devel, guangming.cao, linaro-mm-sig, linux-arm-kernel,
linux-kernel, linux-media, linux-mediatek, matthias.bgg, rdunlap,
wsd_upstream, Guangming Cao
From: Guangming Cao <Guangming.Cao@mediatek.com>
In this patch(https://patchwork.freedesktop.org/patch/310349),
it add a new IOCTL to support dma-buf user to set debug name.
But it also added a limitation of this IOCTL, it needs the
attachments of dmabuf should be empty, otherwise it will fail.
For the original series, the idea was that allowing name change
mid-use could confuse the users about the dma-buf.
However, the rest of the series also makes sure each dma-buf have a unique
inode(https://patchwork.freedesktop.org/patch/310387/), and any accounting
should probably use that, without relying on the name as much.
So, removing this restriction will let dma-buf userspace users to use it
more comfortably and without any side effect.
Signed-off-by: Guangming Cao <Guangming.Cao@mediatek.com>
---
drivers/dma-buf/dma-buf.c | 17 +++--------------
1 file changed, 3 insertions(+), 14 deletions(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 511fe0d217a0..5fbb3a2068a3 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -325,10 +325,8 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll)
/**
* dma_buf_set_name - Set a name to a specific dma_buf to track the usage.
- * The name of the dma-buf buffer can only be set when the dma-buf is not
- * attached to any devices. It could theoritically support changing the
- * name of the dma-buf if the same piece of memory is used for multiple
- * purpose between different devices.
+ * It could support changing the name of the dma-buf if the same
+ * piece of memory is used for multiple purpose between different devices.
*
* @dmabuf: [in] dmabuf buffer that will be renamed.
* @buf: [in] A piece of userspace memory that contains the name of
@@ -341,25 +339,16 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll)
static long dma_buf_set_name(struct dma_buf *dmabuf, const char __user *buf)
{
char *name = strndup_user(buf, DMA_BUF_NAME_LEN);
- long ret = 0;
if (IS_ERR(name))
return PTR_ERR(name);
- dma_resv_lock(dmabuf->resv, NULL);
- if (!list_empty(&dmabuf->attachments)) {
- ret = -EBUSY;
- kfree(name);
- goto out_unlock;
- }
spin_lock(&dmabuf->name_lock);
kfree(dmabuf->name);
dmabuf->name = name;
spin_unlock(&dmabuf->name_lock);
-out_unlock:
- dma_resv_unlock(dmabuf->resv);
- return ret;
+ return 0;
}
static long dma_buf_ioctl(struct file *file,
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v3] dma-buf: remove restriction of IOCTL:DMA_BUF_SET_NAME
2021-10-14 10:25 ` guangming.cao
(?)
@ 2021-10-14 10:35 ` guangming.cao
-1 siblings, 0 replies; 35+ messages in thread
From: guangming.cao @ 2021-10-14 10:35 UTC (permalink / raw)
To: christian.koenig, sumit.semwal
Cc: guangming.cao, dri-devel, linaro-mm-sig, linux-arm-kernel,
linux-kernel, linux-media, linux-mediatek, matthias.bgg, rdunlap,
wsd_upstream, Guangming Cao
From: Guangming Cao <Guangming.Cao@mediatek.com>
On Wed, 2021-10-13 at 14:20 +0200, Christian König wrote:
> Am 13.10.21 um 01:56 schrieb Sumit Semwal:
> > Hello Guangming, Christian,
> >
> >
> >
> > On Tue, 12 Oct 2021, 14:09 , <guangming.cao@mediatek.com> wrote:
> > > From: Guangming Cao <Guangming.Cao@mediatek.com>
> > >
> > > > Am 09.10.21 um 07:55 schrieb guangming.cao@mediatek.com:
> > > > From: Guangming Cao <Guangming.Cao@mediatek.com>
> > > > >
> > > > > If dma-buf don't want userspace users to touch the dmabuf
> > > buffer,
> > > > > it seems we should add this restriction into
> > > dma_buf_ops.mmap,
> > > > > not in this IOCTL:DMA_BUF_SET_NAME.
> > > > >
> > > > > With this restriction, we can only know the kernel users of
> > > the dmabuf
> > > > > by attachments.
> > > > > However, for many userspace users, such as userpsace users of
> > > dma_heap,
> > > > > they also need to mark the usage of dma-buf, and they don't
> > > care about
> > > > > who attached to this dmabuf, and seems it's no meaning to be
> > > waiting for
> > > > > IOCTL:DMA_BUF_SET_NAME rather than mmap.
> > > >
> > > > Sounds valid to me, but I have no idea why this restriction was
> > > added in
> > > > the first place.
> > > >
> > > > Can you double check the git history and maybe identify when
> > > that was
> > > > added? Mentioning this change in the commit message then might
> > > make
> > > > things a bit easier to understand.
> > > >
> > > > Thanks,
> > > > Christian.
> > > It was add in this patch:
> > > https://patchwork.freedesktop.org/patch/310349/.
> > > However, there is no illustration about it.
> > > I guess it wants users to set_name when no attachments on the
> > > dmabuf,
> > > for case with attachments, we can find owner by device in
> > > attachments.
> > > But just I said in commit message, this is might not a good idea.
> > >
> > > Do you have any idea?
> > >
> >
> > For the original series, the idea was that allowing name change
> > mid-use could confuse the users about the dma-buf. However, the
> > rest of the series also makes sure each dma-buf have a unique
> > inode, and any accounting should probably use that, without relying
> > on the name as much.
> > So I don't have an objection to this change.
>
> I suggest to add that explanation and the original commit id into the
> commit message.
>
> With that changed the patch has my rb as well.
>
> Regards,
> Christian.
>
updated, thanks!
Guangming.
> > Best,
> > Sumit.
> > > >
> > > > >
> > > > > Signed-off-by: Guangming Cao <Guangming.Cao@mediatek.com>
> > > > > ---
> > > > > drivers/dma-buf/dma-buf.c | 14 ++------------
> > > > > 1 file changed, 2 insertions(+), 12 deletions(-)
> > > > >
> > > > > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-
> > > buf.c
> > > > > index 511fe0d217a0..db2f4efdec32 100644
> > > > > --- a/drivers/dma-buf/dma-buf.c
> > > > > +++ b/drivers/dma-buf/dma-buf.c
> > > > > @@ -325,10 +325,8 @@ static __poll_t dma_buf_poll(struct file
> > > *file, poll_table *poll)
> > > > >
> > > > > /**
> > > > > * dma_buf_set_name - Set a name to a specific dma_buf to
> > > track the usage.
> > > > > - * The name of the dma-buf buffer can only be set when the
> > > dma-buf is not
> > > > > - * attached to any devices. It could theoritically support
> > > changing the
> > > > > - * name of the dma-buf if the same piece of memory is used
> > > for multiple
> > > > > - * purpose between different devices.
> > > > > + * It could theoretically support changing the name of the
> > > dma-buf if the same
> > > > > + * piece of memory is used for multiple purpose between
> > > different devices.
> > > > > *
> > > > > * @dmabuf: [in] dmabuf buffer that will be renamed.
> > > > > * @buf: [in] A piece of userspace memory that
> > > contains the name of
> > > > > @@ -346,19 +344,11 @@ static long dma_buf_set_name(struct
> > > dma_buf *dmabuf, const char __user *buf)
> > > > > if (IS_ERR(name))
> > > > > return PTR_ERR(name);
> > > > >
> > > > > - dma_resv_lock(dmabuf->resv, NULL);
> > > > > - if (!list_empty(&dmabuf->attachments)) {
> > > > > - ret = -EBUSY;
> > > > > - kfree(name);
> > > > > - goto out_unlock;
> > > > > - }
> > > > > spin_lock(&dmabuf->name_lock);
> > > > > kfree(dmabuf->name);
> > > > > dmabuf->name = name;
> > > > > spin_unlock(&dmabuf->name_lock);
> > > > >
> > > > > -out_unlock:
> > > > > - dma_resv_unlock(dmabuf->resv);
> > > > > return ret;
> > > > > }
> > > > >
> > >
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v3] dma-buf: remove restriction of IOCTL:DMA_BUF_SET_NAME
@ 2021-10-14 10:35 ` guangming.cao
0 siblings, 0 replies; 35+ messages in thread
From: guangming.cao @ 2021-10-14 10:35 UTC (permalink / raw)
To: christian.koenig, sumit.semwal
Cc: guangming.cao, dri-devel, linaro-mm-sig, linux-arm-kernel,
linux-kernel, linux-media, linux-mediatek, matthias.bgg, rdunlap,
wsd_upstream, Guangming Cao
From: Guangming Cao <Guangming.Cao@mediatek.com>
On Wed, 2021-10-13 at 14:20 +0200, Christian König wrote:
> Am 13.10.21 um 01:56 schrieb Sumit Semwal:
> > Hello Guangming, Christian,
> >
> >
> >
> > On Tue, 12 Oct 2021, 14:09 , <guangming.cao@mediatek.com> wrote:
> > > From: Guangming Cao <Guangming.Cao@mediatek.com>
> > >
> > > > Am 09.10.21 um 07:55 schrieb guangming.cao@mediatek.com:
> > > > From: Guangming Cao <Guangming.Cao@mediatek.com>
> > > > >
> > > > > If dma-buf don't want userspace users to touch the dmabuf
> > > buffer,
> > > > > it seems we should add this restriction into
> > > dma_buf_ops.mmap,
> > > > > not in this IOCTL:DMA_BUF_SET_NAME.
> > > > >
> > > > > With this restriction, we can only know the kernel users of
> > > the dmabuf
> > > > > by attachments.
> > > > > However, for many userspace users, such as userpsace users of
> > > dma_heap,
> > > > > they also need to mark the usage of dma-buf, and they don't
> > > care about
> > > > > who attached to this dmabuf, and seems it's no meaning to be
> > > waiting for
> > > > > IOCTL:DMA_BUF_SET_NAME rather than mmap.
> > > >
> > > > Sounds valid to me, but I have no idea why this restriction was
> > > added in
> > > > the first place.
> > > >
> > > > Can you double check the git history and maybe identify when
> > > that was
> > > > added? Mentioning this change in the commit message then might
> > > make
> > > > things a bit easier to understand.
> > > >
> > > > Thanks,
> > > > Christian.
> > > It was add in this patch:
> > > https://patchwork.freedesktop.org/patch/310349/.
> > > However, there is no illustration about it.
> > > I guess it wants users to set_name when no attachments on the
> > > dmabuf,
> > > for case with attachments, we can find owner by device in
> > > attachments.
> > > But just I said in commit message, this is might not a good idea.
> > >
> > > Do you have any idea?
> > >
> >
> > For the original series, the idea was that allowing name change
> > mid-use could confuse the users about the dma-buf. However, the
> > rest of the series also makes sure each dma-buf have a unique
> > inode, and any accounting should probably use that, without relying
> > on the name as much.
> > So I don't have an objection to this change.
>
> I suggest to add that explanation and the original commit id into the
> commit message.
>
> With that changed the patch has my rb as well.
>
> Regards,
> Christian.
>
updated, thanks!
Guangming.
> > Best,
> > Sumit.
> > > >
> > > > >
> > > > > Signed-off-by: Guangming Cao <Guangming.Cao@mediatek.com>
> > > > > ---
> > > > > drivers/dma-buf/dma-buf.c | 14 ++------------
> > > > > 1 file changed, 2 insertions(+), 12 deletions(-)
> > > > >
> > > > > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-
> > > buf.c
> > > > > index 511fe0d217a0..db2f4efdec32 100644
> > > > > --- a/drivers/dma-buf/dma-buf.c
> > > > > +++ b/drivers/dma-buf/dma-buf.c
> > > > > @@ -325,10 +325,8 @@ static __poll_t dma_buf_poll(struct file
> > > *file, poll_table *poll)
> > > > >
> > > > > /**
> > > > > * dma_buf_set_name - Set a name to a specific dma_buf to
> > > track the usage.
> > > > > - * The name of the dma-buf buffer can only be set when the
> > > dma-buf is not
> > > > > - * attached to any devices. It could theoritically support
> > > changing the
> > > > > - * name of the dma-buf if the same piece of memory is used
> > > for multiple
> > > > > - * purpose between different devices.
> > > > > + * It could theoretically support changing the name of the
> > > dma-buf if the same
> > > > > + * piece of memory is used for multiple purpose between
> > > different devices.
> > > > > *
> > > > > * @dmabuf: [in] dmabuf buffer that will be renamed.
> > > > > * @buf: [in] A piece of userspace memory that
> > > contains the name of
> > > > > @@ -346,19 +344,11 @@ static long dma_buf_set_name(struct
> > > dma_buf *dmabuf, const char __user *buf)
> > > > > if (IS_ERR(name))
> > > > > return PTR_ERR(name);
> > > > >
> > > > > - dma_resv_lock(dmabuf->resv, NULL);
> > > > > - if (!list_empty(&dmabuf->attachments)) {
> > > > > - ret = -EBUSY;
> > > > > - kfree(name);
> > > > > - goto out_unlock;
> > > > > - }
> > > > > spin_lock(&dmabuf->name_lock);
> > > > > kfree(dmabuf->name);
> > > > > dmabuf->name = name;
> > > > > spin_unlock(&dmabuf->name_lock);
> > > > >
> > > > > -out_unlock:
> > > > > - dma_resv_unlock(dmabuf->resv);
> > > > > return ret;
> > > > > }
> > > > >
> > >
>
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v3] dma-buf: remove restriction of IOCTL:DMA_BUF_SET_NAME
@ 2021-10-14 10:35 ` guangming.cao
0 siblings, 0 replies; 35+ messages in thread
From: guangming.cao @ 2021-10-14 10:35 UTC (permalink / raw)
To: christian.koenig, sumit.semwal
Cc: guangming.cao, dri-devel, linaro-mm-sig, linux-arm-kernel,
linux-kernel, linux-media, linux-mediatek, matthias.bgg, rdunlap,
wsd_upstream, Guangming Cao
From: Guangming Cao <Guangming.Cao@mediatek.com>
On Wed, 2021-10-13 at 14:20 +0200, Christian König wrote:
> Am 13.10.21 um 01:56 schrieb Sumit Semwal:
> > Hello Guangming, Christian,
> >
> >
> >
> > On Tue, 12 Oct 2021, 14:09 , <guangming.cao@mediatek.com> wrote:
> > > From: Guangming Cao <Guangming.Cao@mediatek.com>
> > >
> > > > Am 09.10.21 um 07:55 schrieb guangming.cao@mediatek.com:
> > > > From: Guangming Cao <Guangming.Cao@mediatek.com>
> > > > >
> > > > > If dma-buf don't want userspace users to touch the dmabuf
> > > buffer,
> > > > > it seems we should add this restriction into
> > > dma_buf_ops.mmap,
> > > > > not in this IOCTL:DMA_BUF_SET_NAME.
> > > > >
> > > > > With this restriction, we can only know the kernel users of
> > > the dmabuf
> > > > > by attachments.
> > > > > However, for many userspace users, such as userpsace users of
> > > dma_heap,
> > > > > they also need to mark the usage of dma-buf, and they don't
> > > care about
> > > > > who attached to this dmabuf, and seems it's no meaning to be
> > > waiting for
> > > > > IOCTL:DMA_BUF_SET_NAME rather than mmap.
> > > >
> > > > Sounds valid to me, but I have no idea why this restriction was
> > > added in
> > > > the first place.
> > > >
> > > > Can you double check the git history and maybe identify when
> > > that was
> > > > added? Mentioning this change in the commit message then might
> > > make
> > > > things a bit easier to understand.
> > > >
> > > > Thanks,
> > > > Christian.
> > > It was add in this patch:
> > > https://patchwork.freedesktop.org/patch/310349/.
> > > However, there is no illustration about it.
> > > I guess it wants users to set_name when no attachments on the
> > > dmabuf,
> > > for case with attachments, we can find owner by device in
> > > attachments.
> > > But just I said in commit message, this is might not a good idea.
> > >
> > > Do you have any idea?
> > >
> >
> > For the original series, the idea was that allowing name change
> > mid-use could confuse the users about the dma-buf. However, the
> > rest of the series also makes sure each dma-buf have a unique
> > inode, and any accounting should probably use that, without relying
> > on the name as much.
> > So I don't have an objection to this change.
>
> I suggest to add that explanation and the original commit id into the
> commit message.
>
> With that changed the patch has my rb as well.
>
> Regards,
> Christian.
>
updated, thanks!
Guangming.
> > Best,
> > Sumit.
> > > >
> > > > >
> > > > > Signed-off-by: Guangming Cao <Guangming.Cao@mediatek.com>
> > > > > ---
> > > > > drivers/dma-buf/dma-buf.c | 14 ++------------
> > > > > 1 file changed, 2 insertions(+), 12 deletions(-)
> > > > >
> > > > > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-
> > > buf.c
> > > > > index 511fe0d217a0..db2f4efdec32 100644
> > > > > --- a/drivers/dma-buf/dma-buf.c
> > > > > +++ b/drivers/dma-buf/dma-buf.c
> > > > > @@ -325,10 +325,8 @@ static __poll_t dma_buf_poll(struct file
> > > *file, poll_table *poll)
> > > > >
> > > > > /**
> > > > > * dma_buf_set_name - Set a name to a specific dma_buf to
> > > track the usage.
> > > > > - * The name of the dma-buf buffer can only be set when the
> > > dma-buf is not
> > > > > - * attached to any devices. It could theoritically support
> > > changing the
> > > > > - * name of the dma-buf if the same piece of memory is used
> > > for multiple
> > > > > - * purpose between different devices.
> > > > > + * It could theoretically support changing the name of the
> > > dma-buf if the same
> > > > > + * piece of memory is used for multiple purpose between
> > > different devices.
> > > > > *
> > > > > * @dmabuf: [in] dmabuf buffer that will be renamed.
> > > > > * @buf: [in] A piece of userspace memory that
> > > contains the name of
> > > > > @@ -346,19 +344,11 @@ static long dma_buf_set_name(struct
> > > dma_buf *dmabuf, const char __user *buf)
> > > > > if (IS_ERR(name))
> > > > > return PTR_ERR(name);
> > > > >
> > > > > - dma_resv_lock(dmabuf->resv, NULL);
> > > > > - if (!list_empty(&dmabuf->attachments)) {
> > > > > - ret = -EBUSY;
> > > > > - kfree(name);
> > > > > - goto out_unlock;
> > > > > - }
> > > > > spin_lock(&dmabuf->name_lock);
> > > > > kfree(dmabuf->name);
> > > > > dmabuf->name = name;
> > > > > spin_unlock(&dmabuf->name_lock);
> > > > >
> > > > > -out_unlock:
> > > > > - dma_resv_unlock(dmabuf->resv);
> > > > > return ret;
> > > > > }
> > > > >
> > >
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v3] dma-buf: remove restriction of IOCTL:DMA_BUF_SET_NAME
2021-10-14 10:25 ` guangming.cao
(?)
@ 2021-10-26 8:04 ` guangming.cao
-1 siblings, 0 replies; 35+ messages in thread
From: guangming.cao @ 2021-10-26 8:04 UTC (permalink / raw)
To: christian.koenig, sumit.semwal
Cc: dri-devel, guangming.cao, linaro-mm-sig, linux-arm-kernel,
linux-kernel, linux-media, linux-mediatek, matthias.bgg, rdunlap,
wsd_upstream, Guangming Cao
From: Guangming Cao <Guangming.Cao@mediatek.com>
On Thu, 2021-10-14 at 18:25 +0800, guangming.cao@mediatek.com wrote:
> From: Guangming Cao <Guangming.Cao@mediatek.com>
>
> In this patch(https://patchwork.freedesktop.org/patch/310349),
> it add a new IOCTL to support dma-buf user to set debug name.
>
> But it also added a limitation of this IOCTL, it needs the
> attachments of dmabuf should be empty, otherwise it will fail.
>
> For the original series, the idea was that allowing name change
> mid-use could confuse the users about the dma-buf.
> However, the rest of the series also makes sure each dma-buf have a
> unique
> inode(https://patchwork.freedesktop.org/patch/310387/), and any
> accounting
> should probably use that, without relying on the name as much.
>
> So, removing this restriction will let dma-buf userspace users to use
> it
> more comfortably and without any side effect.
>
Hi christian, sumit,
Just a gentle ping for this patch, please kindly let me know your comments about this patch.
Thanks!
Guangming
> Signed-off-by: Guangming Cao <Guangming.Cao@mediatek.com>
> ---
> drivers/dma-buf/dma-buf.c | 17 +++--------------
> 1 file changed, 3 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 511fe0d217a0..5fbb3a2068a3 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -325,10 +325,8 @@ static __poll_t dma_buf_poll(struct file *file,
> poll_table *poll)
>
> /**
> * dma_buf_set_name - Set a name to a specific dma_buf to track the
> usage.
> - * The name of the dma-buf buffer can only be set when the dma-buf
> is not
> - * attached to any devices. It could theoritically support changing
> the
> - * name of the dma-buf if the same piece of memory is used for
> multiple
> - * purpose between different devices.
> + * It could support changing the name of the dma-buf if the same
> + * piece of memory is used for multiple purpose between different
> devices.
> *
> * @dmabuf: [in] dmabuf buffer that will be renamed.
> * @buf: [in] A piece of userspace memory that contains the
> name of
> @@ -341,25 +339,16 @@ static __poll_t dma_buf_poll(struct file *file,
> poll_table *poll)
> static long dma_buf_set_name(struct dma_buf *dmabuf, const char
> __user *buf)
> {
> char *name = strndup_user(buf, DMA_BUF_NAME_LEN);
> - long ret = 0;
>
> if (IS_ERR(name))
> return PTR_ERR(name);
>
> - dma_resv_lock(dmabuf->resv, NULL);
> - if (!list_empty(&dmabuf->attachments)) {
> - ret = -EBUSY;
> - kfree(name);
> - goto out_unlock;
> - }
> spin_lock(&dmabuf->name_lock);
> kfree(dmabuf->name);
> dmabuf->name = name;
> spin_unlock(&dmabuf->name_lock);
>
> -out_unlock:
> - dma_resv_unlock(dmabuf->resv);
> - return ret;
> + return 0;
> }
>
> static long dma_buf_ioctl(struct file *file,
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v3] dma-buf: remove restriction of IOCTL:DMA_BUF_SET_NAME
@ 2021-10-26 8:04 ` guangming.cao
0 siblings, 0 replies; 35+ messages in thread
From: guangming.cao @ 2021-10-26 8:04 UTC (permalink / raw)
To: christian.koenig, sumit.semwal
Cc: dri-devel, guangming.cao, linaro-mm-sig, linux-arm-kernel,
linux-kernel, linux-media, linux-mediatek, matthias.bgg, rdunlap,
wsd_upstream, Guangming Cao
From: Guangming Cao <Guangming.Cao@mediatek.com>
On Thu, 2021-10-14 at 18:25 +0800, guangming.cao@mediatek.com wrote:
> From: Guangming Cao <Guangming.Cao@mediatek.com>
>
> In this patch(https://patchwork.freedesktop.org/patch/310349),
> it add a new IOCTL to support dma-buf user to set debug name.
>
> But it also added a limitation of this IOCTL, it needs the
> attachments of dmabuf should be empty, otherwise it will fail.
>
> For the original series, the idea was that allowing name change
> mid-use could confuse the users about the dma-buf.
> However, the rest of the series also makes sure each dma-buf have a
> unique
> inode(https://patchwork.freedesktop.org/patch/310387/), and any
> accounting
> should probably use that, without relying on the name as much.
>
> So, removing this restriction will let dma-buf userspace users to use
> it
> more comfortably and without any side effect.
>
Hi christian, sumit,
Just a gentle ping for this patch, please kindly let me know your comments about this patch.
Thanks!
Guangming
> Signed-off-by: Guangming Cao <Guangming.Cao@mediatek.com>
> ---
> drivers/dma-buf/dma-buf.c | 17 +++--------------
> 1 file changed, 3 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 511fe0d217a0..5fbb3a2068a3 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -325,10 +325,8 @@ static __poll_t dma_buf_poll(struct file *file,
> poll_table *poll)
>
> /**
> * dma_buf_set_name - Set a name to a specific dma_buf to track the
> usage.
> - * The name of the dma-buf buffer can only be set when the dma-buf
> is not
> - * attached to any devices. It could theoritically support changing
> the
> - * name of the dma-buf if the same piece of memory is used for
> multiple
> - * purpose between different devices.
> + * It could support changing the name of the dma-buf if the same
> + * piece of memory is used for multiple purpose between different
> devices.
> *
> * @dmabuf: [in] dmabuf buffer that will be renamed.
> * @buf: [in] A piece of userspace memory that contains the
> name of
> @@ -341,25 +339,16 @@ static __poll_t dma_buf_poll(struct file *file,
> poll_table *poll)
> static long dma_buf_set_name(struct dma_buf *dmabuf, const char
> __user *buf)
> {
> char *name = strndup_user(buf, DMA_BUF_NAME_LEN);
> - long ret = 0;
>
> if (IS_ERR(name))
> return PTR_ERR(name);
>
> - dma_resv_lock(dmabuf->resv, NULL);
> - if (!list_empty(&dmabuf->attachments)) {
> - ret = -EBUSY;
> - kfree(name);
> - goto out_unlock;
> - }
> spin_lock(&dmabuf->name_lock);
> kfree(dmabuf->name);
> dmabuf->name = name;
> spin_unlock(&dmabuf->name_lock);
>
> -out_unlock:
> - dma_resv_unlock(dmabuf->resv);
> - return ret;
> + return 0;
> }
>
> static long dma_buf_ioctl(struct file *file,
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v3] dma-buf: remove restriction of IOCTL:DMA_BUF_SET_NAME
@ 2021-10-26 8:04 ` guangming.cao
0 siblings, 0 replies; 35+ messages in thread
From: guangming.cao @ 2021-10-26 8:04 UTC (permalink / raw)
To: christian.koenig, sumit.semwal
Cc: dri-devel, guangming.cao, linaro-mm-sig, linux-arm-kernel,
linux-kernel, linux-media, linux-mediatek, matthias.bgg, rdunlap,
wsd_upstream, Guangming Cao
From: Guangming Cao <Guangming.Cao@mediatek.com>
On Thu, 2021-10-14 at 18:25 +0800, guangming.cao@mediatek.com wrote:
> From: Guangming Cao <Guangming.Cao@mediatek.com>
>
> In this patch(https://patchwork.freedesktop.org/patch/310349),
> it add a new IOCTL to support dma-buf user to set debug name.
>
> But it also added a limitation of this IOCTL, it needs the
> attachments of dmabuf should be empty, otherwise it will fail.
>
> For the original series, the idea was that allowing name change
> mid-use could confuse the users about the dma-buf.
> However, the rest of the series also makes sure each dma-buf have a
> unique
> inode(https://patchwork.freedesktop.org/patch/310387/), and any
> accounting
> should probably use that, without relying on the name as much.
>
> So, removing this restriction will let dma-buf userspace users to use
> it
> more comfortably and without any side effect.
>
Hi christian, sumit,
Just a gentle ping for this patch, please kindly let me know your comments about this patch.
Thanks!
Guangming
> Signed-off-by: Guangming Cao <Guangming.Cao@mediatek.com>
> ---
> drivers/dma-buf/dma-buf.c | 17 +++--------------
> 1 file changed, 3 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 511fe0d217a0..5fbb3a2068a3 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -325,10 +325,8 @@ static __poll_t dma_buf_poll(struct file *file,
> poll_table *poll)
>
> /**
> * dma_buf_set_name - Set a name to a specific dma_buf to track the
> usage.
> - * The name of the dma-buf buffer can only be set when the dma-buf
> is not
> - * attached to any devices. It could theoritically support changing
> the
> - * name of the dma-buf if the same piece of memory is used for
> multiple
> - * purpose between different devices.
> + * It could support changing the name of the dma-buf if the same
> + * piece of memory is used for multiple purpose between different
> devices.
> *
> * @dmabuf: [in] dmabuf buffer that will be renamed.
> * @buf: [in] A piece of userspace memory that contains the
> name of
> @@ -341,25 +339,16 @@ static __poll_t dma_buf_poll(struct file *file,
> poll_table *poll)
> static long dma_buf_set_name(struct dma_buf *dmabuf, const char
> __user *buf)
> {
> char *name = strndup_user(buf, DMA_BUF_NAME_LEN);
> - long ret = 0;
>
> if (IS_ERR(name))
> return PTR_ERR(name);
>
> - dma_resv_lock(dmabuf->resv, NULL);
> - if (!list_empty(&dmabuf->attachments)) {
> - ret = -EBUSY;
> - kfree(name);
> - goto out_unlock;
> - }
> spin_lock(&dmabuf->name_lock);
> kfree(dmabuf->name);
> dmabuf->name = name;
> spin_unlock(&dmabuf->name_lock);
>
> -out_unlock:
> - dma_resv_unlock(dmabuf->resv);
> - return ret;
> + return 0;
> }
>
> static long dma_buf_ioctl(struct file *file,
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v3] dma-buf: remove restriction of IOCTL:DMA_BUF_SET_NAME
2021-10-14 10:25 ` guangming.cao
(?)
@ 2021-10-26 11:18 ` Christian König
-1 siblings, 0 replies; 35+ messages in thread
From: Christian König @ 2021-10-26 11:18 UTC (permalink / raw)
To: guangming.cao, sumit.semwal
Cc: dri-devel, linaro-mm-sig, linux-arm-kernel, linux-kernel,
linux-media, linux-mediatek, matthias.bgg, rdunlap, wsd_upstream
Am 14.10.21 um 12:25 schrieb guangming.cao@mediatek.com:
> From: Guangming Cao <Guangming.Cao@mediatek.com>
>
> In this patch(https://patchwork.freedesktop.org/patch/310349),
> it add a new IOCTL to support dma-buf user to set debug name.
>
> But it also added a limitation of this IOCTL, it needs the
> attachments of dmabuf should be empty, otherwise it will fail.
>
> For the original series, the idea was that allowing name change
> mid-use could confuse the users about the dma-buf.
> However, the rest of the series also makes sure each dma-buf have a unique
> inode(https://patchwork.freedesktop.org/patch/310387/), and any accounting
> should probably use that, without relying on the name as much.
>
> So, removing this restriction will let dma-buf userspace users to use it
> more comfortably and without any side effect.
>
> Signed-off-by: Guangming Cao <Guangming.Cao@mediatek.com>
We could now cleanup the return value from dma_buf_set_name() into a
void since that function can't fail any more as far as I can see.
But that isn't mandatory I think, patch is Reviewed-by: Christian König
<christian.koenig@amd.com>
Regards,
Christian.
> ---
> drivers/dma-buf/dma-buf.c | 17 +++--------------
> 1 file changed, 3 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 511fe0d217a0..5fbb3a2068a3 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -325,10 +325,8 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll)
>
> /**
> * dma_buf_set_name - Set a name to a specific dma_buf to track the usage.
> - * The name of the dma-buf buffer can only be set when the dma-buf is not
> - * attached to any devices. It could theoritically support changing the
> - * name of the dma-buf if the same piece of memory is used for multiple
> - * purpose between different devices.
> + * It could support changing the name of the dma-buf if the same
> + * piece of memory is used for multiple purpose between different devices.
> *
> * @dmabuf: [in] dmabuf buffer that will be renamed.
> * @buf: [in] A piece of userspace memory that contains the name of
> @@ -341,25 +339,16 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll)
> static long dma_buf_set_name(struct dma_buf *dmabuf, const char __user *buf)
> {
> char *name = strndup_user(buf, DMA_BUF_NAME_LEN);
> - long ret = 0;
>
> if (IS_ERR(name))
> return PTR_ERR(name);
>
> - dma_resv_lock(dmabuf->resv, NULL);
> - if (!list_empty(&dmabuf->attachments)) {
> - ret = -EBUSY;
> - kfree(name);
> - goto out_unlock;
> - }
> spin_lock(&dmabuf->name_lock);
> kfree(dmabuf->name);
> dmabuf->name = name;
> spin_unlock(&dmabuf->name_lock);
>
> -out_unlock:
> - dma_resv_unlock(dmabuf->resv);
> - return ret;
> + return 0;
> }
>
> static long dma_buf_ioctl(struct file *file,
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v3] dma-buf: remove restriction of IOCTL:DMA_BUF_SET_NAME
@ 2021-10-26 11:18 ` Christian König
0 siblings, 0 replies; 35+ messages in thread
From: Christian König @ 2021-10-26 11:18 UTC (permalink / raw)
To: guangming.cao, sumit.semwal
Cc: dri-devel, linaro-mm-sig, linux-arm-kernel, linux-kernel,
linux-media, linux-mediatek, matthias.bgg, rdunlap, wsd_upstream
Am 14.10.21 um 12:25 schrieb guangming.cao@mediatek.com:
> From: Guangming Cao <Guangming.Cao@mediatek.com>
>
> In this patch(https://patchwork.freedesktop.org/patch/310349),
> it add a new IOCTL to support dma-buf user to set debug name.
>
> But it also added a limitation of this IOCTL, it needs the
> attachments of dmabuf should be empty, otherwise it will fail.
>
> For the original series, the idea was that allowing name change
> mid-use could confuse the users about the dma-buf.
> However, the rest of the series also makes sure each dma-buf have a unique
> inode(https://patchwork.freedesktop.org/patch/310387/), and any accounting
> should probably use that, without relying on the name as much.
>
> So, removing this restriction will let dma-buf userspace users to use it
> more comfortably and without any side effect.
>
> Signed-off-by: Guangming Cao <Guangming.Cao@mediatek.com>
We could now cleanup the return value from dma_buf_set_name() into a
void since that function can't fail any more as far as I can see.
But that isn't mandatory I think, patch is Reviewed-by: Christian König
<christian.koenig@amd.com>
Regards,
Christian.
> ---
> drivers/dma-buf/dma-buf.c | 17 +++--------------
> 1 file changed, 3 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 511fe0d217a0..5fbb3a2068a3 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -325,10 +325,8 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll)
>
> /**
> * dma_buf_set_name - Set a name to a specific dma_buf to track the usage.
> - * The name of the dma-buf buffer can only be set when the dma-buf is not
> - * attached to any devices. It could theoritically support changing the
> - * name of the dma-buf if the same piece of memory is used for multiple
> - * purpose between different devices.
> + * It could support changing the name of the dma-buf if the same
> + * piece of memory is used for multiple purpose between different devices.
> *
> * @dmabuf: [in] dmabuf buffer that will be renamed.
> * @buf: [in] A piece of userspace memory that contains the name of
> @@ -341,25 +339,16 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll)
> static long dma_buf_set_name(struct dma_buf *dmabuf, const char __user *buf)
> {
> char *name = strndup_user(buf, DMA_BUF_NAME_LEN);
> - long ret = 0;
>
> if (IS_ERR(name))
> return PTR_ERR(name);
>
> - dma_resv_lock(dmabuf->resv, NULL);
> - if (!list_empty(&dmabuf->attachments)) {
> - ret = -EBUSY;
> - kfree(name);
> - goto out_unlock;
> - }
> spin_lock(&dmabuf->name_lock);
> kfree(dmabuf->name);
> dmabuf->name = name;
> spin_unlock(&dmabuf->name_lock);
>
> -out_unlock:
> - dma_resv_unlock(dmabuf->resv);
> - return ret;
> + return 0;
> }
>
> static long dma_buf_ioctl(struct file *file,
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v3] dma-buf: remove restriction of IOCTL:DMA_BUF_SET_NAME
@ 2021-10-26 11:18 ` Christian König
0 siblings, 0 replies; 35+ messages in thread
From: Christian König @ 2021-10-26 11:18 UTC (permalink / raw)
To: guangming.cao, sumit.semwal
Cc: dri-devel, linaro-mm-sig, linux-arm-kernel, linux-kernel,
linux-media, linux-mediatek, matthias.bgg, rdunlap, wsd_upstream
Am 14.10.21 um 12:25 schrieb guangming.cao@mediatek.com:
> From: Guangming Cao <Guangming.Cao@mediatek.com>
>
> In this patch(https://patchwork.freedesktop.org/patch/310349),
> it add a new IOCTL to support dma-buf user to set debug name.
>
> But it also added a limitation of this IOCTL, it needs the
> attachments of dmabuf should be empty, otherwise it will fail.
>
> For the original series, the idea was that allowing name change
> mid-use could confuse the users about the dma-buf.
> However, the rest of the series also makes sure each dma-buf have a unique
> inode(https://patchwork.freedesktop.org/patch/310387/), and any accounting
> should probably use that, without relying on the name as much.
>
> So, removing this restriction will let dma-buf userspace users to use it
> more comfortably and without any side effect.
>
> Signed-off-by: Guangming Cao <Guangming.Cao@mediatek.com>
We could now cleanup the return value from dma_buf_set_name() into a
void since that function can't fail any more as far as I can see.
But that isn't mandatory I think, patch is Reviewed-by: Christian König
<christian.koenig@amd.com>
Regards,
Christian.
> ---
> drivers/dma-buf/dma-buf.c | 17 +++--------------
> 1 file changed, 3 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 511fe0d217a0..5fbb3a2068a3 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -325,10 +325,8 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll)
>
> /**
> * dma_buf_set_name - Set a name to a specific dma_buf to track the usage.
> - * The name of the dma-buf buffer can only be set when the dma-buf is not
> - * attached to any devices. It could theoritically support changing the
> - * name of the dma-buf if the same piece of memory is used for multiple
> - * purpose between different devices.
> + * It could support changing the name of the dma-buf if the same
> + * piece of memory is used for multiple purpose between different devices.
> *
> * @dmabuf: [in] dmabuf buffer that will be renamed.
> * @buf: [in] A piece of userspace memory that contains the name of
> @@ -341,25 +339,16 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll)
> static long dma_buf_set_name(struct dma_buf *dmabuf, const char __user *buf)
> {
> char *name = strndup_user(buf, DMA_BUF_NAME_LEN);
> - long ret = 0;
>
> if (IS_ERR(name))
> return PTR_ERR(name);
>
> - dma_resv_lock(dmabuf->resv, NULL);
> - if (!list_empty(&dmabuf->attachments)) {
> - ret = -EBUSY;
> - kfree(name);
> - goto out_unlock;
> - }
> spin_lock(&dmabuf->name_lock);
> kfree(dmabuf->name);
> dmabuf->name = name;
> spin_unlock(&dmabuf->name_lock);
>
> -out_unlock:
> - dma_resv_unlock(dmabuf->resv);
> - return ret;
> + return 0;
> }
>
> static long dma_buf_ioctl(struct file *file,
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v3] dma-buf: remove restriction of IOCTL:DMA_BUF_SET_NAME
2021-10-26 11:18 ` Christian König
(?)
@ 2021-10-26 11:52 ` guangming.cao
-1 siblings, 0 replies; 35+ messages in thread
From: guangming.cao @ 2021-10-26 11:52 UTC (permalink / raw)
To: christian.koenig
Cc: dri-devel, guangming.cao, linaro-mm-sig, linux-arm-kernel,
linux-kernel, linux-media, linux-mediatek, matthias.bgg, rdunlap,
sumit.semwal, wsd_upstream, Guangming Cao
From: Guangming Cao <Guangming.Cao@mediatek.com>
On Tue, 2021-10-26 at 13:18 +0200, Christian König wrote:
> Am 14.10.21 um 12:25 schrieb guangming.cao@mediatek.com:
> > From: Guangming Cao <Guangming.Cao@mediatek.com>
> >
> > In this patch(https://patchwork.freedesktop.org/patch/310349),
> > it add a new IOCTL to support dma-buf user to set debug name.
> >
> > But it also added a limitation of this IOCTL, it needs the
> > attachments of dmabuf should be empty, otherwise it will fail.
> >
> > For the original series, the idea was that allowing name change
> > mid-use could confuse the users about the dma-buf.
> > However, the rest of the series also makes sure each dma-buf have a
> > unique
> > inode(https://patchwork.freedesktop.org/patch/310387/), and any
> > accounting
> > should probably use that, without relying on the name as much.
> >
> > So, removing this restriction will let dma-buf userspace users to
> > use it
> > more comfortably and without any side effect.
> >
> > Signed-off-by: Guangming Cao <Guangming.Cao@mediatek.com>
>
> We could now cleanup the return value from dma_buf_set_name() into a
> void since that function can't fail any more as far as I can see.
>
> But that isn't mandatory I think, patch is Reviewed-by: Christian
> König
> <christian.koenig@amd.com>
>
So, here is no need to check return value of 'strndup_user',
just return without error code if the almost impossible error occurs?
Guangming.
> Regards,
> Christian.
>
> > ---
> > drivers/dma-buf/dma-buf.c | 17 +++--------------
> > 1 file changed, 3 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> > index 511fe0d217a0..5fbb3a2068a3 100644
> > --- a/drivers/dma-buf/dma-buf.c
> > +++ b/drivers/dma-buf/dma-buf.c
> > @@ -325,10 +325,8 @@ static __poll_t dma_buf_poll(struct file
> > *file, poll_table *poll)
> >
> > /**
> > * dma_buf_set_name - Set a name to a specific dma_buf to track
> > the usage.
> > - * The name of the dma-buf buffer can only be set when the dma-buf
> > is not
> > - * attached to any devices. It could theoritically support
> > changing the
> > - * name of the dma-buf if the same piece of memory is used for
> > multiple
> > - * purpose between different devices.
> > + * It could support changing the name of the dma-buf if the same
> > + * piece of memory is used for multiple purpose between different
> > devices.
> > *
> > * @dmabuf: [in] dmabuf buffer that will be renamed.
> > * @buf: [in] A piece of userspace memory that contains
> > the name of
> > @@ -341,25 +339,16 @@ static __poll_t dma_buf_poll(struct file
> > *file, poll_table *poll)
> > static long dma_buf_set_name(struct dma_buf *dmabuf, const char
> > __user *buf)
> > {
> > char *name = strndup_user(buf, DMA_BUF_NAME_LEN);
> > - long ret = 0;
> >
> > if (IS_ERR(name))
> > return PTR_ERR(name);
> >
> > - dma_resv_lock(dmabuf->resv, NULL);
> > - if (!list_empty(&dmabuf->attachments)) {
> > - ret = -EBUSY;
> > - kfree(name);
> > - goto out_unlock;
> > - }
> > spin_lock(&dmabuf->name_lock);
> > kfree(dmabuf->name);
> > dmabuf->name = name;
> > spin_unlock(&dmabuf->name_lock);
> >
> > -out_unlock:
> > - dma_resv_unlock(dmabuf->resv);
> > - return ret;
> > + return 0;
> > }
> >
> > static long dma_buf_ioctl(struct file *file,
>
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v3] dma-buf: remove restriction of IOCTL:DMA_BUF_SET_NAME
@ 2021-10-26 11:52 ` guangming.cao
0 siblings, 0 replies; 35+ messages in thread
From: guangming.cao @ 2021-10-26 11:52 UTC (permalink / raw)
To: christian.koenig
Cc: dri-devel, guangming.cao, linaro-mm-sig, linux-arm-kernel,
linux-kernel, linux-media, linux-mediatek, matthias.bgg, rdunlap,
sumit.semwal, wsd_upstream, Guangming Cao
From: Guangming Cao <Guangming.Cao@mediatek.com>
On Tue, 2021-10-26 at 13:18 +0200, Christian König wrote:
> Am 14.10.21 um 12:25 schrieb guangming.cao@mediatek.com:
> > From: Guangming Cao <Guangming.Cao@mediatek.com>
> >
> > In this patch(https://patchwork.freedesktop.org/patch/310349),
> > it add a new IOCTL to support dma-buf user to set debug name.
> >
> > But it also added a limitation of this IOCTL, it needs the
> > attachments of dmabuf should be empty, otherwise it will fail.
> >
> > For the original series, the idea was that allowing name change
> > mid-use could confuse the users about the dma-buf.
> > However, the rest of the series also makes sure each dma-buf have a
> > unique
> > inode(https://patchwork.freedesktop.org/patch/310387/), and any
> > accounting
> > should probably use that, without relying on the name as much.
> >
> > So, removing this restriction will let dma-buf userspace users to
> > use it
> > more comfortably and without any side effect.
> >
> > Signed-off-by: Guangming Cao <Guangming.Cao@mediatek.com>
>
> We could now cleanup the return value from dma_buf_set_name() into a
> void since that function can't fail any more as far as I can see.
>
> But that isn't mandatory I think, patch is Reviewed-by: Christian
> König
> <christian.koenig@amd.com>
>
So, here is no need to check return value of 'strndup_user',
just return without error code if the almost impossible error occurs?
Guangming.
> Regards,
> Christian.
>
> > ---
> > drivers/dma-buf/dma-buf.c | 17 +++--------------
> > 1 file changed, 3 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> > index 511fe0d217a0..5fbb3a2068a3 100644
> > --- a/drivers/dma-buf/dma-buf.c
> > +++ b/drivers/dma-buf/dma-buf.c
> > @@ -325,10 +325,8 @@ static __poll_t dma_buf_poll(struct file
> > *file, poll_table *poll)
> >
> > /**
> > * dma_buf_set_name - Set a name to a specific dma_buf to track
> > the usage.
> > - * The name of the dma-buf buffer can only be set when the dma-buf
> > is not
> > - * attached to any devices. It could theoritically support
> > changing the
> > - * name of the dma-buf if the same piece of memory is used for
> > multiple
> > - * purpose between different devices.
> > + * It could support changing the name of the dma-buf if the same
> > + * piece of memory is used for multiple purpose between different
> > devices.
> > *
> > * @dmabuf: [in] dmabuf buffer that will be renamed.
> > * @buf: [in] A piece of userspace memory that contains
> > the name of
> > @@ -341,25 +339,16 @@ static __poll_t dma_buf_poll(struct file
> > *file, poll_table *poll)
> > static long dma_buf_set_name(struct dma_buf *dmabuf, const char
> > __user *buf)
> > {
> > char *name = strndup_user(buf, DMA_BUF_NAME_LEN);
> > - long ret = 0;
> >
> > if (IS_ERR(name))
> > return PTR_ERR(name);
> >
> > - dma_resv_lock(dmabuf->resv, NULL);
> > - if (!list_empty(&dmabuf->attachments)) {
> > - ret = -EBUSY;
> > - kfree(name);
> > - goto out_unlock;
> > - }
> > spin_lock(&dmabuf->name_lock);
> > kfree(dmabuf->name);
> > dmabuf->name = name;
> > spin_unlock(&dmabuf->name_lock);
> >
> > -out_unlock:
> > - dma_resv_unlock(dmabuf->resv);
> > - return ret;
> > + return 0;
> > }
> >
> > static long dma_buf_ioctl(struct file *file,
>
>
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v3] dma-buf: remove restriction of IOCTL:DMA_BUF_SET_NAME
@ 2021-10-26 11:52 ` guangming.cao
0 siblings, 0 replies; 35+ messages in thread
From: guangming.cao @ 2021-10-26 11:52 UTC (permalink / raw)
To: christian.koenig
Cc: dri-devel, guangming.cao, linaro-mm-sig, linux-arm-kernel,
linux-kernel, linux-media, linux-mediatek, matthias.bgg, rdunlap,
sumit.semwal, wsd_upstream, Guangming Cao
From: Guangming Cao <Guangming.Cao@mediatek.com>
On Tue, 2021-10-26 at 13:18 +0200, Christian König wrote:
> Am 14.10.21 um 12:25 schrieb guangming.cao@mediatek.com:
> > From: Guangming Cao <Guangming.Cao@mediatek.com>
> >
> > In this patch(https://patchwork.freedesktop.org/patch/310349),
> > it add a new IOCTL to support dma-buf user to set debug name.
> >
> > But it also added a limitation of this IOCTL, it needs the
> > attachments of dmabuf should be empty, otherwise it will fail.
> >
> > For the original series, the idea was that allowing name change
> > mid-use could confuse the users about the dma-buf.
> > However, the rest of the series also makes sure each dma-buf have a
> > unique
> > inode(https://patchwork.freedesktop.org/patch/310387/), and any
> > accounting
> > should probably use that, without relying on the name as much.
> >
> > So, removing this restriction will let dma-buf userspace users to
> > use it
> > more comfortably and without any side effect.
> >
> > Signed-off-by: Guangming Cao <Guangming.Cao@mediatek.com>
>
> We could now cleanup the return value from dma_buf_set_name() into a
> void since that function can't fail any more as far as I can see.
>
> But that isn't mandatory I think, patch is Reviewed-by: Christian
> König
> <christian.koenig@amd.com>
>
So, here is no need to check return value of 'strndup_user',
just return without error code if the almost impossible error occurs?
Guangming.
> Regards,
> Christian.
>
> > ---
> > drivers/dma-buf/dma-buf.c | 17 +++--------------
> > 1 file changed, 3 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> > index 511fe0d217a0..5fbb3a2068a3 100644
> > --- a/drivers/dma-buf/dma-buf.c
> > +++ b/drivers/dma-buf/dma-buf.c
> > @@ -325,10 +325,8 @@ static __poll_t dma_buf_poll(struct file
> > *file, poll_table *poll)
> >
> > /**
> > * dma_buf_set_name - Set a name to a specific dma_buf to track
> > the usage.
> > - * The name of the dma-buf buffer can only be set when the dma-buf
> > is not
> > - * attached to any devices. It could theoritically support
> > changing the
> > - * name of the dma-buf if the same piece of memory is used for
> > multiple
> > - * purpose between different devices.
> > + * It could support changing the name of the dma-buf if the same
> > + * piece of memory is used for multiple purpose between different
> > devices.
> > *
> > * @dmabuf: [in] dmabuf buffer that will be renamed.
> > * @buf: [in] A piece of userspace memory that contains
> > the name of
> > @@ -341,25 +339,16 @@ static __poll_t dma_buf_poll(struct file
> > *file, poll_table *poll)
> > static long dma_buf_set_name(struct dma_buf *dmabuf, const char
> > __user *buf)
> > {
> > char *name = strndup_user(buf, DMA_BUF_NAME_LEN);
> > - long ret = 0;
> >
> > if (IS_ERR(name))
> > return PTR_ERR(name);
> >
> > - dma_resv_lock(dmabuf->resv, NULL);
> > - if (!list_empty(&dmabuf->attachments)) {
> > - ret = -EBUSY;
> > - kfree(name);
> > - goto out_unlock;
> > - }
> > spin_lock(&dmabuf->name_lock);
> > kfree(dmabuf->name);
> > dmabuf->name = name;
> > spin_unlock(&dmabuf->name_lock);
> >
> > -out_unlock:
> > - dma_resv_unlock(dmabuf->resv);
> > - return ret;
> > + return 0;
> > }
> >
> > static long dma_buf_ioctl(struct file *file,
>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v3] dma-buf: remove restriction of IOCTL:DMA_BUF_SET_NAME
2021-10-26 11:52 ` guangming.cao
(?)
@ 2021-10-28 11:35 ` Christian König
-1 siblings, 0 replies; 35+ messages in thread
From: Christian König @ 2021-10-28 11:35 UTC (permalink / raw)
To: guangming.cao
Cc: dri-devel, linaro-mm-sig, linux-arm-kernel, linux-kernel,
linux-media, linux-mediatek, matthias.bgg, rdunlap, sumit.semwal,
wsd_upstream
Am 26.10.21 um 13:52 schrieb guangming.cao@mediatek.com:
> From: Guangming Cao <Guangming.Cao@mediatek.com>
>
> On Tue, 2021-10-26 at 13:18 +0200, Christian König wrote:
>> Am 14.10.21 um 12:25 schrieb guangming.cao@mediatek.com:
>>> From: Guangming Cao <Guangming.Cao@mediatek.com>
>>>
>>> In this patch(https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.freedesktop.org%2Fpatch%2F310349&data=04%7C01%7Cchristian.koenig%40amd.com%7C6652f423b96547b8321308d998772121%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637708459727236977%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=4EFg60sCxQzNuMKB0VjhbpvoeGlcAbofErMtcjPtIfw%3D&reserved=0),
>>> it add a new IOCTL to support dma-buf user to set debug name.
>>>
>>> But it also added a limitation of this IOCTL, it needs the
>>> attachments of dmabuf should be empty, otherwise it will fail.
>>>
>>> For the original series, the idea was that allowing name change
>>> mid-use could confuse the users about the dma-buf.
>>> However, the rest of the series also makes sure each dma-buf have a
>>> unique
>>> inode(https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.freedesktop.org%2Fpatch%2F310387%2F&data=04%7C01%7Cchristian.koenig%40amd.com%7C6652f423b96547b8321308d998772121%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637708459727236977%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=c%2Bbso%2BSxiPjwlv%2B9tAzA4x0gf5UktREhSiAsODFkihk%3D&reserved=0), and any
>>> accounting
>>> should probably use that, without relying on the name as much.
>>>
>>> So, removing this restriction will let dma-buf userspace users to
>>> use it
>>> more comfortably and without any side effect.
>>>
>>> Signed-off-by: Guangming Cao <Guangming.Cao@mediatek.com>
>> We could now cleanup the return value from dma_buf_set_name() into a
>> void since that function can't fail any more as far as I can see.
>>
>> But that isn't mandatory I think, patch is Reviewed-by: Christian
>> König
>> <christian.koenig@amd.com>
>>
> So, here is no need to check return value of 'strndup_user',
> just return without error code if the almost impossible error occurs?
Good point, totally missed that one.
In that case I'm going to push the patch to drm-misc-next as is.
Regards,
Christian.
>
> Guangming.
>
>> Regards,
>> Christian.
>>
>>> ---
>>> drivers/dma-buf/dma-buf.c | 17 +++--------------
>>> 1 file changed, 3 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
>>> index 511fe0d217a0..5fbb3a2068a3 100644
>>> --- a/drivers/dma-buf/dma-buf.c
>>> +++ b/drivers/dma-buf/dma-buf.c
>>> @@ -325,10 +325,8 @@ static __poll_t dma_buf_poll(struct file
>>> *file, poll_table *poll)
>>>
>>> /**
>>> * dma_buf_set_name - Set a name to a specific dma_buf to track
>>> the usage.
>>> - * The name of the dma-buf buffer can only be set when the dma-buf
>>> is not
>>> - * attached to any devices. It could theoritically support
>>> changing the
>>> - * name of the dma-buf if the same piece of memory is used for
>>> multiple
>>> - * purpose between different devices.
>>> + * It could support changing the name of the dma-buf if the same
>>> + * piece of memory is used for multiple purpose between different
>>> devices.
>>> *
>>> * @dmabuf: [in] dmabuf buffer that will be renamed.
>>> * @buf: [in] A piece of userspace memory that contains
>>> the name of
>>> @@ -341,25 +339,16 @@ static __poll_t dma_buf_poll(struct file
>>> *file, poll_table *poll)
>>> static long dma_buf_set_name(struct dma_buf *dmabuf, const char
>>> __user *buf)
>>> {
>>> char *name = strndup_user(buf, DMA_BUF_NAME_LEN);
>>> - long ret = 0;
>>>
>>> if (IS_ERR(name))
>>> return PTR_ERR(name);
>>>
>>> - dma_resv_lock(dmabuf->resv, NULL);
>>> - if (!list_empty(&dmabuf->attachments)) {
>>> - ret = -EBUSY;
>>> - kfree(name);
>>> - goto out_unlock;
>>> - }
>>> spin_lock(&dmabuf->name_lock);
>>> kfree(dmabuf->name);
>>> dmabuf->name = name;
>>> spin_unlock(&dmabuf->name_lock);
>>>
>>> -out_unlock:
>>> - dma_resv_unlock(dmabuf->resv);
>>> - return ret;
>>> + return 0;
>>> }
>>>
>>> static long dma_buf_ioctl(struct file *file,
>>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v3] dma-buf: remove restriction of IOCTL:DMA_BUF_SET_NAME
@ 2021-10-28 11:35 ` Christian König
0 siblings, 0 replies; 35+ messages in thread
From: Christian König @ 2021-10-28 11:35 UTC (permalink / raw)
To: guangming.cao
Cc: dri-devel, linaro-mm-sig, linux-arm-kernel, linux-kernel,
linux-media, linux-mediatek, matthias.bgg, rdunlap, sumit.semwal,
wsd_upstream
Am 26.10.21 um 13:52 schrieb guangming.cao@mediatek.com:
> From: Guangming Cao <Guangming.Cao@mediatek.com>
>
> On Tue, 2021-10-26 at 13:18 +0200, Christian König wrote:
>> Am 14.10.21 um 12:25 schrieb guangming.cao@mediatek.com:
>>> From: Guangming Cao <Guangming.Cao@mediatek.com>
>>>
>>> In this patch(https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.freedesktop.org%2Fpatch%2F310349&data=04%7C01%7Cchristian.koenig%40amd.com%7C6652f423b96547b8321308d998772121%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637708459727236977%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=4EFg60sCxQzNuMKB0VjhbpvoeGlcAbofErMtcjPtIfw%3D&reserved=0),
>>> it add a new IOCTL to support dma-buf user to set debug name.
>>>
>>> But it also added a limitation of this IOCTL, it needs the
>>> attachments of dmabuf should be empty, otherwise it will fail.
>>>
>>> For the original series, the idea was that allowing name change
>>> mid-use could confuse the users about the dma-buf.
>>> However, the rest of the series also makes sure each dma-buf have a
>>> unique
>>> inode(https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.freedesktop.org%2Fpatch%2F310387%2F&data=04%7C01%7Cchristian.koenig%40amd.com%7C6652f423b96547b8321308d998772121%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637708459727236977%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=c%2Bbso%2BSxiPjwlv%2B9tAzA4x0gf5UktREhSiAsODFkihk%3D&reserved=0), and any
>>> accounting
>>> should probably use that, without relying on the name as much.
>>>
>>> So, removing this restriction will let dma-buf userspace users to
>>> use it
>>> more comfortably and without any side effect.
>>>
>>> Signed-off-by: Guangming Cao <Guangming.Cao@mediatek.com>
>> We could now cleanup the return value from dma_buf_set_name() into a
>> void since that function can't fail any more as far as I can see.
>>
>> But that isn't mandatory I think, patch is Reviewed-by: Christian
>> König
>> <christian.koenig@amd.com>
>>
> So, here is no need to check return value of 'strndup_user',
> just return without error code if the almost impossible error occurs?
Good point, totally missed that one.
In that case I'm going to push the patch to drm-misc-next as is.
Regards,
Christian.
>
> Guangming.
>
>> Regards,
>> Christian.
>>
>>> ---
>>> drivers/dma-buf/dma-buf.c | 17 +++--------------
>>> 1 file changed, 3 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
>>> index 511fe0d217a0..5fbb3a2068a3 100644
>>> --- a/drivers/dma-buf/dma-buf.c
>>> +++ b/drivers/dma-buf/dma-buf.c
>>> @@ -325,10 +325,8 @@ static __poll_t dma_buf_poll(struct file
>>> *file, poll_table *poll)
>>>
>>> /**
>>> * dma_buf_set_name - Set a name to a specific dma_buf to track
>>> the usage.
>>> - * The name of the dma-buf buffer can only be set when the dma-buf
>>> is not
>>> - * attached to any devices. It could theoritically support
>>> changing the
>>> - * name of the dma-buf if the same piece of memory is used for
>>> multiple
>>> - * purpose between different devices.
>>> + * It could support changing the name of the dma-buf if the same
>>> + * piece of memory is used for multiple purpose between different
>>> devices.
>>> *
>>> * @dmabuf: [in] dmabuf buffer that will be renamed.
>>> * @buf: [in] A piece of userspace memory that contains
>>> the name of
>>> @@ -341,25 +339,16 @@ static __poll_t dma_buf_poll(struct file
>>> *file, poll_table *poll)
>>> static long dma_buf_set_name(struct dma_buf *dmabuf, const char
>>> __user *buf)
>>> {
>>> char *name = strndup_user(buf, DMA_BUF_NAME_LEN);
>>> - long ret = 0;
>>>
>>> if (IS_ERR(name))
>>> return PTR_ERR(name);
>>>
>>> - dma_resv_lock(dmabuf->resv, NULL);
>>> - if (!list_empty(&dmabuf->attachments)) {
>>> - ret = -EBUSY;
>>> - kfree(name);
>>> - goto out_unlock;
>>> - }
>>> spin_lock(&dmabuf->name_lock);
>>> kfree(dmabuf->name);
>>> dmabuf->name = name;
>>> spin_unlock(&dmabuf->name_lock);
>>>
>>> -out_unlock:
>>> - dma_resv_unlock(dmabuf->resv);
>>> - return ret;
>>> + return 0;
>>> }
>>>
>>> static long dma_buf_ioctl(struct file *file,
>>
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v3] dma-buf: remove restriction of IOCTL:DMA_BUF_SET_NAME
@ 2021-10-28 11:35 ` Christian König
0 siblings, 0 replies; 35+ messages in thread
From: Christian König @ 2021-10-28 11:35 UTC (permalink / raw)
To: guangming.cao
Cc: dri-devel, linaro-mm-sig, linux-arm-kernel, linux-kernel,
linux-media, linux-mediatek, matthias.bgg, rdunlap, sumit.semwal,
wsd_upstream
Am 26.10.21 um 13:52 schrieb guangming.cao@mediatek.com:
> From: Guangming Cao <Guangming.Cao@mediatek.com>
>
> On Tue, 2021-10-26 at 13:18 +0200, Christian König wrote:
>> Am 14.10.21 um 12:25 schrieb guangming.cao@mediatek.com:
>>> From: Guangming Cao <Guangming.Cao@mediatek.com>
>>>
>>> In this patch(https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.freedesktop.org%2Fpatch%2F310349&data=04%7C01%7Cchristian.koenig%40amd.com%7C6652f423b96547b8321308d998772121%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637708459727236977%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=4EFg60sCxQzNuMKB0VjhbpvoeGlcAbofErMtcjPtIfw%3D&reserved=0),
>>> it add a new IOCTL to support dma-buf user to set debug name.
>>>
>>> But it also added a limitation of this IOCTL, it needs the
>>> attachments of dmabuf should be empty, otherwise it will fail.
>>>
>>> For the original series, the idea was that allowing name change
>>> mid-use could confuse the users about the dma-buf.
>>> However, the rest of the series also makes sure each dma-buf have a
>>> unique
>>> inode(https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.freedesktop.org%2Fpatch%2F310387%2F&data=04%7C01%7Cchristian.koenig%40amd.com%7C6652f423b96547b8321308d998772121%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637708459727236977%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=c%2Bbso%2BSxiPjwlv%2B9tAzA4x0gf5UktREhSiAsODFkihk%3D&reserved=0), and any
>>> accounting
>>> should probably use that, without relying on the name as much.
>>>
>>> So, removing this restriction will let dma-buf userspace users to
>>> use it
>>> more comfortably and without any side effect.
>>>
>>> Signed-off-by: Guangming Cao <Guangming.Cao@mediatek.com>
>> We could now cleanup the return value from dma_buf_set_name() into a
>> void since that function can't fail any more as far as I can see.
>>
>> But that isn't mandatory I think, patch is Reviewed-by: Christian
>> König
>> <christian.koenig@amd.com>
>>
> So, here is no need to check return value of 'strndup_user',
> just return without error code if the almost impossible error occurs?
Good point, totally missed that one.
In that case I'm going to push the patch to drm-misc-next as is.
Regards,
Christian.
>
> Guangming.
>
>> Regards,
>> Christian.
>>
>>> ---
>>> drivers/dma-buf/dma-buf.c | 17 +++--------------
>>> 1 file changed, 3 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
>>> index 511fe0d217a0..5fbb3a2068a3 100644
>>> --- a/drivers/dma-buf/dma-buf.c
>>> +++ b/drivers/dma-buf/dma-buf.c
>>> @@ -325,10 +325,8 @@ static __poll_t dma_buf_poll(struct file
>>> *file, poll_table *poll)
>>>
>>> /**
>>> * dma_buf_set_name - Set a name to a specific dma_buf to track
>>> the usage.
>>> - * The name of the dma-buf buffer can only be set when the dma-buf
>>> is not
>>> - * attached to any devices. It could theoritically support
>>> changing the
>>> - * name of the dma-buf if the same piece of memory is used for
>>> multiple
>>> - * purpose between different devices.
>>> + * It could support changing the name of the dma-buf if the same
>>> + * piece of memory is used for multiple purpose between different
>>> devices.
>>> *
>>> * @dmabuf: [in] dmabuf buffer that will be renamed.
>>> * @buf: [in] A piece of userspace memory that contains
>>> the name of
>>> @@ -341,25 +339,16 @@ static __poll_t dma_buf_poll(struct file
>>> *file, poll_table *poll)
>>> static long dma_buf_set_name(struct dma_buf *dmabuf, const char
>>> __user *buf)
>>> {
>>> char *name = strndup_user(buf, DMA_BUF_NAME_LEN);
>>> - long ret = 0;
>>>
>>> if (IS_ERR(name))
>>> return PTR_ERR(name);
>>>
>>> - dma_resv_lock(dmabuf->resv, NULL);
>>> - if (!list_empty(&dmabuf->attachments)) {
>>> - ret = -EBUSY;
>>> - kfree(name);
>>> - goto out_unlock;
>>> - }
>>> spin_lock(&dmabuf->name_lock);
>>> kfree(dmabuf->name);
>>> dmabuf->name = name;
>>> spin_unlock(&dmabuf->name_lock);
>>>
>>> -out_unlock:
>>> - dma_resv_unlock(dmabuf->resv);
>>> - return ret;
>>> + return 0;
>>> }
>>>
>>> static long dma_buf_ioctl(struct file *file,
>>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 35+ messages in thread
end of thread, other threads:[~2021-10-28 11:38 UTC | newest]
Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-09 2:47 [PATCH] dma-buf: remove restriction of IOCTL:DMA_BUF_SET_NAME guangming.cao
2021-10-09 2:47 ` guangming.cao
2021-10-09 2:47 ` guangming.cao
2021-10-09 4:41 ` Randy Dunlap
2021-10-09 4:41 ` Randy Dunlap
2021-10-09 4:41 ` Randy Dunlap
2021-10-09 5:55 ` [PATCH v2] " guangming.cao
2021-10-09 5:55 ` guangming.cao
2021-10-09 5:55 ` guangming.cao
2021-10-11 8:20 ` Christian König
2021-10-11 8:20 ` Christian König
2021-10-11 8:20 ` Christian König
2021-10-12 8:41 ` guangming.cao
2021-10-12 8:41 ` guangming.cao
2021-10-12 8:41 ` guangming.cao
2021-10-12 23:56 ` Sumit Semwal
2021-10-13 12:20 ` Christian König
2021-10-14 10:25 ` [PATCH v3] " guangming.cao
2021-10-14 10:25 ` guangming.cao
2021-10-14 10:25 ` guangming.cao
2021-10-14 10:35 ` guangming.cao
2021-10-14 10:35 ` guangming.cao
2021-10-14 10:35 ` guangming.cao
2021-10-26 8:04 ` guangming.cao
2021-10-26 8:04 ` guangming.cao
2021-10-26 8:04 ` guangming.cao
2021-10-26 11:18 ` Christian König
2021-10-26 11:18 ` Christian König
2021-10-26 11:18 ` Christian König
2021-10-26 11:52 ` guangming.cao
2021-10-26 11:52 ` guangming.cao
2021-10-26 11:52 ` guangming.cao
2021-10-28 11:35 ` Christian König
2021-10-28 11:35 ` Christian König
2021-10-28 11:35 ` Christian König
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.