<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> </head> <body text="#000000" bgcolor="#FFFFFF"> <div class="moz-cite-prefix">Am 24.05.2018 um 03:38 schrieb Qiang Yu: </div> <blockquote type="cite" cite="mid:CAKGbVbv10zGLe34J1BXdKHg1Kg0=7A3jUy_8pnjVjNs1T=r8mw@mail.gmail.com">[SNIP] <blockquote type="cite"> <blockquote type="cite"> <pre wrap="">Adding fence is done already, and I did wait it before unmap. But then I see when the buffer is shared between processes, the "perfect wait" is just wait the fence from this process's task, so it's better to also distinguish fences. If so, I just think why we don't just wait tasks from this process in the preclose before unmap/free buffer in the drm_release()? </pre> </blockquote> <pre wrap=""> Well it depends on your VM management. When userspace expects that the VM space the BO used is reusable immediately than the TTM callback won't work. On the other hand you can just grab the list of fences on a BO and filter out the ones from your current process and wait for those. See amdgpu_sync_resv() as an example how to do that. </pre> </blockquote> <pre wrap=""> In current lima implementation, user space driver is responsible not unmap/free buffer before task is complete. And VM map/unmap is not differed.</pre> </blockquote> Well it's up to you how to design userspace, but in the past doing it like that turned out to be a rather bad design decision. Keep in mind that the kernel driver must guarantee that a shaders can never access freed up memory. Otherwise taking over the system from an unprivileged processes becomes just a typing exercise when you manage to access freed memory which is now used for a page table. Because of this we have a separate tracking in amdgpu so that we not only know who is using which BO, who is using which VM. <blockquote type="cite" cite="mid:CAKGbVbv10zGLe34J1BXdKHg1Kg0=7A3jUy_8pnjVjNs1T=r8mw@mail.gmail.com"> <pre wrap="">This works simple and fine except the case that user press Ctrl+C to terminate the application which will force to close drm fd.</pre> </blockquote> I'm not sure if that actually works as fine as you think. For an example of what we had to add to prevent security breaches, take a look at amdgpu_gem_object_close(). <blockquote type="cite" cite="mid:CAKGbVbv10zGLe34J1BXdKHg1Kg0=7A3jUy_8pnjVjNs1T=r8mw@mail.gmail.com"> <pre wrap="">I'd more prefer to wait buffer fence before vm unmap and filter like amdgpu_sync_resv() compared to implement refcount in kernel task. But these two ways are all not as simple as preclose.</pre> </blockquote> Well, I would rather say you should either delay VM unmap operations until all users of the VM are done with their work using the ttm_bo_destroy callback. Or you block in the gem_close_object callback until all tasks using the BO are done with it. <blockquote type="cite" cite="mid:CAKGbVbv10zGLe34J1BXdKHg1Kg0=7A3jUy_8pnjVjNs1T=r8mw@mail.gmail.com"> <pre wrap="">So I still don't understand why you don't want to get preclose back even have to introduce other complicated mechanism to cover the case free/unmap buffer before this process's task is done?</pre> </blockquote> We intentionally removed the preclose callback to prevent certain use cases, bringing it back to allow your use case looks rather fishy to me. BTW: What exactly is the issue with using the postclose callback? Regards, Christian. <blockquote type="cite" cite="mid:CAKGbVbv10zGLe34J1BXdKHg1Kg0=7A3jUy_8pnjVjNs1T=r8mw@mail.gmail.com"> <pre wrap=""> Regards, Qiang </pre> </blockquote> </body> </html>