| Message ID | 1450315926-30894-1-git-send-email-yu.dai@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 17 Dec 2015, yu.dai@intel.com wrote: > From: Alex Dai <yu.dai@intel.com> > > The device struct_mutex needs to be held before releasing any GEM > objects allocated by GuC. This is indeed so, but your patch subject needs to say it fixes an actual bug rather than a "warning message problem" which makes one think it's benign. Also, if you see a warning splat in dmesg, please include that in the commit message when you fix it. It's *much* easier to match bug reports with fixes when you have that. BR, Jani. > > Signed-off-by: Alex Dai <yu.dai@intel.com> > --- > drivers/gpu/drm/i915/intel_guc_loader.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c > index 625272f4..4748651 100644 > --- a/drivers/gpu/drm/i915/intel_guc_loader.c > +++ b/drivers/gpu/drm/i915/intel_guc_loader.c > @@ -631,10 +631,10 @@ 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_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 12/18/2015 01:55 AM, Jani Nikula wrote: > On Thu, 17 Dec 2015, yu.dai@intel.com wrote: > > From: Alex Dai <yu.dai@intel.com> > > > > The device struct_mutex needs to be held before releasing any GEM > > objects allocated by GuC. > > This is indeed so, but your patch subject needs to say it fixes an > actual bug rather than a "warning message problem" which makes one think > it's benign. > > Also, if you see a warning splat in dmesg, please include that in the > commit message when you fix it. It's *much* easier to match bug reports > with fixes when you have that. > I will change subject to something like "Fix a potential issue ..." and with a warning message log in comment. Thanks, Alex > > > > Signed-off-by: Alex Dai <yu.dai@intel.com> > > --- > > drivers/gpu/drm/i915/intel_guc_loader.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c > > index 625272f4..4748651 100644 > > --- a/drivers/gpu/drm/i915/intel_guc_loader.c > > +++ b/drivers/gpu/drm/i915/intel_guc_loader.c > > @@ -631,10 +631,10 @@ 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_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; >
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c index 625272f4..4748651 100644 --- a/drivers/gpu/drm/i915/intel_guc_loader.c +++ b/drivers/gpu/drm/i915/intel_guc_loader.c @@ -631,10 +631,10 @@ 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_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;