From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?UTF-8?B?6rmA7Iq57Jqw?= Subject: Re: [RFC][PATCH 1/2] dma-buf: add importer private data to attachment Date: Fri, 07 Jun 2013 11:32:36 +0900 Message-ID: <51B14644.9070706@samsung.com> 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> Reply-To: sw0312.kim@samsung.com Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: In-reply-to: <51AF3BD7.5070001@canonical.com> Sender: linux-media-owner@vger.kernel.org To: Maarten Lankhorst 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 List-Id: dri-devel@lists.freedesktop.org Hello Maarten, On 2013=EB=85=84 06=EC=9B=94 05=EC=9D=BC 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 priv= ate 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 functi= on 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); >> =20 >> /** >> + * 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 re= turn >> + * negative error codes. >> + * >> + */ >> +struct dma_buf_attachment *dma_buf_get_attachment(struct dma_buf *d= mabuf, >> + 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 =3D=3D 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.. >=20 > Spot the race condition between dma_buf_get_attachment and dma_buf_at= tach.... Both dma_buf_get_attachment and dma_buf_attach has mutet with dmabuf->lock, and dma_buf_get_attachment is used for get attachment fro= m 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 specificall= y. Thanks and Best Regards, - Seung-Woo Kim >=20 > ~Maarten >=20 >=20 --=20 Seung-Woo Kim Samsung Software R&D Center --