On 11/19/24 11:46, Zack Rusin wrote: >> int vmw_cmdbuf_surface_define(struct vmw_private *dev_priv, >> struct vmw_sw_context *sw_context, >> struct vmw_surface_metadata *metadata, >> @@ -2484,6 +2522,9 @@ int vmw_cmdbuf_surface_define(struct vmw_private *dev_priv, >> return ret; >> } >> >> + res->hw_destroy = vmw_cmdbuf_surface_hw_destroy; >> + surface->cmdbuf_destroy = false; >> + >> return 0; >> } >> >> -- >> 2.43.0 >> > > Hmm, this looks like a hack. So what seems to be happening is that the > reference count on the resource is still active. When vmw_resource > reference count goes to zero it should call hw_destroy, which should > invoke vmw_gb_surface_destroy which should delete the surface. It > looks like the reference count of userspace resources is off, so with > this patch we'd be destroying the gb surface but the vmw_resource is > still alive. If that's the case, then what we want to do is fix the > reference counting of the vmw_resource rather than working around it. > > z I have looked more closely at the resource refcounts here. In case there is an error in surface creation, then refcounts were not being freed properly, this is fixed in v3 of this patch series. As for hw_destroy, I have put binding the hw_destroy function when command buffer gets committed, so it only gets invoked when userspace has not submitted the destroy command. This also fixes an issue where if there is an error in command buffer checks and the surface create command gets reverted, the kernel still registered hw_destroy function and submitted a destroy surface command for the surface which was never created on device. -- Maaz Mombasawala <maaz.mombasawala@xxxxxxxxxxxx>
