* [PATCH] drm/amdgpu: address the static checker warnings @ 2020-04-24 10:41 Evan Quan 2020-04-24 11:02 ` Dan Carpenter 0 siblings, 1 reply; 6+ messages in thread From: Evan Quan @ 2020-04-24 10:41 UTC (permalink / raw) To: amd-gfx; +Cc: Alexander.Deucher, Evan Quan, dan.carpenter drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:4199 amdgpu_device_gpu_recover() error: we previously assumed 'hive' could be null (see line 4196) This is introduced by "drm/amdgpu: optimize the gpu reset for XGMI setup V2". Change-Id: I9c22b57abc9f512114112f93fb035f1fecf26beb Signed-off-by: Evan Quan <evan.quan@amd.com> Reported-by: Dan Carpenter <dan.carpenter@oracle.com> --- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index 71278942f9f0..898338dc9605 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -4274,7 +4274,8 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev, if (!amdgpu_device_lock_adev(tmp_adev, !hive)) { DRM_INFO("Bailing on TDR for s_job:%llx, as another already in progress", job ? job->base.id : -1); - mutex_unlock(&hive->hive_lock); + if (hive) + mutex_unlock(&hive->hive_lock); return 0; } -- 2.26.2 _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amdgpu: address the static checker warnings 2020-04-24 10:41 [PATCH] drm/amdgpu: address the static checker warnings Evan Quan @ 2020-04-24 11:02 ` Dan Carpenter 2020-04-26 3:18 ` Quan, Evan 0 siblings, 1 reply; 6+ messages in thread From: Dan Carpenter @ 2020-04-24 11:02 UTC (permalink / raw) To: Evan Quan; +Cc: Alexander.Deucher, amd-gfx On Fri, Apr 24, 2020 at 06:41:15PM +0800, Evan Quan wrote: > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:4199 amdgpu_device_gpu_recover() > error: we previously assumed 'hive' could be null (see line 4196) > > This is introduced by "drm/amdgpu: optimize the gpu reset for XGMI setup V2". > > Change-Id: I9c22b57abc9f512114112f93fb035f1fecf26beb > Signed-off-by: Evan Quan <evan.quan@amd.com> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > index 71278942f9f0..898338dc9605 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > @@ -4274,7 +4274,8 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev, > if (!amdgpu_device_lock_adev(tmp_adev, !hive)) { > DRM_INFO("Bailing on TDR for s_job:%llx, as another already in progress", > job ? job->base.id : -1); > - mutex_unlock(&hive->hive_lock); > + if (hive) > + mutex_unlock(&hive->hive_lock); In the current code, we know for a fact that "hive" is NULL at this point. Presumably this will be changed in the future? Otherwise why not just delete the mutex_unlock() because it is dead code. regards, dan carpenter _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] drm/amdgpu: address the static checker warnings 2020-04-24 11:02 ` Dan Carpenter @ 2020-04-26 3:18 ` Quan, Evan 2020-04-28 2:31 ` Quan, Evan 0 siblings, 1 reply; 6+ messages in thread From: Quan, Evan @ 2020-04-26 3:18 UTC (permalink / raw) To: Dan Carpenter; +Cc: Deucher, Alexander, amd-gfx@lists.freedesktop.org -----Original Message----- From: Dan Carpenter <dan.carpenter@oracle.com> Sent: Friday, April 24, 2020 7:02 PM To: Quan, Evan <Evan.Quan@amd.com> Cc: amd-gfx@lists.freedesktop.org; Deucher, Alexander <Alexander.Deucher@amd.com> Subject: Re: [PATCH] drm/amdgpu: address the static checker warnings On Fri, Apr 24, 2020 at 06:41:15PM +0800, Evan Quan wrote: > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:4199 amdgpu_device_gpu_recover() > error: we previously assumed 'hive' could be null (see line 4196) > > This is introduced by "drm/amdgpu: optimize the gpu reset for XGMI setup V2". > > Change-Id: I9c22b57abc9f512114112f93fb035f1fecf26beb > Signed-off-by: Evan Quan <evan.quan@amd.com> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > index 71278942f9f0..898338dc9605 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > @@ -4274,7 +4274,8 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev, > if (!amdgpu_device_lock_adev(tmp_adev, !hive)) { > DRM_INFO("Bailing on TDR for s_job:%llx, as another already in progress", > job ? job->base.id : -1); > - mutex_unlock(&hive->hive_lock); > + if (hive) > + mutex_unlock(&hive->hive_lock); In the current code, we know for a fact that "hive" is NULL at this point. [Evan] No, that's true for SGPU setup only. For XGMI setup(multiple dgpus interconnected with bridges), the "hive" here is not NULL. Presumably this will be changed in the future? Otherwise why not just delete the mutex_unlock() because it is dead code. regards, dan carpenter _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] drm/amdgpu: address the static checker warnings 2020-04-26 3:18 ` Quan, Evan @ 2020-04-28 2:31 ` Quan, Evan 2020-04-28 11:01 ` Dan Carpenter 0 siblings, 1 reply; 6+ messages in thread From: Quan, Evan @ 2020-04-28 2:31 UTC (permalink / raw) To: Dan Carpenter; +Cc: Deucher, Alexander, amd-gfx@lists.freedesktop.org Ping.. -----Original Message----- From: Quan, Evan Sent: Sunday, April 26, 2020 11:19 AM To: Dan Carpenter <dan.carpenter@oracle.com> Cc: amd-gfx@lists.freedesktop.org; Deucher, Alexander <Alexander.Deucher@amd.com> Subject: RE: [PATCH] drm/amdgpu: address the static checker warnings -----Original Message----- From: Dan Carpenter <dan.carpenter@oracle.com> Sent: Friday, April 24, 2020 7:02 PM To: Quan, Evan <Evan.Quan@amd.com> Cc: amd-gfx@lists.freedesktop.org; Deucher, Alexander <Alexander.Deucher@amd.com> Subject: Re: [PATCH] drm/amdgpu: address the static checker warnings On Fri, Apr 24, 2020 at 06:41:15PM +0800, Evan Quan wrote: > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:4199 > amdgpu_device_gpu_recover() > error: we previously assumed 'hive' could be null (see line 4196) > > This is introduced by "drm/amdgpu: optimize the gpu reset for XGMI setup V2". > > Change-Id: I9c22b57abc9f512114112f93fb035f1fecf26beb > Signed-off-by: Evan Quan <evan.quan@amd.com> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > index 71278942f9f0..898338dc9605 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > @@ -4274,7 +4274,8 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev, > if (!amdgpu_device_lock_adev(tmp_adev, !hive)) { > DRM_INFO("Bailing on TDR for s_job:%llx, as another already in progress", > job ? job->base.id : -1); > - mutex_unlock(&hive->hive_lock); > + if (hive) > + mutex_unlock(&hive->hive_lock); In the current code, we know for a fact that "hive" is NULL at this point. [Evan] No, that's true for SGPU setup only. For XGMI setup(multiple dgpus interconnected with bridges), the "hive" here is not NULL. Presumably this will be changed in the future? Otherwise why not just delete the mutex_unlock() because it is dead code. regards, dan carpenter _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amdgpu: address the static checker warnings 2020-04-28 2:31 ` Quan, Evan @ 2020-04-28 11:01 ` Dan Carpenter 2020-04-29 2:32 ` Quan, Evan 0 siblings, 1 reply; 6+ messages in thread From: Dan Carpenter @ 2020-04-28 11:01 UTC (permalink / raw) To: Quan, Evan; +Cc: Deucher, Alexander, amd-gfx@lists.freedesktop.org On Tue, Apr 28, 2020 at 02:31:12AM +0000, Quan, Evan wrote: > Ping.. Are you asking me? It's really hard for me to read your emails. Normally email clients put "> " in front of the quoted parts of the email. Your email client doesn't line wrap at 72 characters either. :( > [Evan] No, that's true for SGPU setup only. For XGMI setup(multiple dgpus interconnected with bridges), the "hive" here is not NULL. Presumably you are talking about out of tree code so I can't comment on this. In the current linux-next we know that amdgpu_device_lock_adev() will only return false when "hive" is NULL. regards, dan carpenter _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] drm/amdgpu: address the static checker warnings 2020-04-28 11:01 ` Dan Carpenter @ 2020-04-29 2:32 ` Quan, Evan 0 siblings, 0 replies; 6+ messages in thread From: Quan, Evan @ 2020-04-29 2:32 UTC (permalink / raw) To: Dan Carpenter; +Cc: Deucher, Alexander, amd-gfx@lists.freedesktop.org Thanks, I think I get your point. With hive!= NULL, it never fails on the amdgpu_device_lock_adev(). Sorry for miss that. Will send a new patch. Regards, Evan -----Original Message----- From: Dan Carpenter <dan.carpenter@oracle.com> Sent: Tuesday, April 28, 2020 7:02 PM To: Quan, Evan <Evan.Quan@amd.com> Cc: amd-gfx@lists.freedesktop.org; Deucher, Alexander <Alexander.Deucher@amd.com> Subject: Re: [PATCH] drm/amdgpu: address the static checker warnings On Tue, Apr 28, 2020 at 02:31:12AM +0000, Quan, Evan wrote: > Ping.. Are you asking me? It's really hard for me to read your emails. Normally email clients put "> " in front of the quoted parts of the email. Your email client doesn't line wrap at 72 characters either. :( > [Evan] No, that's true for SGPU setup only. For XGMI setup(multiple dgpus interconnected with bridges), the "hive" here is not NULL. Presumably you are talking about out of tree code so I can't comment on this. In the current linux-next we know that amdgpu_device_lock_adev() will only return false when "hive" is NULL. regards, dan carpenter _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-04-29 2:32 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-04-24 10:41 [PATCH] drm/amdgpu: address the static checker warnings Evan Quan 2020-04-24 11:02 ` Dan Carpenter 2020-04-26 3:18 ` Quan, Evan 2020-04-28 2:31 ` Quan, Evan 2020-04-28 11:01 ` Dan Carpenter 2020-04-29 2:32 ` Quan, Evan
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.