On Mon, Jun 16, 2025 at 01:38:17PM +0200, Christian König wrote: > On 6/16/25 12:38, Simona Vetter wrote: > >> 6. Now FD2HANDLE is called with 10 and here is what happens: > >> > >> drm_prime_lookup_buf_by_handle() is called for handle 10, so we > >> don't find anything. > >> > >> obj->dma_buf is true so we branch into the if and call > >> drm_prime_add_buf_handle() with handle 10. > >> > >> Now we have called drm_prime_add_buf_handle() both for handle 8 and > >> handle 10 and so we have both 8 and 10 for the same DMA-buf in our tree. > > > > So this is the case that broke, and which the various igt prime tests > > actually had testcases for. Unless I'm completely confused here now. > > > >> So FD2HANDLE could return either 8 or 10 depending on which is looked up > >> first. > >> > >> I'm not 100% sure if that has any bad consequences, but I'm pretty sure > >> that this is not intentional. > >> > >> Should we fix that? If yes than how? > > > > I dont think there's an issue, all we guarantee is that if you call > > FD2HANDLE or HANDLE2FD, then you get something consistent back. From a > > userspace pov there's two cases: > > > > 1. We've already seen this buffer, it got handle 8, that's the one we've > > stored in the lookup caches. If you then do GETFB2 you get handle 10, > > which could be confusing. So you do > > > > temp_dmabuf_fd = ioctl(HANDLE2FD, 10); > > new_id = ioctl(FD2HANDLE, temp_dmabuf_fd); > > close(temp_dma_buf_fd); > > ioctl(GEM_CLOSE, 10); > > > > At this point new_id is 8, > > No, exactly that is not guaranteed. > > The previous call to HANDLE2FD stored 10 into the lookup cache > additionally to 8 with the same dma_buf pointer. > > And if you now get 8 or 10 as return from FD2HANDLE depends on how the > red/black tree is balanced. It can be that 8 comes first or it can be > that 10 comes first because the tree is only sorted by dma_buf pointer > and that criteria is identical for both 8 and 10. > > As far as I can see this case is not correctly handled.
Hm right, I did type some testcases with flink, but not one that does funny stuff like this :-/ -Sima > > Regards, > Christian. > > and you already have that in your userspace > > cache, so all is good. > > > > 2. Userspace doesn't have the buffer already, but it doesn't know that. It > > does the exact dance as above, except this time around it gets back the > > same gem_handle as it got from GETFB2 and knows that it does not have to > > close that handle (since it's the only one), and that it should add that > > handle to the userspace-side dma-buf import/export side. > > > > It's a bit a contrived dance, but I don't think we have an issue here. > > > > Cheers, Sima > > > >> > >> Regards, > >> Christian. -- Simona Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
