Message ID | 1369990487-23510-2-git-send-email-sw0312.kim@samsung.com (mailing list archive) |
---|---|
State | Not Applicable, archived |
Headers |
Received: from mail.tu-berlin.de ([130.149.7.33]) by www.linuxtv.org with esmtp (Exim 4.72) (envelope-from <linux-media-owner@vger.kernel.org>) id 1UiL71-0001U1-Cf; Fri, 31 May 2013 10:55:03 +0200 X-tubIT-Incoming-IP: 209.132.180.67 Received: from vger.kernel.org ([209.132.180.67]) by mail.tu-berlin.de (exim-4.72/mailfrontend-5) with esmtp id 1UiL6z-0004yN-6j; Fri, 31 May 2013 10:55:03 +0200 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753814Ab3EaIy5 (ORCPT <rfc822;mkrufky@linuxtv.org> + 1 other); Fri, 31 May 2013 04:54:57 -0400 Received: from mailout2.samsung.com ([203.254.224.25]:23259 "EHLO mailout2.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751878Ab3EaIyk (ORCPT <rfc822;linux-media@vger.kernel.org>); Fri, 31 May 2013 04:54:40 -0400 Received: from epcpsbgr2.samsung.com (u142.gpu120.samsung.co.kr [203.254.230.142]) by mailout2.samsung.com (Oracle Communications Messaging Server 7u4-24.01 (7.0.4.24.0) 64bit (built Nov 17 2011)) with ESMTP id <0MNN002KSNF2B9B0@mailout2.samsung.com>; Fri, 31 May 2013 17:54:38 +0900 (KST) Received: from epcpsbgm2.samsung.com ( [172.20.52.112]) by epcpsbgr2.samsung.com (EPCPMTA) with SMTP id C8.D3.08825.E4568A15; Fri, 31 May 2013 17:54:38 +0900 (KST) X-AuditID: cbfee68e-b7f276d000002279-30-51a8654ee6ce Received: from epmmp2 ( [203.254.227.17]) by epcpsbgm2.samsung.com (EPCPMTA) with SMTP id 17.7B.21068.E4568A15; Fri, 31 May 2013 17:54:38 +0900 (KST) Received: from localhost.localdomain ([10.90.8.56]) by mmp2.samsung.com (Oracle Communications Messaging Server 7u4-24.01 (7.0.4.24.0) 64bit (built Nov 17 2011)) with ESMTPA id <0MNN00BWKNEZZT60@mmp2.samsung.com>; Fri, 31 May 2013 17:54:38 +0900 (KST) From: Seung-Woo Kim <sw0312.kim@samsung.com> To: dri-devel@lists.freedesktop.org, linux-media@vger.kernel.org, linaro-mm-sig@lists.linaro.org, sumit.semwal@linaro.org, airlied@linux.ie Cc: linux-kernel@vger.kernel.org, daniel.vetter@ffwll.ch, inki.dae@samsung.com, sw0312.kim@samsung.com, kyungmin.park@samsung.com Subject: [RFC][PATCH 1/2] dma-buf: add importer private data to attachment Date: Fri, 31 May 2013 17:54:46 +0900 Message-id: <1369990487-23510-2-git-send-email-sw0312.kim@samsung.com> X-Mailer: git-send-email 1.7.4.1 In-reply-to: <1369990487-23510-1-git-send-email-sw0312.kim@samsung.com> References: <1369990487-23510-1-git-send-email-sw0312.kim@samsung.com> X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFuplkeLIzCtJLcpLzFFi42JZI2JSoOuXuiLQ4PoGJYvecyeZLBY+vMts ceXrezaLSfcnsFicbXrDbvHlykMmi8u75rBZ9GzYympx6u5ndosZk1+yOXB57P22gMXjzrU9 bB7bvz1g9bjffZzJ4/a/x8wefVtWMXp83iQXwB7FZZOSmpNZllqkb5fAlfHlr0bBMrGKNWeu MzUwLhHqYuTkkBAwkbjyu4EZwhaTuHBvPRuILSSwlFHi567KLkYOsJqnMw27GLmAwtMZJe68 P80K4TQzSczZ38IK0sAmoCOxf8lvMFtEoJdRYs09JpAiZoFmRolHy/4wgSSEBbwkNsw9AWaz CKhK/L6+hAXE5hVwk1g2+yUjxBUKEgvuvQW7glPAXWJ932UWiIvcJO7Nu88OUXOMXeLO7QyI OQIS3yYfYoG4VFZi0wGoZyQlDq64wTKBUXgBI8MqRtHUguSC4qT0IiO94sTc4tK8dL3k/NxN jMCoOP3vWd8OxpsHrA8xJgONm8gsJZqcD4yqvJJ4Q2MzIwtTE1NjI3NLM9KElcR51VqsA4UE 0hNLUrNTUwtSi+KLSnNSiw8xMnFwSjUwNsSsm9XNf776eZOzxDm+/26/mZdPfbKkNmzb9v7m l6cv77l8j/WoItcBA8mqJ/miPMGlE1dbeExmiXe4fyNHrvz3QsnorWd8P59l0FqxX3jHJl31 6z5mey81bI2e2bIkyPXrR56N18uFhEol2YIlTPo0p8adSp+xJ0NnstIEpZTsxPm+mzcfUWIp zkg01GIuKk4EAIuQ6ZSgAgAA X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrDIsWRmVeSWpSXmKPExsVy+t9jQV2/1BWBBh/3SFn0njvJZLHw4V1m iytf37NZTLo/gcXibNMbdosvVx4yWVzeNYfNomfDVlaLU3c/s1vMmPySzYHLY++3BSwed67t YfPY/u0Bq8f97uNMHrf/PWb26NuyitHj8ya5APaoBkabjNTElNQihdS85PyUzLx0WyXv4Hjn eFMzA0NdQ0sLcyWFvMTcVFslF58AXbfMHKADlRTKEnNKgUIBicXFSvp2mCaEhrjpWsA0Ruj6 hgTB9RgZoIGENYwZX/5qFCwTq1hz5jpTA+MSoS5GDg4JAROJpzMNuxg5gUwxiQv31rN1MXJx CAlMZ5S48/40K4TTzCQxZ38LK0gVm4COxP4lv8FsEYFeRok195hAipgFmhklHi37wwSSEBbw ktgw9wSYzSKgKvH7+hIWEJtXwE1i2eyXjBDrFCQW3HvLBmJzCrhLrO+7DFYjBFRzb9599gmM vAsYGVYxiqYWJBcUJ6XnGukVJ+YWl+al6yXn525iBMfdM+kdjKsaLA4xCnAwKvHwHkxZHijE mlhWXJl7iFGCg1lJhFc3eEWgEG9KYmVValF+fFFpTmrxIcZkoKsmMkuJJucDU0JeSbyhsYmZ kaWRuaGFkbE5acJK4rwHW60DhQTSE0tSs1NTC1KLYLYwcXBKNTDOi7vIc1LuoLelkVqF8Adj /Zu5fZt839Tu5N310TJRa/s04Ts/bvZPnn5T2zFVZseinJOcIQceNk7TkZXxPPklt3DelAlz am68+xHyL1T/qEV6UdVuSTvuJZf4DErXd8sYVV3da98t3b3iZ8CpHbvSXoTJf9scsN5w+t1z d14dyl+x+eavC0tblFiKMxINtZiLihMBcBWjZ/8CAAA= DLP-Filter: Pass X-MTR: 20000000000000000@CPGS X-CFilter-Loop: Reflected Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: <linux-media.vger.kernel.org> X-Mailing-List: linux-media@vger.kernel.org X-PMX-Version: 6.0.0.2142326, Antispam-Engine: 2.7.2.2107409, Antispam-Data: 2013.5.31.84229 X-PMX-Spam: Gauge=IIIIIIII, Probability=8%, Report=' MULTIPLE_RCPTS 0.1, HTML_00_01 0.05, HTML_00_10 0.05, BODYTEXTP_SIZE_3000_LESS 0, BODY_SIZE_2000_2999 0, BODY_SIZE_5000_LESS 0, BODY_SIZE_7000_LESS 0, URI_ENDS_IN_HTML 0, __ANY_URI 0, __CP_URI_IN_BODY 0, __HAS_FROM 0, __HAS_MSGID 0, __HAS_X_MAILER 0, __HAS_X_MAILING_LIST 0, __IN_REP_TO 0, __MIME_TEXT_ONLY 0, __MULTIPLE_RCPTS_CC_X2 0, __MULTIPLE_RCPTS_TO_X5 0, __SANE_MSGID 0, __SUBJ_ALPHA_END 0, __TO_MALFORMED_2 0, __TO_NO_NAME 0, __URI_NO_WWW 0, __URI_NS ' |
Commit Message
Seung-Woo Kim
May 31, 2013, 8:54 a.m. UTC
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(-)
Comments
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.... ~Maarten -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 > >
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 <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. 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-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <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. > > 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-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel >
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);