MIME-version: 1.0
Content-type: text/plain; charset=UTF-8
Content-transfer-encoding: 8BIT
Message-id: <51B14644.9070706@samsung.com>
Date: Fri, 07 Jun 2013 11:32:36 +0900
From: =?UTF-8?B?6rmA7Iq57Jqw?= <sw0312.kim@samsung.com>
Reply-to: sw0312.kim@samsung.com
User-Agent: Mozilla/5.0 (X11; Linux i686; rv:16.0) Gecko/20121011
 Thunderbird/16.0.1
To: Maarten Lankhorst <maarten.lankhorst@canonical.com>
Cc: dri-devel@lists.freedesktop.org, linux-media@vger.kernel.org,
        linaro-mm-sig@lists.linaro.org, sumit.semwal@linaro.org,
        airlied@linux.ie, daniel.vetter@ffwll.ch, kyungmin.park@samsung.com,
        linux-kernel@vger.kernel.org, Seung-Woo Kim <sw0312.kim@samsung.com>
Subject: Re: [RFC][PATCH 1/2] dma-buf: add importer private data to attachment
References: <1369990487-23510-1-git-send-email-sw0312.kim@samsung.com>
 <1369990487-23510-2-git-send-email-sw0312.kim@samsung.com>
 <51AF3BD7.5070001@canonical.com>
In-reply-to: <51AF3BD7.5070001@canonical.com>
DLP-Filter: Pass
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2930
Lines: 90

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 <sw0312.kim@samsung.com>
>> ---
>>  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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/