* [PATCH] drm/amdgpu: Replace tmp_adev with hive in amdgpu_pci_slot_reset
@ 2025-04-09 13:18 Ce Sun
2025-04-09 13:32 ` Christian König
0 siblings, 1 reply; 7+ messages in thread
From: Ce Sun @ 2025-04-09 13:18 UTC (permalink / raw)
To: amd-gfx; +Cc: dan.carpenter, Ce Sun
Checking hive is more readable.
The following smatch warning:
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:6820 amdgpu_pci_slot_reset()
warn: iterator used outside loop: 'tmp_adev'
Fixes: 8ba904f54148 ("drm/amdgpu: Multi-GPU DPC recovery support")
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Signed-off-by: Ce Sun <cesun102@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 60269fba5745..eb0589a09a27 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -6846,7 +6846,7 @@ pci_ers_result_t amdgpu_pci_slot_reset(struct pci_dev *pdev)
dev_info(adev->dev, "PCIe error recovery succeeded\n");
} else {
dev_err(adev->dev, "PCIe error recovery failed, err:%d\n", r);
- if (tmp_adev) {
+ if (hive) {
list_for_each_entry(tmp_adev, &device_list, reset_list)
amdgpu_device_unset_mp1_state(tmp_adev);
amdgpu_device_unlock_reset_domain(adev->reset_domain);
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] drm/amdgpu: Replace tmp_adev with hive in amdgpu_pci_slot_reset
2025-04-09 13:18 [PATCH] drm/amdgpu: Replace tmp_adev with hive in amdgpu_pci_slot_reset Ce Sun
@ 2025-04-09 13:32 ` Christian König
0 siblings, 0 replies; 7+ messages in thread
From: Christian König @ 2025-04-09 13:32 UTC (permalink / raw)
To: Ce Sun, amd-gfx; +Cc: dan.carpenter
Am 09.04.25 um 15:18 schrieb Ce Sun:
> Checking hive is more readable.
>
> The following smatch warning:
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:6820 amdgpu_pci_slot_reset()
> warn: iterator used outside loop: 'tmp_adev'
Please also remove the setting of hive and tmp_adev to NULL at the declaration.
This now becomes unnecessary and I'm pretty sure that some automated checker could complain about that as well.
Apart from that looks good to me.
Thanks,
Christian.
>
> Fixes: 8ba904f54148 ("drm/amdgpu: Multi-GPU DPC recovery support")
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Signed-off-by: Ce Sun <cesun102@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 60269fba5745..eb0589a09a27 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -6846,7 +6846,7 @@ pci_ers_result_t amdgpu_pci_slot_reset(struct pci_dev *pdev)
> dev_info(adev->dev, "PCIe error recovery succeeded\n");
> } else {
> dev_err(adev->dev, "PCIe error recovery failed, err:%d\n", r);
> - if (tmp_adev) {
> + if (hive) {
> list_for_each_entry(tmp_adev, &device_list, reset_list)
> amdgpu_device_unset_mp1_state(tmp_adev);
> amdgpu_device_unlock_reset_domain(adev->reset_domain);
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] drm/amdgpu: Replace tmp_adev with hive in amdgpu_pci_slot_reset
@ 2025-04-09 13:39 Ce Sun
2025-04-09 13:44 ` Christian König
2025-04-09 13:49 ` Lazar, Lijo
0 siblings, 2 replies; 7+ messages in thread
From: Ce Sun @ 2025-04-09 13:39 UTC (permalink / raw)
To: amd-gfx; +Cc: dan.carpenter, Ce Sun
Checking hive is more readable.
The following smatch warning:
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:6820 amdgpu_pci_slot_reset()
warn: iterator used outside loop: 'tmp_adev'
Fixes: 8ba904f54148 ("drm/amdgpu: Multi-GPU DPC recovery support")
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Signed-off-by: Ce Sun <cesun102@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 60269fba5745..6fb234832ff3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -6784,8 +6784,8 @@ pci_ers_result_t amdgpu_pci_slot_reset(struct pci_dev *pdev)
struct drm_device *dev = pci_get_drvdata(pdev);
struct amdgpu_device *adev = drm_to_adev(dev);
struct amdgpu_reset_context reset_context;
- struct amdgpu_device *tmp_adev = NULL;
- struct amdgpu_hive_info *hive = NULL;
+ struct amdgpu_device *tmp_adev;
+ struct amdgpu_hive_info *hive;
struct list_head device_list;
int r = 0, i;
u32 memsize;
@@ -6846,7 +6846,7 @@ pci_ers_result_t amdgpu_pci_slot_reset(struct pci_dev *pdev)
dev_info(adev->dev, "PCIe error recovery succeeded\n");
} else {
dev_err(adev->dev, "PCIe error recovery failed, err:%d\n", r);
- if (tmp_adev) {
+ if (hive) {
list_for_each_entry(tmp_adev, &device_list, reset_list)
amdgpu_device_unset_mp1_state(tmp_adev);
amdgpu_device_unlock_reset_domain(adev->reset_domain);
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] drm/amdgpu: Replace tmp_adev with hive in amdgpu_pci_slot_reset
2025-04-09 13:39 Ce Sun
@ 2025-04-09 13:44 ` Christian König
2025-04-09 13:49 ` Lazar, Lijo
1 sibling, 0 replies; 7+ messages in thread
From: Christian König @ 2025-04-09 13:44 UTC (permalink / raw)
To: Ce Sun, amd-gfx; +Cc: dan.carpenter
Am 09.04.25 um 15:39 schrieb Ce Sun:
> Checking hive is more readable.
>
> The following smatch warning:
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:6820 amdgpu_pci_slot_reset()
> warn: iterator used outside loop: 'tmp_adev'
>
> Fixes: 8ba904f54148 ("drm/amdgpu: Multi-GPU DPC recovery support")
I think we should drop that. While the existing code didn't looked nice it would actually work as far as I can see.
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Signed-off-by: Ce Sun <cesun102@amd.com>
Apart from that Reviewed-by: Christian König <christian.koenig@amd.com>.
Regards,
Christian
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 60269fba5745..6fb234832ff3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -6784,8 +6784,8 @@ pci_ers_result_t amdgpu_pci_slot_reset(struct pci_dev *pdev)
> struct drm_device *dev = pci_get_drvdata(pdev);
> struct amdgpu_device *adev = drm_to_adev(dev);
> struct amdgpu_reset_context reset_context;
> - struct amdgpu_device *tmp_adev = NULL;
> - struct amdgpu_hive_info *hive = NULL;
> + struct amdgpu_device *tmp_adev;
> + struct amdgpu_hive_info *hive;
> struct list_head device_list;
> int r = 0, i;
> u32 memsize;
> @@ -6846,7 +6846,7 @@ pci_ers_result_t amdgpu_pci_slot_reset(struct pci_dev *pdev)
> dev_info(adev->dev, "PCIe error recovery succeeded\n");
> } else {
> dev_err(adev->dev, "PCIe error recovery failed, err:%d\n", r);
> - if (tmp_adev) {
> + if (hive) {
> list_for_each_entry(tmp_adev, &device_list, reset_list)
> amdgpu_device_unset_mp1_state(tmp_adev);
> amdgpu_device_unlock_reset_domain(adev->reset_domain);
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] drm/amdgpu: Replace tmp_adev with hive in amdgpu_pci_slot_reset
2025-04-09 13:39 Ce Sun
2025-04-09 13:44 ` Christian König
@ 2025-04-09 13:49 ` Lazar, Lijo
2025-04-09 13:56 ` Dan Carpenter
1 sibling, 1 reply; 7+ messages in thread
From: Lazar, Lijo @ 2025-04-09 13:49 UTC (permalink / raw)
To: Ce Sun, amd-gfx; +Cc: dan.carpenter
On 4/9/2025 7:09 PM, Ce Sun wrote:
> Checking hive is more readable.
>
> The following smatch warning:
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:6820 amdgpu_pci_slot_reset()
> warn: iterator used outside loop: 'tmp_adev'
>
> Fixes: 8ba904f54148 ("drm/amdgpu: Multi-GPU DPC recovery support")
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Signed-off-by: Ce Sun <cesun102@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 60269fba5745..6fb234832ff3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -6784,8 +6784,8 @@ pci_ers_result_t amdgpu_pci_slot_reset(struct pci_dev *pdev)
> struct drm_device *dev = pci_get_drvdata(pdev);
> struct amdgpu_device *adev = drm_to_adev(dev);
> struct amdgpu_reset_context reset_context;
> - struct amdgpu_device *tmp_adev = NULL;
> - struct amdgpu_hive_info *hive = NULL;
> + struct amdgpu_device *tmp_adev;
> + struct amdgpu_hive_info *hive;
> struct list_head device_list;
> int r = 0, i;
> u32 memsize;
> @@ -6846,7 +6846,7 @@ pci_ers_result_t amdgpu_pci_slot_reset(struct pci_dev *pdev)
> dev_info(adev->dev, "PCIe error recovery succeeded\n");
> } else {
> dev_err(adev->dev, "PCIe error recovery failed, err:%d\n", r);
> - if (tmp_adev) {
> + if (hive) {
This doesn't look correct. I think the original logic is to have a
single device list or multi-device list when there is a hive. Now this
skips the step for single device case.
Thanks,
Lijo
> list_for_each_entry(tmp_adev, &device_list, reset_list)
> amdgpu_device_unset_mp1_state(tmp_adev);
> amdgpu_device_unlock_reset_domain(adev->reset_domain);
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] drm/amdgpu: Replace tmp_adev with hive in amdgpu_pci_slot_reset
2025-04-09 13:49 ` Lazar, Lijo
@ 2025-04-09 13:56 ` Dan Carpenter
2025-04-10 8:09 ` Lazar, Lijo
0 siblings, 1 reply; 7+ messages in thread
From: Dan Carpenter @ 2025-04-09 13:56 UTC (permalink / raw)
To: Lazar, Lijo; +Cc: Ce Sun, amd-gfx
On Wed, Apr 09, 2025 at 07:19:25PM +0530, Lazar, Lijo wrote:
>
>
> On 4/9/2025 7:09 PM, Ce Sun wrote:
> > Checking hive is more readable.
> >
> > The following smatch warning:
> > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:6820 amdgpu_pci_slot_reset()
> > warn: iterator used outside loop: 'tmp_adev'
> >
> > Fixes: 8ba904f54148 ("drm/amdgpu: Multi-GPU DPC recovery support")
> > Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> > Signed-off-by: Ce Sun <cesun102@amd.com>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > index 60269fba5745..6fb234832ff3 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > @@ -6784,8 +6784,8 @@ pci_ers_result_t amdgpu_pci_slot_reset(struct pci_dev *pdev)
> > struct drm_device *dev = pci_get_drvdata(pdev);
> > struct amdgpu_device *adev = drm_to_adev(dev);
> > struct amdgpu_reset_context reset_context;
> > - struct amdgpu_device *tmp_adev = NULL;
> > - struct amdgpu_hive_info *hive = NULL;
> > + struct amdgpu_device *tmp_adev;
> > + struct amdgpu_hive_info *hive;
> > struct list_head device_list;
> > int r = 0, i;
> > u32 memsize;
> > @@ -6846,7 +6846,7 @@ pci_ers_result_t amdgpu_pci_slot_reset(struct pci_dev *pdev)
> > dev_info(adev->dev, "PCIe error recovery succeeded\n");
> > } else {
> > dev_err(adev->dev, "PCIe error recovery failed, err:%d\n", r);
> > - if (tmp_adev) {
> > + if (hive) {
>
> This doesn't look correct. I think the original logic is to have a
> single device list or multi-device list when there is a hive. Now this
> skips the step for single device case.
>
I don't know what the original intentions were, but what I know is that
this code will behave exactly the same as the original code.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] drm/amdgpu: Replace tmp_adev with hive in amdgpu_pci_slot_reset
2025-04-09 13:56 ` Dan Carpenter
@ 2025-04-10 8:09 ` Lazar, Lijo
0 siblings, 0 replies; 7+ messages in thread
From: Lazar, Lijo @ 2025-04-10 8:09 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Ce Sun, amd-gfx
On 4/9/2025 7:26 PM, Dan Carpenter wrote:
> On Wed, Apr 09, 2025 at 07:19:25PM +0530, Lazar, Lijo wrote:
>>
>>
>> On 4/9/2025 7:09 PM, Ce Sun wrote:
>>> Checking hive is more readable.
>>>
>>> The following smatch warning:
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:6820 amdgpu_pci_slot_reset()
>>> warn: iterator used outside loop: 'tmp_adev'
>>>
>>> Fixes: 8ba904f54148 ("drm/amdgpu: Multi-GPU DPC recovery support")
>>> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
>>> Signed-off-by: Ce Sun <cesun102@amd.com>
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 6 +++---
>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> index 60269fba5745..6fb234832ff3 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> @@ -6784,8 +6784,8 @@ pci_ers_result_t amdgpu_pci_slot_reset(struct pci_dev *pdev)
>>> struct drm_device *dev = pci_get_drvdata(pdev);
>>> struct amdgpu_device *adev = drm_to_adev(dev);
>>> struct amdgpu_reset_context reset_context;
>>> - struct amdgpu_device *tmp_adev = NULL;
>>> - struct amdgpu_hive_info *hive = NULL;
>>> + struct amdgpu_device *tmp_adev;
>>> + struct amdgpu_hive_info *hive;
>>> struct list_head device_list;
>>> int r = 0, i;
>>> u32 memsize;
>>> @@ -6846,7 +6846,7 @@ pci_ers_result_t amdgpu_pci_slot_reset(struct pci_dev *pdev)
>>> dev_info(adev->dev, "PCIe error recovery succeeded\n");
>>> } else {
>>> dev_err(adev->dev, "PCIe error recovery failed, err:%d\n", r);
>>> - if (tmp_adev) {
>>> + if (hive) {
>>
>> This doesn't look correct. I think the original logic is to have a
>> single device list or multi-device list when there is a hive. Now this
>> skips the step for single device case.
>>
>
> I don't know what the original intentions were, but what I know is that
> this code will behave exactly the same as the original code.
>
Yes, this correct with respect to the new patch. But the patch which
triggered this warning actually broke the single device handling path.
Thanks,
Lijo
> regards,
> dan carpenter
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-04-10 13:08 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-09 13:18 [PATCH] drm/amdgpu: Replace tmp_adev with hive in amdgpu_pci_slot_reset Ce Sun
2025-04-09 13:32 ` Christian König
-- strict thread matches above, loose matches on Subject: below --
2025-04-09 13:39 Ce Sun
2025-04-09 13:44 ` Christian König
2025-04-09 13:49 ` Lazar, Lijo
2025-04-09 13:56 ` Dan Carpenter
2025-04-10 8:09 ` Lazar, Lijo
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.