| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAAQKjZOxMR-s3Fc9toXspN_a7azj==4mzAnCffWio8oEXyC9pQ@mail.gmail.com> Date: Tue, 10 Jan 2012 18:19:08 +0900 From: InKi Dae <daeinki@...il.com> To: "Semwal, Sumit" <sumit.semwal@...com> Cc: Rob Clark <rob@...com>, t.stanislaws@...sung.com, linux@....linux.org.uk, arnd@...db.de, linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org, linaro-mm-sig@...ts.linaro.org, linux-mm@...ck.org, m.szyprowski@...sung.com, Sumit Semwal <sumit.semwal@...aro.org>, linux-arm-kernel@...ts.infradead.org, linux-media@...r.kernel.org Subject: Re: [RFC v2 1/2] dma-buf: Introduce dma buffer sharing mechanism 2012/1/10 InKi Dae <daeinki@...il.com>: > 2012/1/10 Semwal, Sumit <sumit.semwal@...com>: >> On Tue, Jan 10, 2012 at 7:44 AM, Rob Clark <rob@...com> wrote: >>> On Mon, Jan 9, 2012 at 7:34 PM, InKi Dae <daeinki@...il.com> wrote: >>>> 2012/1/10 Rob Clark <rob@...com>: >>>> at least with no IOMMU, the memory information(containing physical >>>> memory address) would be copied to vb2_xx_buf object if drm gem >>>> exported its own buffer and vb2 wants to use that buffer at this time, >>>> sg table is used to share that buffer. and the problem I pointed out >>>> is that this buffer(also physical memory region) could be released by >>>> vb2 framework(as you know, vb2_xx_buf object and the memory region for >>>> buf->dma_addr pointing) but the Exporter(drm gem) couldn't know that >>>> so some problems would be induced once drm gem tries to release or >>>> access that buffer. and I have tried to resolve this issue adding >>>> get_shared_cnt() callback to dma-buf.h but I'm not sure that this is >>>> good way. maybe there would be better way. >> Hi Inki, >> As also mentioned in the documentation patch, importer (the user of >> the buffer) - in this case for current RFC patches on >> v4l2-as-a-user[1] vb2 framework - shouldn't release the backing memory >> of the buffer directly - it should only use the dma-buf callbacks in >> the right sequence to let the exporter know that it is done using this >> buffer, so the exporter can release it if allowed and needed. > > thank you for your comments.:) and below are some tables about dmabuf > operations with ideal use and these tables indicate reference count of > when buffer is created, shared and released. so if there are any > problems, please let me know. P.S. these are just simple cases so > there would be others. > > > in case of using only drm gem and dmabuf, > > operations gem refcount file f_count buf refcount > ------------------------------------------------------------------------------------------------ > 1. gem create A:1 A:0 > 2. export(handle A -> fd) A:2 A:1 A:0 > 3. import(fd -> handle B) A:2, B:1 A:2 A:1 > 4. file close(A) A:2, B:1 A:1 A:1 > 5. gem close(A) A:1, B:1 A:1 A:1 > 6. gem close(B) A:1, B:0 A:1 A:0 > 7. file close(A) A:0 A:0 > ----------------------------------------------------------------------------------------------- > 3. handle B shares the buf of handle A. > 6. release handle B but its buf. > 7. release gem handle A and dmabuf of file A and also physical memory region. > > > and in case of using drm gem, vb2 and dmabuf, > > operations gem, vb2 refcount file f_count buf refcount > ------------------------------------------------------------------------------------------------ > 1. gem create A:1 A:0 > (GEM side) > 2. export(handle A -> fd) A:2 A:1 A:0 > (GEM side) > 3. import(fd -> handle B) A:2, B:1 A:2 A:1 > (VB2 side) > 4. file close(A) A:2, B:1 A:1 A:1 > (VB2 side) > 5. vb2 close(B) A:2, B:0 A:1 A:0 > (VB2 side) > 6. gem close(A) A:1 A:1 A:0 > (GEM side) > 7. file close(A) A:0 A:0 > (GEM side) > ------------------------------------------------------------------------------------------------ > 3. vb2 handle B is shared with the buf of gem handle A. > 5. release vb2 handle B and decrease refcount of the buf pointed by it. > 7. release gem handle A and dmabuf of file A and also physical memory region. > Ah, sorry, it seems that file close shouldn't be called because file->f_count of the file would be dropped by Importer and if f_count is 0 then the file would be released by fput() so I'm not sure but again: in case of using only drm gem and dmabuf, operations gem refcount file f_count buf refcount -------------------------------------------------------------------------------------------------- 1. gem create(A) A:1 A:0 2. export(handle A -> fd) A:2 A:1 A:0 3. import(fd -> handle B) A:2, B:1 A:2 A:1 4. gem close(B) A:2, B:release A:1 A:0 5. gem close(A) A:1, A:1 A:0 6. gem close(A) A:release A:release A:release ------------------------------------------------------------------------------------------------- and in case of using drm gem, vb2 and dmabuf, operations gem, vb2 refcount file f_count buf refcount ------------------------------------------------------------------------------------------------ 1. gem create A:1 A:0 A:0 (GEM side) 2. export(handle A -> fd) A:2 A:1 A:0 (GEM side) 3. import(fd -> handle B) A:2, B:1 A:2 A:1 (VB2 side) 5. vb2 close(B) A:2, B:release A:1 A:0 (VB2 side) 6. gem close(A) A:1 A:1 A:0 (GEM side) 6. gem close(A) A:release A:release A:release (GEM side) ------------------------------------------------------------------------------------------------ >>> >>> the exporter (in this case your driver's drm/gem bits) shouldn't >>> release that mapping / sgtable until the importer (in this case v4l2) >>> calls dma_buf_unmap fxn.. >>> >>> It would be an error if the importer did a dma_buf_put() without first >>> calling dma_buf_unmap_attachment() (if currently mapped) and then >>> dma_buf_detach() (if currently attached). Perhaps somewhere there >>> should be some sanity checking debug code which could be enabled to do >>> a WARN_ON() if the importer does the wrong thing. It shouldn't really >>> be part of the API, I don't think, but it actually does seem like a >>> good thing, esp. as new drivers start trying to use dmabuf, to have >>> some debug options which could be enabled. >>> >>> It is entirely possible that something was missed on the vb2 patches, >>> but the way it is intended to work is like this: >>> https://github.com/robclark/kernel-omap4/blob/0961428143cd10269223e3d0f24bc3a66a96185f/drivers/media/video/videobuf2-core.c#L92 >>> >>> where it does a detach() before the dma_buf_put(), and the vb2-contig >>> backend checks here that it is also unmapped(): >>> https://github.com/robclark/kernel-omap4/blob/0961428143cd10269223e3d0f24bc3a66a96185f/drivers/media/video/videobuf2-dma-contig.c#L251 >> >> The proposed RFC for V4L2 adaptation at [1] does exactly the same >> thing; detach() before dma_buf_put(), and check in vb2-contig backend >> for unmapped() as mentioned above. >> >>> >>> BR, >>> -R >>> >> BR, >> Sumit. >> >> [1]: V4l2 as a dma-buf user RFC: >> http://comments.gmane.org/gmane.linux.drivers.video-input-infrastructure/42966 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists