Re: [RFC][PATCH 1/2] dma-buf: add importer private data to attachment

2013-06-09 Thread 김승우 


On 2013년 06월 07일 20:24, Maarten Lankhorst wrote:
> Op 07-06-13 04:32, 김승우 schreef:
>> Hello Maarten,
>>
>> On 2013년 06월 05일 22:23, Maarten Lankhorst wrote:
>>> Op 31-05-13 10:54, Seung-Woo Kim schreef:
 dma-buf attachment has only exporter private data, but importer private 
 data
 can be useful for importer especially to re-import the same dma-buf.
 To use importer private data in attachment of the device, the function to
 search attachment in the attachment list of dma-buf is also added.

 Signed-off-by: Seung-Woo Kim 
 ---
  drivers/base/dma-buf.c  |   31 +++
  include/linux/dma-buf.h |4 
  2 files changed, 35 insertions(+), 0 deletions(-)

 diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
 index 08fe897..a1eaaf2 100644
 --- a/drivers/base/dma-buf.c
 +++ b/drivers/base/dma-buf.c
 @@ -259,6 +259,37 @@ err_attach:
  EXPORT_SYMBOL_GPL(dma_buf_attach);
  
  /**
 + * dma_buf_get_attachment - Get attachment with the device from dma_buf's
 + * attachments list
 + * @dmabuf:   [in]buffer to find device from.
 + * @dev:  [in]device to be found.
 + *
 + * Returns struct dma_buf_attachment * attaching the device; may return
 + * negative error codes.
 + *
 + */
 +struct dma_buf_attachment *dma_buf_get_attachment(struct dma_buf *dmabuf,
 +struct device *dev)
 +{
 +  struct dma_buf_attachment *attach;
 +
 +  if (WARN_ON(!dmabuf || !dev))
 +  return ERR_PTR(-EINVAL);
 +
 +  mutex_lock(&dmabuf->lock);
 +  list_for_each_entry(attach, &dmabuf->attachments, node) {
 +  if (attach->dev == dev) {
 +  mutex_unlock(&dmabuf->lock);
 +  return attach;
 +  }
 +  }
 +  mutex_unlock(&dmabuf->lock);
 +
 +  return ERR_PTR(-ENODEV);
 +}
 +EXPORT_SYMBOL_GPL(dma_buf_get_attachment);
>>> NAK in any form..
>>>
>>> Spot the race condition between dma_buf_get_attachment and 
>>> dma_buf_attach
>> Both dma_buf_get_attachment and dma_buf_attach has mutet with
>> dmabuf->lock, and dma_buf_get_attachment is used for get attachment from
>> same device before calling dma_buf_attach.
> 
> hint: what happens if 2 threads do this:
> 
> if (IS_ERR(attach = dma_buf_get_attachment(buf, dev)))
> attach = dma_buf_attach(buf, dev);
> 
> There really is no correct usecase for this kind of thing, so please don't do 
> it.

Ok, dma_buf_get_attachment can not prevent attachments from one device.

Anyway, do you think that importer side private data, not to allow
re-import one dma-buf to a device, has some advantage?
If that, I'll add some check_and_attach function, otherwise, I'll find
other way.

Thanks and Regards,
- Seung-Woo Kim

> 
>> While, dma_buf_detach can removes attachment because it does not have
>> ref count. So importer should check ref count in its importer private
>> data before calling dma_buf_detach if the importer want to use
>> dma_buf_get_attachment.
>>
>> And dma_buf_get_attachment is for the importer, so exporter attach and
>> detach callback operation should not call it as like exporter detach
>> callback operation should not call dma_buf_attach if you mean this kind
>> of race.
>>
>> If you have other considerations here, please describe more specifically.
>>
>> Thanks and Best Regards,
>> - Seung-Woo Kim
>>
>>> ~Maarten
>>>
>>>
> 
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 
Seung-Woo Kim
Samsung Software R&D Center
--

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC][PATCH 1/2] dma-buf: add importer private data to attachment

2013-06-07 Thread Maarten Lankhorst 
Op 07-06-13 04:32, 김승우 schreef:
> Hello Maarten,
>
> On 2013년 06월 05일 22:23, Maarten Lankhorst wrote:
>> Op 31-05-13 10:54, Seung-Woo Kim schreef:
>>> dma-buf attachment has only exporter private data, but importer private data
>>> can be useful for importer especially to re-import the same dma-buf.
>>> To use importer private data in attachment of the device, the function to
>>> search attachment in the attachment list of dma-buf is also added.
>>>
>>> Signed-off-by: Seung-Woo Kim 
>>> ---
>>>  drivers/base/dma-buf.c  |   31 +++
>>>  include/linux/dma-buf.h |4 
>>>  2 files changed, 35 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
>>> index 08fe897..a1eaaf2 100644
>>> --- a/drivers/base/dma-buf.c
>>> +++ b/drivers/base/dma-buf.c
>>> @@ -259,6 +259,37 @@ err_attach:
>>>  EXPORT_SYMBOL_GPL(dma_buf_attach);
>>>  
>>>  /**
>>> + * dma_buf_get_attachment - Get attachment with the device from dma_buf's
>>> + * attachments list
>>> + * @dmabuf:[in]buffer to find device from.
>>> + * @dev:   [in]device to be found.
>>> + *
>>> + * Returns struct dma_buf_attachment * attaching the device; may return
>>> + * negative error codes.
>>> + *
>>> + */
>>> +struct dma_buf_attachment *dma_buf_get_attachment(struct dma_buf *dmabuf,
>>> + struct device *dev)
>>> +{
>>> +   struct dma_buf_attachment *attach;
>>> +
>>> +   if (WARN_ON(!dmabuf || !dev))
>>> +   return ERR_PTR(-EINVAL);
>>> +
>>> +   mutex_lock(&dmabuf->lock);
>>> +   list_for_each_entry(attach, &dmabuf->attachments, node) {
>>> +   if (attach->dev == dev) {
>>> +   mutex_unlock(&dmabuf->lock);
>>> +   return attach;
>>> +   }
>>> +   }
>>> +   mutex_unlock(&dmabuf->lock);
>>> +
>>> +   return ERR_PTR(-ENODEV);
>>> +}
>>> +EXPORT_SYMBOL_GPL(dma_buf_get_attachment);
>> NAK in any form..
>>
>> Spot the race condition between dma_buf_get_attachment and dma_buf_attach
> Both dma_buf_get_attachment and dma_buf_attach has mutet with
> dmabuf->lock, and dma_buf_get_attachment is used for get attachment from
> same device before calling dma_buf_attach.

hint: what happens if 2 threads do this:

if (IS_ERR(attach = dma_buf_get_attachment(buf, dev)))
attach = dma_buf_attach(buf, dev);

There really is no correct usecase for this kind of thing, so please don't do 
it.

> While, dma_buf_detach can removes attachment because it does not have
> ref count. So importer should check ref count in its importer private
> data before calling dma_buf_detach if the importer want to use
> dma_buf_get_attachment.
>
> And dma_buf_get_attachment is for the importer, so exporter attach and
> detach callback operation should not call it as like exporter detach
> callback operation should not call dma_buf_attach if you mean this kind
> of race.
>
> If you have other considerations here, please describe more specifically.
>
> Thanks and Best Regards,
> - Seung-Woo Kim
>
>> ~Maarten
>>
>>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC][PATCH 1/2] dma-buf: add importer private data to attachment

2013-06-06 Thread 김승우 
Hello Maarten,

On 2013년 06월 05일 22:23, Maarten Lankhorst wrote:
> Op 31-05-13 10:54, Seung-Woo Kim schreef:
>> dma-buf attachment has only exporter private data, but importer private data
>> can be useful for importer especially to re-import the same dma-buf.
>> To use importer private data in attachment of the device, the function to
>> search attachment in the attachment list of dma-buf is also added.
>>
>> Signed-off-by: Seung-Woo Kim 
>> ---
>>  drivers/base/dma-buf.c  |   31 +++
>>  include/linux/dma-buf.h |4 
>>  2 files changed, 35 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
>> index 08fe897..a1eaaf2 100644
>> --- a/drivers/base/dma-buf.c
>> +++ b/drivers/base/dma-buf.c
>> @@ -259,6 +259,37 @@ err_attach:
>>  EXPORT_SYMBOL_GPL(dma_buf_attach);
>>  
>>  /**
>> + * dma_buf_get_attachment - Get attachment with the device from dma_buf's
>> + * attachments list
>> + * @dmabuf: [in]buffer to find device from.
>> + * @dev:[in]device to be found.
>> + *
>> + * Returns struct dma_buf_attachment * attaching the device; may return
>> + * negative error codes.
>> + *
>> + */
>> +struct dma_buf_attachment *dma_buf_get_attachment(struct dma_buf *dmabuf,
>> +  struct device *dev)
>> +{
>> +struct dma_buf_attachment *attach;
>> +
>> +if (WARN_ON(!dmabuf || !dev))
>> +return ERR_PTR(-EINVAL);
>> +
>> +mutex_lock(&dmabuf->lock);
>> +list_for_each_entry(attach, &dmabuf->attachments, node) {
>> +if (attach->dev == dev) {
>> +mutex_unlock(&dmabuf->lock);
>> +return attach;
>> +}
>> +}
>> +mutex_unlock(&dmabuf->lock);
>> +
>> +return ERR_PTR(-ENODEV);
>> +}
>> +EXPORT_SYMBOL_GPL(dma_buf_get_attachment);
> NAK in any form..
> 
> Spot the race condition between dma_buf_get_attachment and dma_buf_attach

Both dma_buf_get_attachment and dma_buf_attach has mutet with
dmabuf->lock, and dma_buf_get_attachment is used for get attachment from
same device before calling dma_buf_attach.

While, dma_buf_detach can removes attachment because it does not have
ref count. So importer should check ref count in its importer private
data before calling dma_buf_detach if the importer want to use
dma_buf_get_attachment.

And dma_buf_get_attachment is for the importer, so exporter attach and
detach callback operation should not call it as like exporter detach
callback operation should not call dma_buf_attach if you mean this kind
of race.

If you have other considerations here, please describe more specifically.

Thanks and Best Regards,
- Seung-Woo Kim

> 
> ~Maarten
> 
> 

-- 
Seung-Woo Kim
Samsung Software R&D Center
--

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC][PATCH 1/2] dma-buf: add importer private data to attachment

2013-06-05 Thread Maarten Lankhorst 
Op 31-05-13 10:54, Seung-Woo Kim schreef:
> dma-buf attachment has only exporter private data, but importer private data
> can be useful for importer especially to re-import the same dma-buf.
> To use importer private data in attachment of the device, the function to
> search attachment in the attachment list of dma-buf is also added.
>
> Signed-off-by: Seung-Woo Kim 
> ---
>  drivers/base/dma-buf.c  |   31 +++
>  include/linux/dma-buf.h |4 
>  2 files changed, 35 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
> index 08fe897..a1eaaf2 100644
> --- a/drivers/base/dma-buf.c
> +++ b/drivers/base/dma-buf.c
> @@ -259,6 +259,37 @@ err_attach:
>  EXPORT_SYMBOL_GPL(dma_buf_attach);
>  
>  /**
> + * dma_buf_get_attachment - Get attachment with the device from dma_buf's
> + * attachments list
> + * @dmabuf:  [in]buffer to find device from.
> + * @dev: [in]device to be found.
> + *
> + * Returns struct dma_buf_attachment * attaching the device; may return
> + * negative error codes.
> + *
> + */
> +struct dma_buf_attachment *dma_buf_get_attachment(struct dma_buf *dmabuf,
> +   struct device *dev)
> +{
> + struct dma_buf_attachment *attach;
> +
> + if (WARN_ON(!dmabuf || !dev))
> + return ERR_PTR(-EINVAL);
> +
> + mutex_lock(&dmabuf->lock);
> + list_for_each_entry(attach, &dmabuf->attachments, node) {
> + if (attach->dev == dev) {
> + mutex_unlock(&dmabuf->lock);
> + return attach;
> + }
> + }
> + mutex_unlock(&dmabuf->lock);
> +
> + return ERR_PTR(-ENODEV);
> +}
> +EXPORT_SYMBOL_GPL(dma_buf_get_attachment);
NAK in any form..

Spot the race condition between dma_buf_get_attachment and dma_buf_attach

~Maarten

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[RFC][PATCH 1/2] dma-buf: add importer private data to attachment

2013-05-31 Thread Seung-Woo Kim 
dma-buf attachment has only exporter private data, but importer private data
can be useful for importer especially to re-import the same dma-buf.
To use importer private data in attachment of the device, the function to
search attachment in the attachment list of dma-buf is also added.

Signed-off-by: Seung-Woo Kim 
---
 drivers/base/dma-buf.c  |   31 +++
 include/linux/dma-buf.h |4 
 2 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
index 08fe897..a1eaaf2 100644
--- a/drivers/base/dma-buf.c
+++ b/drivers/base/dma-buf.c
@@ -259,6 +259,37 @@ err_attach:
 EXPORT_SYMBOL_GPL(dma_buf_attach);
 
 /**
+ * dma_buf_get_attachment - Get attachment with the device from dma_buf's
+ * attachments list
+ * @dmabuf:[in]buffer to find device from.
+ * @dev:   [in]device to be found.
+ *
+ * Returns struct dma_buf_attachment * attaching the device; may return
+ * negative error codes.
+ *
+ */
+struct dma_buf_attachment *dma_buf_get_attachment(struct dma_buf *dmabuf,
+ struct device *dev)
+{
+   struct dma_buf_attachment *attach;
+
+   if (WARN_ON(!dmabuf || !dev))
+   return ERR_PTR(-EINVAL);
+
+   mutex_lock(&dmabuf->lock);
+   list_for_each_entry(attach, &dmabuf->attachments, node) {
+   if (attach->dev == dev) {
+   mutex_unlock(&dmabuf->lock);
+   return attach;
+   }
+   }
+   mutex_unlock(&dmabuf->lock);
+
+   return ERR_PTR(-ENODEV);
+}
+EXPORT_SYMBOL_GPL(dma_buf_get_attachment);
+
+/**
  * dma_buf_detach - Remove the given attachment from dmabuf's attachments list;
  * optionally calls detach() of dma_buf_ops for device-specific detach
  * @dmabuf:[in]buffer to detach from.
diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
index dfac5ed..09cff0f 100644
--- a/include/linux/dma-buf.h
+++ b/include/linux/dma-buf.h
@@ -136,6 +136,7 @@ struct dma_buf {
  * @dev: device attached to the buffer.
  * @node: list of dma_buf_attachment.
  * @priv: exporter specific attachment data.
+ * @importer_priv: importer specific attachment data.
  *
  * This structure holds the attachment information between the dma_buf buffer
  * and its user device(s). The list contains one attachment struct per device
@@ -146,6 +147,7 @@ struct dma_buf_attachment {
struct device *dev;
struct list_head node;
void *priv;
+   void *importer_priv;
 };
 
 /**
@@ -164,6 +166,8 @@ static inline void get_dma_buf(struct dma_buf *dmabuf)
 
 struct dma_buf_attachment *dma_buf_attach(struct dma_buf *dmabuf,
struct device *dev);
+struct dma_buf_attachment *dma_buf_get_attachment(struct dma_buf *dmabuf,
+   struct device *dev);
 void dma_buf_detach(struct dma_buf *dmabuf,
struct dma_buf_attachment *dmabuf_attach);
 
-- 
1.7.4.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/