Re: [PATCH RFC 05/24] Revert "drm: Nerf the preclose callback for modern drivers"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, May 23, 2018 at 2:59 PM, Qiang Yu <yuq825@xxxxxxxxx> wrote:
> On Wed, May 23, 2018 at 5:04 PM, Daniel Vetter <daniel@xxxxxxxx> wrote:
>> On Tue, May 22, 2018 at 09:04:17AM +0800, Qiang Yu wrote:
>>> On Tue, May 22, 2018 at 3:37 AM, Eric Anholt <eric@xxxxxxxxxx> wrote:
>>> > Qiang Yu <yuq825@xxxxxxxxx> writes:
>>> >
>>> >> This reverts commit 45c3d213a400c952ab7119f394c5293bb6877e6b.
>>> >>
>>> >> lima driver need preclose to wait all task in the context
>>> >> created within closing file to finish before free all the
>>> >> buffer object. Otherwise pending tesk may fail and get
>>> >> noisy MMU fault message.
>>> >>
>>> >> Move this wait to each buffer object free function can
>>> >> achieve the same result but some buffer object is shared
>>> >> with other file context, but we only want to wait the
>>> >> closing file context's tasks. So the implementation is
>>> >> not that straight forword compared to the preclose one.
>>> >
>>> > You should just separate your MMU structures from drm_file, and have
>>> > drm_file and the jobs using it keep a reference on them.  This is what
>>> > I've done in V3D as well.
>>>
>>> It's not the VM/MMU struct that causes this problem, it's each buffer
>>> object that gets freed before task is done (postclose is after buffer free).
>>> If you mean I should keep reference of all buffers for tasks, that's not
>>> as simple as just waiting task done before free buffers.
>>
>> Why can't you do that waiting in the postclose hook? If it's the lack of
>> reference-counting in your driver for gem bo, then I'd say you need to
>> roll out some reference counting. Relying on the implicit reference
>> provided by the core is kinda not so great (which was the reason I've
>> thrown out the preclose hook). There's also per-bo open/close hooks.
>
> It's possible to not use preclose, but the implementation is not as simple
> and straight forward as the preclose I think. There're two method I can
> think of:
> 1. do wait when free buffers callback unmap buffer from this process's
>     lima VM (wait buffer reservation object), this is fine and simple, but
>     there's case that this buffer is shared between two processes, so the
>     best way should be only waiting fences from this process, so we'd
>     better do some record for fences for a "perfect waiting"
> 2. keep a reference of involved buffers for a task, unreference it when
>     task done, also keep a reference of the buffer mapping in this process's
>     lima VM (this is more complicated to implement)
>
> But if there's a preclose, just wait all this process's task done, then
> unmap/free buffers, it's simple and straight forward. I'd like to hear if
> there's other better way for only use postclose.

Refcount your buffers. Borrowing references from other places tends to
result in a maintenance headache with no end. So solution 2.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux