| Message ID | 1452711710-4505-1-git-send-email-yu.dai@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 13/01/16 19:01, yu.dai@intel.com wrote: > From: Alex Dai <yu.dai@intel.com> > > During driver unloading, the guc_client created for command submission > needs to be released to avoid memory leak. > > The struct_mutex needs to be held before tearing down GuC. > > v1: Move i915_guc_submission_disable out of i915_guc_submission_fini and > take struct_mutex lock before release GuC client. (Dave Gordon) > v2: Add the locking for failure case in guc_fw_fetch. (Dave Gordon) > Add i915_guc_submission_fini for failure case in intel_guc_ucode_load. > > Signed-off-by: Alex Dai <yu.dai@intel.com> LGTM. Reviewed-by: Dave Gordon <david.s.gordon@intel.com> > > diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c > index d20788f..3accd91 100644 > --- a/drivers/gpu/drm/i915/intel_guc_loader.c > +++ b/drivers/gpu/drm/i915/intel_guc_loader.c > @@ -445,6 +445,7 @@ fail: > > direct_interrupts_to_host(dev_priv); > i915_guc_submission_disable(dev); > + i915_guc_submission_fini(dev); > > return err; > } > @@ -561,10 +562,12 @@ fail: > DRM_ERROR("Failed to fetch GuC firmware from %s (error %d)\n", > guc_fw->guc_fw_path, err); > > + mutex_lock(&dev->struct_mutex); > obj = guc_fw->guc_fw_obj; > if (obj) > drm_gem_object_unreference(&obj->base); > guc_fw->guc_fw_obj = NULL; > + mutex_unlock(&dev->struct_mutex); > > release_firmware(fw); /* OK even if fw is NULL */ > guc_fw->guc_fw_fetch_status = GUC_FIRMWARE_FAIL; > @@ -631,10 +634,11 @@ void intel_guc_ucode_fini(struct drm_device *dev) > struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw; > > + mutex_lock(&dev->struct_mutex); > direct_interrupts_to_host(dev_priv); > + i915_guc_submission_disable(dev); > i915_guc_submission_fini(dev); > > - mutex_lock(&dev->struct_mutex); > if (guc_fw->guc_fw_obj) > drm_gem_object_unreference(&guc_fw->guc_fw_obj->base); > guc_fw->guc_fw_obj = NULL; >
On 13/01/16 19:11, Dave Gordon wrote: > On 13/01/16 19:01, yu.dai@intel.com wrote: >> From: Alex Dai <yu.dai@intel.com> >> >> During driver unloading, the guc_client created for command submission >> needs to be released to avoid memory leak. >> >> The struct_mutex needs to be held before tearing down GuC. >> >> v1: Move i915_guc_submission_disable out of i915_guc_submission_fini and >> take struct_mutex lock before release GuC client. (Dave Gordon) >> v2: Add the locking for failure case in guc_fw_fetch. (Dave Gordon) >> Add i915_guc_submission_fini for failure case in >> intel_guc_ucode_load. >> >> Signed-off-by: Alex Dai <yu.dai@intel.com> > > LGTM. > > Reviewed-by: Dave Gordon <david.s.gordon@intel.com> Patch merged, thanks! Regards, Tvrtko
On Mon, Jan 18, 2016 at 10:01:24AM +0000, Tvrtko Ursulin wrote: > > On 13/01/16 19:11, Dave Gordon wrote: > >On 13/01/16 19:01, yu.dai@intel.com wrote: > >>From: Alex Dai <yu.dai@intel.com> > >> > >>During driver unloading, the guc_client created for command submission > >>needs to be released to avoid memory leak. > >> > >>The struct_mutex needs to be held before tearing down GuC. > >> > >>v1: Move i915_guc_submission_disable out of i915_guc_submission_fini and > >> take struct_mutex lock before release GuC client. (Dave Gordon) > >>v2: Add the locking for failure case in guc_fw_fetch. (Dave Gordon) > >> Add i915_guc_submission_fini for failure case in > >>intel_guc_ucode_load. > >> > >>Signed-off-by: Alex Dai <yu.dai@intel.com> > > > >LGTM. > > > >Reviewed-by: Dave Gordon <david.s.gordon@intel.com> > > Patch merged, thanks! CI resulted in warnings for this patch. Per our latest discussion in Jesse's meeting please reply to the CI mail with your analysis of why these are all preexisting failures and with links to bugzilla. Otherwise this patch can't go in. I'll paste you the link to the internally wiki with the BKM. -Daniel
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c index d20788f..3accd91 100644 --- a/drivers/gpu/drm/i915/intel_guc_loader.c +++ b/drivers/gpu/drm/i915/intel_guc_loader.c @@ -445,6 +445,7 @@ fail: direct_interrupts_to_host(dev_priv); i915_guc_submission_disable(dev); + i915_guc_submission_fini(dev); return err; } @@ -561,10 +562,12 @@ fail: DRM_ERROR("Failed to fetch GuC firmware from %s (error %d)\n", guc_fw->guc_fw_path, err); + mutex_lock(&dev->struct_mutex); obj = guc_fw->guc_fw_obj; if (obj) drm_gem_object_unreference(&obj->base); guc_fw->guc_fw_obj = NULL; + mutex_unlock(&dev->struct_mutex); release_firmware(fw); /* OK even if fw is NULL */ guc_fw->guc_fw_fetch_status = GUC_FIRMWARE_FAIL; @@ -631,10 +634,11 @@ void intel_guc_ucode_fini(struct drm_device *dev) struct drm_i915_private *dev_priv = dev->dev_private; struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw; + mutex_lock(&dev->struct_mutex); direct_interrupts_to_host(dev_priv); + i915_guc_submission_disable(dev); i915_guc_submission_fini(dev); - mutex_lock(&dev->struct_mutex); if (guc_fw->guc_fw_obj) drm_gem_object_unreference(&guc_fw->guc_fw_obj->base); guc_fw->guc_fw_obj = NULL;