[PATCH] drm/amdgpu: Skip put_reset_domain if it doesnt exist

[PATCH] drm/amdgpu: Skip put_reset_domain if it doesnt exist

[Vignesh Chander]

Change-Id: Ifd6121fb94db3fadaa1dee61d35699abe1259409
Signed-off-by: Vignesh Chander <Vignesh.Chander@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
index 47159e9a0884..80fb6ef929e5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
@@ -217,7 +217,8 @@ static void amdgpu_xgmi_hive_release(struct kobject *kobj)
 	struct amdgpu_hive_info *hive = container_of(
 		kobj, struct amdgpu_hive_info, kobj);
 
-	amdgpu_reset_put_reset_domain(hive->reset_domain);
+	if (hive->reset_domain)
+		amdgpu_reset_put_reset_domain(hive->reset_domain);
 	hive->reset_domain = NULL;
 
 	mutex_destroy(&hive->hive_lock);
-- 
2.25.1

Re: [PATCH] drm/amdgpu: Skip put_reset_domain if it doesnt exist

[Hamza Mahfooz]

On 2022-09-28 13:37, Vignesh Chander wrote:
> Change-Id: Ifd6121fb94db3fadaa1dee61d35699abe1259409
> Signed-off-by: Vignesh Chander <Vignesh.Chander@amd.com>

Please remove the Change-Id and provide commit message body.

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> index 47159e9a0884..80fb6ef929e5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> @@ -217,7 +217,8 @@ static void amdgpu_xgmi_hive_release(struct kobject *kobj)
>   	struct amdgpu_hive_info *hive = container_of(
>   		kobj, struct amdgpu_hive_info, kobj);
>   
> -	amdgpu_reset_put_reset_domain(hive->reset_domain);
> +	if (hive->reset_domain)
> +		amdgpu_reset_put_reset_domain(hive->reset_domain);
>   	hive->reset_domain = NULL;
>   
>   	mutex_destroy(&hive->hive_lock);

-- 
Hamza

Re: [PATCH] drm/amdgpu: Skip put_reset_domain if it doesnt exist

[Christian König]

Am 28.09.22 um 19:37 schrieb Vignesh Chander:
> Change-Id: Ifd6121fb94db3fadaa1dee61d35699abe1259409
> Signed-off-by: Vignesh Chander <Vignesh.Chander@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> index 47159e9a0884..80fb6ef929e5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> @@ -217,7 +217,8 @@ static void amdgpu_xgmi_hive_release(struct kobject *kobj)
>   	struct amdgpu_hive_info *hive = container_of(
>   		kobj, struct amdgpu_hive_info, kobj);
>   
> -	amdgpu_reset_put_reset_domain(hive->reset_domain);
> +	if (hive->reset_domain)
> +		amdgpu_reset_put_reset_domain(hive->reset_domain);

As general coding style rule _put functions are usually NULL pointer 
tolerant. So this shouldn't be necessary.

If that really is necessary then please adjust the 
amdgpu_reset_put_reset_domain() function instead.

Christian.

>   	hive->reset_domain = NULL;
>   
>   	mutex_destroy(&hive->hive_lock);

RE: [PATCH] drm/amdgpu: Skip put_reset_domain if it doesnt exist

[Liu, Shaoyun]

[AMD Official Use Only - General]

Please add description like under sriov xgmi configuration , the hive reset is handled by host driver , hive->reset_domain  is not been  in initialized  so  need to skip it .

Regards
Shaoyun.liu


-----Original Message-----
From: Chander, Vignesh <Vignesh.Chander@amd.com>
Sent: Wednesday, September 28, 2022 1:38 PM
To: amd-gfx@lists.freedesktop.org
Cc: Liu, Shaoyun <Shaoyun.Liu@amd.com>; Chander, Vignesh <Vignesh.Chander@amd.com>
Subject: [PATCH] drm/amdgpu: Skip put_reset_domain if it doesnt exist

Change-Id: Ifd6121fb94db3fadaa1dee61d35699abe1259409
Signed-off-by: Vignesh Chander <Vignesh.Chander@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
index 47159e9a0884..80fb6ef929e5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
@@ -217,7 +217,8 @@ static void amdgpu_xgmi_hive_release(struct kobject *kobj)
        struct amdgpu_hive_info *hive = container_of(
                kobj, struct amdgpu_hive_info, kobj);

-       amdgpu_reset_put_reset_domain(hive->reset_domain);
+       if (hive->reset_domain)
+               amdgpu_reset_put_reset_domain(hive->reset_domain);
        hive->reset_domain = NULL;

        mutex_destroy(&hive->hive_lock);
--
2.25.1

[PATCH] drm/amdgpu: Skip put_reset_domain if it doesnt exist

[Vignesh Chander]

For sriov, the reset domain is no longer created so need to check if it
exists before doing a put.
Signed-off-by: Vignesh Chander <Vignesh.Chander@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
index 47159e9a0884..80fb6ef929e5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
@@ -217,7 +217,8 @@ static void amdgpu_xgmi_hive_release(struct kobject *kobj)
 	struct amdgpu_hive_info *hive = container_of(
 		kobj, struct amdgpu_hive_info, kobj);
 
-	amdgpu_reset_put_reset_domain(hive->reset_domain);
+	if (hive->reset_domain)
+		amdgpu_reset_put_reset_domain(hive->reset_domain);
 	hive->reset_domain = NULL;
 
 	mutex_destroy(&hive->hive_lock);
-- 
2.25.1

Re: [PATCH] drm/amdgpu: Skip put_reset_domain if it doesnt exist

[Christian König]

Am 28.09.22 um 19:43 schrieb Vignesh Chander:
> For sriov, the reset domain is no longer created so need to check if it
> exists before doing a put.

Why is the reset domain no longer created for SRIOV?

Regards,
Christian.

> Signed-off-by: Vignesh Chander <Vignesh.Chander@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> index 47159e9a0884..80fb6ef929e5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> @@ -217,7 +217,8 @@ static void amdgpu_xgmi_hive_release(struct kobject *kobj)
>   	struct amdgpu_hive_info *hive = container_of(
>   		kobj, struct amdgpu_hive_info, kobj);
>   
> -	amdgpu_reset_put_reset_domain(hive->reset_domain);
> +	if (hive->reset_domain)
> +		amdgpu_reset_put_reset_domain(hive->reset_domain);
>   	hive->reset_domain = NULL;
>   
>   	mutex_destroy(&hive->hive_lock);

RE: [PATCH] drm/amdgpu: Skip put_reset_domain if it doesnt exist

[Chander, Vignesh]

[AMD Official Use Only - General]

Hi Christian, 

It is because the host driver handles the reset for xgmi sriov case. I will update the commit body as

For xgmi sriov, the reset is handled by host driver and hive->reset_domain
is not initialized so need to check if it exists before doing a put.

Regards, 

VIGNESH CHANDER 
Senior Software Development Engineer  |  AMD
SW GPU Virtualization
----------------------------------------------------------------------------------------------------------------------------------
1 Commerce Valley Dr E, Markham, ON L3T 7X6, Canada
  

-----Original Message-----
From: Christian König <ckoenig.leichtzumerken@gmail.com> 
Sent: Wednesday, September 28, 2022 1:45 PM
To: Chander, Vignesh <Vignesh.Chander@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Liu, Shaoyun <Shaoyun.Liu@amd.com>
Subject: Re: [PATCH] drm/amdgpu: Skip put_reset_domain if it doesnt exist

Am 28.09.22 um 19:43 schrieb Vignesh Chander:
> For sriov, the reset domain is no longer created so need to check if it
> exists before doing a put.

Why is the reset domain no longer created for SRIOV?

Regards,
Christian.

> Signed-off-by: Vignesh Chander <Vignesh.Chander@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> index 47159e9a0884..80fb6ef929e5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> @@ -217,7 +217,8 @@ static void amdgpu_xgmi_hive_release(struct kobject *kobj)
>   	struct amdgpu_hive_info *hive = container_of(
>   		kobj, struct amdgpu_hive_info, kobj);
>   
> -	amdgpu_reset_put_reset_domain(hive->reset_domain);
> +	if (hive->reset_domain)
> +		amdgpu_reset_put_reset_domain(hive->reset_domain);
>   	hive->reset_domain = NULL;
>   
>   	mutex_destroy(&hive->hive_lock);

[PATCH] drm/amdgpu: Skip put_reset_domain if it doesnt exist

[Vignesh Chander]

For xgmi sriov, the reset is handled by host driver and hive->reset_domain
is not initialized so need to check if it exists before doing a put.
Signed-off-by: Vignesh Chander <Vignesh.Chander@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
index 47159e9a0884..80fb6ef929e5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
@@ -217,7 +217,8 @@ static void amdgpu_xgmi_hive_release(struct kobject *kobj)
 	struct amdgpu_hive_info *hive = container_of(
 		kobj, struct amdgpu_hive_info, kobj);
 
-	amdgpu_reset_put_reset_domain(hive->reset_domain);
+	if (hive->reset_domain)
+		amdgpu_reset_put_reset_domain(hive->reset_domain);
 	hive->reset_domain = NULL;
 
 	mutex_destroy(&hive->hive_lock);
-- 
2.25.1

Re: [PATCH] drm/amdgpu: Skip put_reset_domain if it doesnt exist

[Christian König]

Am 28.09.22 um 20:22 schrieb Vignesh Chander:
> For xgmi sriov, the reset is handled by host driver and hive->reset_domain
> is not initialized so need to check if it exists before doing a put.

Once more: General practice is to make the *_put_*() functions NULL 
tolerant.

So rather make this than this patch here.

Regards,
Christian.

> Signed-off-by: Vignesh Chander <Vignesh.Chander@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> index 47159e9a0884..80fb6ef929e5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> @@ -217,7 +217,8 @@ static void amdgpu_xgmi_hive_release(struct kobject *kobj)
>   	struct amdgpu_hive_info *hive = container_of(
>   		kobj, struct amdgpu_hive_info, kobj);
>   
> -	amdgpu_reset_put_reset_domain(hive->reset_domain);
> +	if (hive->reset_domain)
> +		amdgpu_reset_put_reset_domain(hive->reset_domain);
>   	hive->reset_domain = NULL;
>   
>   	mutex_destroy(&hive->hive_lock);

[PATCH] drm/amdgpu: Skip put_reset_domain if it doesnt exist

[Vignesh Chander]

For xgmi sriov, the reset is handled by host driver and hive->reset_domain
is not initialized so need to check if it exists before doing a put.
Signed-off-by: Vignesh Chander <Vignesh.Chander@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
index dc43fcb93eac..f5318fedf2f0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
@@ -113,7 +113,8 @@ static inline bool amdgpu_reset_get_reset_domain(struct amdgpu_reset_domain *dom
 
 static inline void amdgpu_reset_put_reset_domain(struct amdgpu_reset_domain *domain)
 {
-	kref_put(&domain->refcount, amdgpu_reset_destroy_reset_domain);
+	if (domain)
+		kref_put(&domain->refcount, amdgpu_reset_destroy_reset_domain);
 }
 
 static inline bool amdgpu_reset_domain_schedule(struct amdgpu_reset_domain *domain,
-- 
2.25.1

Re: [PATCH] drm/amdgpu: Skip put_reset_domain if it doesnt exist

[Liu, Shaoyun]

[-- Attachment #1: Type: text/plain, Size: 1439 bytes --]

Looks OK to me.
Reviewed by : shaoyun.liu <shaoyun.liu@amd.com>


________________________________
From: Chander, Vignesh <Vignesh.Chander@amd.com>
Sent: September 28, 2022 3:03 PM
To: amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
Cc: Liu, Shaoyun <Shaoyun.Liu@amd.com>; Chander, Vignesh <Vignesh.Chander@amd.com>
Subject: [PATCH] drm/amdgpu: Skip put_reset_domain if it doesnt exist

For xgmi sriov, the reset is handled by host driver and hive->reset_domain
is not initialized so need to check if it exists before doing a put.
Signed-off-by: Vignesh Chander <Vignesh.Chander@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
index dc43fcb93eac..f5318fedf2f0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
@@ -113,7 +113,8 @@ static inline bool amdgpu_reset_get_reset_domain(struct amdgpu_reset_domain *dom

 static inline void amdgpu_reset_put_reset_domain(struct amdgpu_reset_domain *domain)
 {
-       kref_put(&domain->refcount, amdgpu_reset_destroy_reset_domain);
+       if (domain)
+               kref_put(&domain->refcount, amdgpu_reset_destroy_reset_domain);
 }

 static inline bool amdgpu_reset_domain_schedule(struct amdgpu_reset_domain *domain,
--
2.25.1


[-- Attachment #2: Type: text/html, Size: 2566 bytes --]