On 01/09/2016 11:43 AM, Daniel Vetter wrote:
On Fri, Jan 8, 2016 at 9:53 PM, Thomas Hellstrom thellstrom@vmware.com wrote:
On 01/08/2016 09:36 PM, Daniel Vetter wrote:
Again since the drm core takes care of event unlinking/disarming this is now just needless code.
Cc: Thomas Hellström thellstrom@vmware.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com
Hmm,
IIRC this is actually a list of events that core drm is not aware of yet. They sit on this list waiting for a fence to pass and are then transferred to core drm....
Yes I know. Earlier patches in the series extract new core functions to setup/tear down such events and send them out, which is what's needed to make this trick possible. Exynos similarly uses events similarly, and is also converted. Same for nouveau it seems, but there the code doesn't use the reserve/send split, so I'm unclear how/whether at all it correctly handles this race. -Daniel
Ah. Hmm I should've looked more closely at the rest of the series.
In any case, this particular patch leaves, from what I can tell, the eaction fpriv list intact when it is later freed in postclose, which is bad. Also each eaction is left with a dangling pointer to a freed pending event which is also very bad since that pointer will be dereferenced as soon as the fence's seqno has passed. So as far as i can tell, this function needs to remain except for the event destruction.
Thinking of it, this must be a problem that is more general problem than for vmwgfx only, I mean, unless the driver traverses all core pending event list to find relevant pending events to process, something in the driver must actually point to the pending event (be it a pointer in the fence object or, as in the vmwgfx case, a pointer in the fence action object) and that pointer must somehow be invalidated when the pending event is freed...
Which also brings up a question, where are the pending events actually destroyed? I can see they are unlinked in drm_fops.c.
/Thomas