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

From: Eric Anholt <eric@anholt.net>
To: Qiang Yu <yuq825@gmail.com>
Cc: Simon Shields <simon@lineageos.org>, Marek Vasut <marex@denx.de>,
	Connor Abbott <cwabbott0@gmail.com>,
	Neil Armstrong <narmstrong@baylibre.com>,
	Andrei Paulau <7134956@gmail.com>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	Vasily Khoruzhick <anarsoul@gmail.com>,
	Erico Nunes <nunes.erico@gmail.com>
Subject: Re: [PATCH RFC 05/24] Revert "drm: Nerf the preclose callback for modern drivers"
Date: Thu, 31 May 2018 10:51:13 -0700	[thread overview]
Message-ID: <87po1br9wu.fsf@anholt.net> (raw)
In-Reply-To: <CAKGbVbvcmYUSVNWXAJE3jqP90MCohgwKft3EF8pziRCov3aAUQ@mail.gmail.com>

[-- Attachment #1.1: Type: text/plain, Size: 1514 bytes --]

Qiang Yu <yuq825@gmail.com> writes:

>>>
>>> It is OK if evil user free/unmap the buffer when task is not done
>>> in my implementation. It will generate a MMU fault in that case and kernel
>>> driver will do recovery.
>>>
>>> So does the Ctrl+C case, if don't deal with it, just get some noisy MMU
>>> fault warning and a HW reset recovery.
>>
>> How about an app rendering to shared buffers, which glFlush()es and
>> exits cleanly but doesn't close the DRI screen?  What would cause that
>> app's rendering to get completed succesfully instead of faulting to
>> death?
> Do you mean the same case as Ctrl+C when an app exit without waiting
> all task finished in userspace?

Basically the same, but I'm saying that the app is doing everything
right and terminating successfully, rather than being interrupted (which
you might otherwise use to justify its rendering failing)

>> You really do need to refcount the buffers used in a rendering job so
>> they don't get freed early.
> Do you mean refcount the buffers in kernel space job? This is OK but
> not necessary, I can wait task complete in gem_close_object which
> will be called by drm_release for each buffer too (I still think better
> waiting in preclose at once but it's gone).

Just wait for all tasks to complete when any object is freed?  That's
going to be bad for performance.  Or are you saying that you already
have the connection between the task and its objects (and, if so, why
aren't you just doing refcounting correctly through that path?)

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel