Re: [PATCH] drm/amdgpu: fix memory leak in amdgpu_fru_get_product_info()

[Luben Tuikov]

On 2023-09-22 16:58, Luben Tuikov wrote:
> On 2023-09-22 01:27, Yang Wang wrote:
>> fix a memory leak that occurs when csum is 0,
>> the origin function will return directly and forgets to free 'pia' resource.
>>
>> Fixes: 0dbf2c562625 ("drm/amdgpu: Interpret IPMI data for product 
>> information (v2)")
>>
>> CC: Luben Tuikov 
>> Signed-off-by: Yang Wang 
> 
> Ah, yes, we should free "pia". Good catch!
> 
> Reviewed-by: Luben Tuikov 

Retracted--see my follow-up email of making this a one-liner change, instead
of adding a whole bunch of changes which are unnecessary to fix this memory 
leak.

Regards,
Luben

> 
> Regards,
> Luben
> 
>> ---
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c | 11 ++-
>>  1 file changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
>> index 401651f28ba2..50b6eb447726 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
>> @@ -111,7 +111,7 @@ int amdgpu_fru_get_product_info(struct amdgpu_device 
>> *adev)
>>  {
>>  unsigned char buf[8], *pia;
>>  u32 addr, fru_addr;
>> -int size, len;
>> +int size, len, ret = 0;
>>  u8 csum;
>>  
>>  if (!is_fru_eeprom_supported(adev, _addr))
>> @@ -171,16 +171,17 @@ int amdgpu_fru_get_product_info(struct amdgpu_device 
>> *adev)
>>  /* Read the whole PIA. */
>>  len = amdgpu_eeprom_read(adev->pm.fru_eeprom_i2c_bus, addr, pia, size);
>>  if (len != size) {
>> -kfree(pia);
>>  DRM_ERROR("Couldn't read the Product Info Area: %d", len);
>> -return len < 0 ? len : -EIO;
>> +ret = len < 0 ? len : -EIO;
>> +goto Out;
>>  }
> 
> 
>>  
>>  for (csum = 0; size > 0; size--)
>>  csum += pia[size - 1];
>>  if (csum) {
>>  DRM_ERROR("Bad Product Info Area checksum: 0x%02x", csum);
>> -return -EIO;
>> +ret = -EIO;
>> +goto Out;
>>  }
>>  
>>  /* Now extract useful information from the PIA.
>> @@ -220,7 +221,7 @@ int amdgpu_fru_get_product_info(struct amdgpu_device 
>> *adev)
>>  adev->serial[sizeof(adev->serial) - 1] = '\0';
>>  Out:
>>  kfree(pia);
>> -return 0;
>> +return ret;
>>  }
>>  
>>  /**
> 

-- 
Regards,
Luben

Re: [PATCH] drm/amdgpu: fix memory leak in amdgpu_fru_get_product_info()

[Luben Tuikov]

On 2023-09-22 01:27, Yang Wang wrote:
> fix a memory leak that occurs when csum is 0,
> the origin function will return directly and forgets to free 'pia' resource.
> 
> Fixes: 0dbf2c562625 ("drm/amdgpu: Interpret IPMI data for product information 
> (v2)")
> 
> CC: Luben Tuikov 
> Signed-off-by: Yang Wang 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c | 11 ++-
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
> index 401651f28ba2..50b6eb447726 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
> @@ -111,7 +111,7 @@ int amdgpu_fru_get_product_info(struct amdgpu_device 
> *adev)
>  {
>   unsigned char buf[8], *pia;
>   u32 addr, fru_addr;
> - int size, len;
> + int size, len, ret = 0;
>   u8 csum;
>  
>   if (!is_fru_eeprom_supported(adev, _addr))
> @@ -171,16 +171,17 @@ int amdgpu_fru_get_product_info(struct amdgpu_device 
> *adev)
>   /* Read the whole PIA. */
>   len = amdgpu_eeprom_read(adev->pm.fru_eeprom_i2c_bus, addr, pia, size);
>   if (len != size) {
> - kfree(pia);
>   DRM_ERROR("Couldn't read the Product Info Area: %d", len);
> - return len < 0 ? len : -EIO;
> + ret = len < 0 ? len : -EIO;
> + goto Out;
>   }
>  
>   for (csum = 0; size > 0; size--)
>   csum += pia[size - 1];
>   if (csum) {
>   DRM_ERROR("Bad Product Info Area checksum: 0x%02x", csum);
> - return -EIO;
> + ret = -EIO;

Actually the memory leak is ONLY here.

I wonder if you could've simply added a

kfree(pia);

> + goto Out;
>   }

before the goto Out; which would result in a one-line change.

Yeah, please do that instead.

So, don't add "ret" and what not. Just add the one liner "kfree(pia);" before
the "goto Out;" and make it a SINGLE-LINE change to fix this memory leak.

You don't need so many (formulaic) changes of adding "ret" and what not.
Just a one-liner change, please.

>  
>   /* Now extract useful information from the PIA.
> @@ -220,7 +221,7 @@ int amdgpu_fru_get_product_info(struct amdgpu_device 
> *adev)
>   adev->serial[sizeof(adev->serial) - 1] = '\0';
>  Out:
>   kfree(pia);
> - return 0;
> + return ret;
>  }
>  
>  /**

-- 
Regards,
Luben

Re: [PATCH] drm/amdgpu: fix memory leak in amdgpu_fru_get_product_info()

[Luben Tuikov]

On 2023-09-22 01:27, Yang Wang wrote:
> fix a memory leak that occurs when csum is 0,
> the origin function will return directly and forgets to free 'pia' resource.
> 
> Fixes: 0dbf2c562625 ("drm/amdgpu: Interpret IPMI data for product information 
> (v2)")
> 
> CC: Luben Tuikov 
> Signed-off-by: Yang Wang 

Ah, yes, we should free "pia". Good catch!

Reviewed-by: Luben Tuikov 

Regards,
Luben

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c | 11 ++-
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
> index 401651f28ba2..50b6eb447726 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
> @@ -111,7 +111,7 @@ int amdgpu_fru_get_product_info(struct amdgpu_device 
> *adev)
>  {
>   unsigned char buf[8], *pia;
>   u32 addr, fru_addr;
> - int size, len;
> + int size, len, ret = 0;
>   u8 csum;
>  
>   if (!is_fru_eeprom_supported(adev, _addr))
> @@ -171,16 +171,17 @@ int amdgpu_fru_get_product_info(struct amdgpu_device 
> *adev)
>   /* Read the whole PIA. */
>   len = amdgpu_eeprom_read(adev->pm.fru_eeprom_i2c_bus, addr, pia, size);
>   if (len != size) {
> - kfree(pia);
>   DRM_ERROR("Couldn't read the Product Info Area: %d", len);
> - return len < 0 ? len : -EIO;
> + ret = len < 0 ? len : -EIO;
> + goto Out;
>   }


>  
>   for (csum = 0; size > 0; size--)
>   csum += pia[size - 1];
>   if (csum) {
>   DRM_ERROR("Bad Product Info Area checksum: 0x%02x", csum);
> - return -EIO;
> + ret = -EIO;
> + goto Out;
>   }
>  
>   /* Now extract useful information from the PIA.
> @@ -220,7 +221,7 @@ int amdgpu_fru_get_product_info(struct amdgpu_device 
> *adev)
>   adev->serial[sizeof(adev->serial) - 1] = '\0';
>  Out:
>   kfree(pia);
> - return 0;
> + return ret;
>  }
>  
>  /**

-- 
Regards,
Luben

Re: [PATCH] drm/amdgpu: fix memory leak in amdgpu_fru_get_product_info()

[Alex Deucher]

On Fri, Sep 22, 2023 at 2:06 AM Yang Wang  wrote:
>
> fix a memory leak that occurs when csum is 0,
> the origin function will return directly and forgets to free 'pia' resource.
>
> Fixes: 0dbf2c562625 ("drm/amdgpu: Interpret IPMI data for product information 
> (v2)")
>
> CC: Luben Tuikov 
> Signed-off-by: Yang Wang 

Reviewed-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c | 11 ++-
>  1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
> index 401651f28ba2..50b6eb447726 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
> @@ -111,7 +111,7 @@ int amdgpu_fru_get_product_info(struct amdgpu_device 
> *adev)
>  {
> unsigned char buf[8], *pia;
> u32 addr, fru_addr;
> -   int size, len;
> +   int size, len, ret = 0;
> u8 csum;
>
> if (!is_fru_eeprom_supported(adev, _addr))
> @@ -171,16 +171,17 @@ int amdgpu_fru_get_product_info(struct amdgpu_device 
> *adev)
> /* Read the whole PIA. */
> len = amdgpu_eeprom_read(adev->pm.fru_eeprom_i2c_bus, addr, pia, 
> size);
> if (len != size) {
> -   kfree(pia);
> DRM_ERROR("Couldn't read the Product Info Area: %d", len);
> -   return len < 0 ? len : -EIO;
> +   ret = len < 0 ? len : -EIO;
> +   goto Out;
> }
>
> for (csum = 0; size > 0; size--)
> csum += pia[size - 1];
> if (csum) {
> DRM_ERROR("Bad Product Info Area checksum: 0x%02x", csum);
> -   return -EIO;
> +   ret = -EIO;
> +   goto Out;
> }
>
> /* Now extract useful information from the PIA.
> @@ -220,7 +221,7 @@ int amdgpu_fru_get_product_info(struct amdgpu_device 
> *adev)
> adev->serial[sizeof(adev->serial) - 1] = '\0';
>  Out:
> kfree(pia);
> -   return 0;
> +   return ret;
>  }
>
>  /**
> --
> 2.34.1
>

[PATCH] drm/amdgpu: fix memory leak in amdgpu_fru_get_product_info()

[Yang Wang]

fix a memory leak that occurs when csum is 0,
the origin function will return directly and forgets to free 'pia' resource.

Fixes: 0dbf2c562625 ("drm/amdgpu: Interpret IPMI data for product information 
(v2)")

CC: Luben Tuikov 
Signed-off-by: Yang Wang 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
index 401651f28ba2..50b6eb447726 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
@@ -111,7 +111,7 @@ int amdgpu_fru_get_product_info(struct amdgpu_device *adev)
 {
unsigned char buf[8], *pia;
u32 addr, fru_addr;
-   int size, len;
+   int size, len, ret = 0;
u8 csum;
 
if (!is_fru_eeprom_supported(adev, _addr))
@@ -171,16 +171,17 @@ int amdgpu_fru_get_product_info(struct amdgpu_device 
*adev)
/* Read the whole PIA. */
len = amdgpu_eeprom_read(adev->pm.fru_eeprom_i2c_bus, addr, pia, size);
if (len != size) {
-   kfree(pia);
DRM_ERROR("Couldn't read the Product Info Area: %d", len);
-   return len < 0 ? len : -EIO;
+   ret = len < 0 ? len : -EIO;
+   goto Out;
}
 
for (csum = 0; size > 0; size--)
csum += pia[size - 1];
if (csum) {
DRM_ERROR("Bad Product Info Area checksum: 0x%02x", csum);
-   return -EIO;
+   ret = -EIO;
+   goto Out;
}
 
/* Now extract useful information from the PIA.
@@ -220,7 +221,7 @@ int amdgpu_fru_get_product_info(struct amdgpu_device *adev)
adev->serial[sizeof(adev->serial) - 1] = '\0';
 Out:
kfree(pia);
-   return 0;
+   return ret;
 }
 
 /**
-- 
2.34.1

Re: [PATCH] drm/amdgpu: fix memory leak in mes self test

[Christian König]

Am 21.04.23 um 09:06 schrieb Jack Xiao:

The fences associated with mes queue have to be freed
up during amdgpu_ring_fini.

Signed-off-by: Jack Xiao 


Well big NAK to this! The fences are supposed to be freed by the fence 
handling code.


Christian.


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
index dc474b809604..4a35cee4cb83 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
@@ -361,6 +361,8 @@ void amdgpu_ring_fini(struct amdgpu_ring *ring)
amdgpu_bo_free_kernel(>ring_obj,
  >gpu_addr,
  (void **)>ring);
+   } else {
+   kfree(ring->fence_drv.fences);
}
  
  	dma_fence_put(ring->vmid_wait);

RE: [PATCH] drm/amdgpu: fix memory leak in mes self test

[Zhang, Hawking]

[AMD Official Use Only - General]

Reviewed-by: Hawking Zhang 

Regards,
Hawking
-Original Message-
From: Xiao, Jack 
Sent: Friday, April 21, 2023 15:06
To: amd-gfx@lists.freedesktop.org; Zhang, Hawking 
Cc: Xiao, Jack 
Subject: [PATCH] drm/amdgpu: fix memory leak in mes self test

The fences associated with mes queue have to be freed up during 
amdgpu_ring_fini.

Signed-off-by: Jack Xiao 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
index dc474b809604..4a35cee4cb83 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
@@ -361,6 +361,8 @@ void amdgpu_ring_fini(struct amdgpu_ring *ring)
amdgpu_bo_free_kernel(>ring_obj,
  >gpu_addr,
  (void **)>ring);
+   } else {
+   kfree(ring->fence_drv.fences);
}

dma_fence_put(ring->vmid_wait);
--
2.37.3

[PATCH] drm/amdgpu: fix memory leak in mes self test

[Jack Xiao]

The fences associated with mes queue have to be freed
up during amdgpu_ring_fini.

Signed-off-by: Jack Xiao 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
index dc474b809604..4a35cee4cb83 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
@@ -361,6 +361,8 @@ void amdgpu_ring_fini(struct amdgpu_ring *ring)
amdgpu_bo_free_kernel(>ring_obj,
  >gpu_addr,
  (void **)>ring);
+   } else {
+   kfree(ring->fence_drv.fences);
}
 
dma_fence_put(ring->vmid_wait);
-- 
2.37.3

Re: [PATCH] drm/amdgpu: Fix memory leak in amdgpu_cs_pass1

[Christian König]

Am 10.11.22 um 15:33 schrieb Dong Chenchen:

When p->gang_size equals 0, amdgpu_cs_pass1() will return directly
without freeing chunk_array, which will cause a memory leak issue,
this patch fixes it.

Fixes: 4624459c84d7 ("drm/amdgpu: add gang submit frontend v6")
Signed-off-by: Dong Chenchen 


Good catch, thanks. Patch is Reviewed-by: Christian König 




---
  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 1bbd39b3b0fc..0e24d6b80e0b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -287,8 +287,10 @@ static int amdgpu_cs_pass1(struct amdgpu_cs_parser *p,
}
}
  
-	if (!p->gang_size)

-   return -EINVAL;
+   if (!p->gang_size) {
+   ret = -EINVAL;
+   goto free_partial_kdata;
+   }
  
  	for (i = 0; i < p->gang_size; ++i) {

ret = amdgpu_job_alloc(p->adev, num_ibs[i], >jobs[i], vm);

Re: [PATCH] drm/amdgpu: Fix memory leak in amdgpu_cs_pass1

[Alex Deucher]

On Thu, Nov 10, 2022 at 1:08 PM Luben Tuikov  wrote:
>
> Thanks for fixing this.
>
> Please add a Cc tag to stable, and repost.

No need for stable.  This patch just went upstream in 6.1, so I can
include it in my 6.1 fixes pull next week.  Applied.

Thanks!

Alex

>
> Reviewed-by: Luben Tuikov 
>
> Regards,
> Luben
>
> On 2022-11-10 09:33, Dong Chenchen wrote:
> > When p->gang_size equals 0, amdgpu_cs_pass1() will return directly
> > without freeing chunk_array, which will cause a memory leak issue,
> > this patch fixes it.
> >
> > Fixes: 4624459c84d7 ("drm/amdgpu: add gang submit frontend v6")
> > Signed-off-by: Dong Chenchen 
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 6 --
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> > index 1bbd39b3b0fc..0e24d6b80e0b 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> > @@ -287,8 +287,10 @@ static int amdgpu_cs_pass1(struct amdgpu_cs_parser *p,
> >   }
> >   }
> >
> > - if (!p->gang_size)
> > - return -EINVAL;
> > + if (!p->gang_size) {
> > + ret = -EINVAL;
> > + goto free_partial_kdata;
> > + }
> >
> >   for (i = 0; i < p->gang_size; ++i) {
> >   ret = amdgpu_job_alloc(p->adev, num_ibs[i], >jobs[i], vm);
>

Re: [PATCH] drm/amdgpu: Fix memory leak in amdgpu_cs_pass1

[Luben Tuikov]

Thanks for fixing this.

Please add a Cc tag to stable, and repost.

Reviewed-by: Luben Tuikov 

Regards,
Luben

On 2022-11-10 09:33, Dong Chenchen wrote:
> When p->gang_size equals 0, amdgpu_cs_pass1() will return directly
> without freeing chunk_array, which will cause a memory leak issue,
> this patch fixes it.
> 
> Fixes: 4624459c84d7 ("drm/amdgpu: add gang submit frontend v6")
> Signed-off-by: Dong Chenchen 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 1bbd39b3b0fc..0e24d6b80e0b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -287,8 +287,10 @@ static int amdgpu_cs_pass1(struct amdgpu_cs_parser *p,
>   }
>   }
>  
> - if (!p->gang_size)
> - return -EINVAL;
> + if (!p->gang_size) {
> + ret = -EINVAL;
> + goto free_partial_kdata;
> + }
>  
>   for (i = 0; i < p->gang_size; ++i) {
>   ret = amdgpu_job_alloc(p->adev, num_ibs[i], >jobs[i], vm);

[PATCH] drm/amdgpu: Fix memory leak in amdgpu_cs_pass1

[Dong Chenchen]

When p->gang_size equals 0, amdgpu_cs_pass1() will return directly
without freeing chunk_array, which will cause a memory leak issue,
this patch fixes it.

Fixes: 4624459c84d7 ("drm/amdgpu: add gang submit frontend v6")
Signed-off-by: Dong Chenchen 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 1bbd39b3b0fc..0e24d6b80e0b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -287,8 +287,10 @@ static int amdgpu_cs_pass1(struct amdgpu_cs_parser *p,
}
}
 
-   if (!p->gang_size)
-   return -EINVAL;
+   if (!p->gang_size) {
+   ret = -EINVAL;
+   goto free_partial_kdata;
+   }
 
for (i = 0; i < p->gang_size; ++i) {
ret = amdgpu_job_alloc(p->adev, num_ibs[i], >jobs[i], vm);
-- 
2.25.1

Re: [PATCH] drm/amdgpu: Fix memory leak in hpd_rx_irq_create_workqueue()

[Alex Deucher]

Applied.  Thanks!

On Mon, Sep 12, 2022 at 10:12 PM Rafael Mendonca  wrote:
>
> If construction of the array of work queues to handle hpd_rx_irq offload
> work fails, we need to unwind. Destroy all the created workqueues and
> the allocated memory for the hpd_rx_irq_offload_work_queue struct array.
>
> Fixes: 8e794421bc98 ("drm/amd/display: Fork thread to offload work of 
> hpd_rx_irq")
> Signed-off-by: Rafael Mendonca 
> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 10 +-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 5140d9c2bf3b..6a2e455c5466 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -1295,13 +1295,21 @@ static struct hpd_rx_irq_offload_work_queue 
> *hpd_rx_irq_create_workqueue(struct
>
> if (hpd_rx_offload_wq[i].wq == NULL) {
> DRM_ERROR("create amdgpu_dm_hpd_rx_offload_wq fail!");
> -   return NULL;
> +   goto out_err;
> }
>
> spin_lock_init(_rx_offload_wq[i].offload_lock);
> }
>
> return hpd_rx_offload_wq;
> +
> +out_err:
> +   for (i = 0; i < max_caps; i++) {
> +   if (hpd_rx_offload_wq[i].wq)
> +   destroy_workqueue(hpd_rx_offload_wq[i].wq);
> +   }
> +   kfree(hpd_rx_offload_wq);
> +   return NULL;
>  }
>
>  struct amdgpu_stutter_quirk {
> --
> 2.34.1
>

[PATCH] drm/amdgpu: Fix memory leak in hpd_rx_irq_create_workqueue()

[Rafael Mendonca]

If construction of the array of work queues to handle hpd_rx_irq offload
work fails, we need to unwind. Destroy all the created workqueues and
the allocated memory for the hpd_rx_irq_offload_work_queue struct array.

Fixes: 8e794421bc98 ("drm/amd/display: Fork thread to offload work of 
hpd_rx_irq")
Signed-off-by: Rafael Mendonca 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 5140d9c2bf3b..6a2e455c5466 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1295,13 +1295,21 @@ static struct hpd_rx_irq_offload_work_queue 
*hpd_rx_irq_create_workqueue(struct
 
if (hpd_rx_offload_wq[i].wq == NULL) {
DRM_ERROR("create amdgpu_dm_hpd_rx_offload_wq fail!");
-   return NULL;
+   goto out_err;
}
 
spin_lock_init(_rx_offload_wq[i].offload_lock);
}
 
return hpd_rx_offload_wq;
+
+out_err:
+   for (i = 0; i < max_caps; i++) {
+   if (hpd_rx_offload_wq[i].wq)
+   destroy_workqueue(hpd_rx_offload_wq[i].wq);
+   }
+   kfree(hpd_rx_offload_wq);
+   return NULL;
 }
 
 struct amdgpu_stutter_quirk {
-- 
2.34.1

Re: [PATCH] drm/amdgpu: Fix memory leak

[Alex Deucher]

On Wed, Mar 17, 2021 at 5:56 AM xinhui pan  wrote:
>
> drm_gem_object_put() should be paired with drm_gem_object_lookup().
>
> All gem objs are saved in fb->base.obj[]. Need put the old first before
> assign a new obj.
>
> Trigger VRAM leak by running command below
> $ service gdm restart
>
> Signed-off-by: xinhui pan 

Acked-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> index bebed0f307a1..46dafea8da8b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> @@ -955,8 +955,9 @@ int amdgpu_display_framebuffer_init(struct drm_device 
> *dev,
> }
>
> for (i = 1; i < rfb->base.format->num_planes; ++i) {
> +   drm_gem_object_get(rfb->base.obj[0]);
> +   drm_gem_object_put(rfb->base.obj[i]);
> rfb->base.obj[i] = rfb->base.obj[0];
> -   drm_gem_object_get(rfb->base.obj[i]);
> }
>
> return 0;
> @@ -1002,6 +1003,7 @@ amdgpu_display_user_framebuffer_create(struct 
> drm_device *dev,
> return ERR_PTR(ret);
> }
>
> +   drm_gem_object_put(obj);
> return _fb->base;
>  }
>
> --
> 2.25.1
>
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amdgpu: Fix memory leak

[xinhui pan]

drm_gem_object_put() should be paired with drm_gem_object_lookup().

All gem objs are saved in fb->base.obj[]. Need put the old first before
assign a new obj.

Trigger VRAM leak by running command below
$ service gdm restart

Signed-off-by: xinhui pan 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index bebed0f307a1..46dafea8da8b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -955,8 +955,9 @@ int amdgpu_display_framebuffer_init(struct drm_device *dev,
}
 
for (i = 1; i < rfb->base.format->num_planes; ++i) {
+   drm_gem_object_get(rfb->base.obj[0]);
+   drm_gem_object_put(rfb->base.obj[i]);
rfb->base.obj[i] = rfb->base.obj[0];
-   drm_gem_object_get(rfb->base.obj[i]);
}
 
return 0;
@@ -1002,6 +1003,7 @@ amdgpu_display_user_framebuffer_create(struct drm_device 
*dev,
return ERR_PTR(ret);
}
 
+   drm_gem_object_put(obj);
return _fb->base;
 }
 
-- 
2.25.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amdgpu: Fix memory leak in amdgpu_fence_emit

[Navid Emamdoost]

In the impelementation of amdgpu_fence_emit() if dma_fence_wait() fails
and returns an errno, before returning the allocated memory for fence
should be released.

Fixes: 3d2aca8c8620 ("drm/amdgpu: fix old fence check in amdgpu_fence_emit")
Signed-off-by: Navid Emamdoost 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
index 23085b352cf2..2f59c9270a7e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
@@ -166,8 +166,10 @@ int amdgpu_fence_emit(struct amdgpu_ring *ring, struct 
dma_fence **f,
if (old) {
r = dma_fence_wait(old, false);
dma_fence_put(old);
-   if (r)
+   if (r) {
+   dma_fence_put(fence);
return r;
+   }
}
}
 
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: Fix memory leak in amdgpu_fence_emit

[Koenig, Christian]

Am 21.10.19 um 20:09 schrieb Navid Emamdoost:
> In the impelementation of amdgpu_fence_emit() if dma_fence_wait() fails
> and returns an errno, before returning the allocated memory for fence
> should be released.
>
> Fixes: 3d2aca8c8620 ("drm/amdgpu: fix old fence check in amdgpu_fence_emit")
> Signed-off-by: Navid Emamdoost 

Reviewed-by: Christian König 

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> index 23085b352cf2..2f59c9270a7e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> @@ -166,8 +166,10 @@ int amdgpu_fence_emit(struct amdgpu_ring *ring, struct 
> dma_fence **f,
>   if (old) {
>   r = dma_fence_wait(old, false);
>   dma_fence_put(old);
> - if (r)
> + if (r) {
> + dma_fence_put(fence);
>   return r;
> + }
>   }
>   }
>   

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: fix memory leak

[Koenig, Christian]

Am 04.10.19 um 15:51 schrieb Nirmoy Das:
> cleanup error handling code and make sure temporary info array
> with the handles are freed by amdgpu_bo_list_put() on
> idr_replace()'s failure.
>
> Signed-off-by: Nirmoy Das 

Reviewed-by: Christian König 

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 14 +++---
>   1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
> index 7bcf86c61999..61e38e43ad1d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
> @@ -270,7 +270,7 @@ int amdgpu_bo_list_ioctl(struct drm_device *dev, void 
> *data,
>   
>   r = amdgpu_bo_create_list_entry_array(>in, );
>   if (r)
> - goto error_free;
> + return r;
>   
>   switch (args->in.operation) {
>   case AMDGPU_BO_LIST_OP_CREATE:
> @@ -283,8 +283,7 @@ int amdgpu_bo_list_ioctl(struct drm_device *dev, void 
> *data,
>   r = idr_alloc(>bo_list_handles, list, 1, 0, GFP_KERNEL);
>   mutex_unlock(>bo_list_lock);
>   if (r < 0) {
> - amdgpu_bo_list_put(list);
> - return r;
> + goto error_put_list;
>   }
>   
>   handle = r;
> @@ -306,9 +305,8 @@ int amdgpu_bo_list_ioctl(struct drm_device *dev, void 
> *data,
>   mutex_unlock(>bo_list_lock);
>   
>   if (IS_ERR(old)) {
> - amdgpu_bo_list_put(list);
>   r = PTR_ERR(old);
> - goto error_free;
> + goto error_put_list;
>   }
>   
>   amdgpu_bo_list_put(old);
> @@ -325,8 +323,10 @@ int amdgpu_bo_list_ioctl(struct drm_device *dev, void 
> *data,
>   
>   return 0;
>   
> +error_put_list:
> + amdgpu_bo_list_put(list);
> +
>   error_free:
> - if (info)
> - kvfree(info);
> + kvfree(info);
>   return r;
>   }

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amdgpu: fix memory leak

[Nirmoy Das]

cleanup error handling code and make sure temporary info array
with the handles are freed by amdgpu_bo_list_put() on
idr_replace()'s failure.

Signed-off-by: Nirmoy Das 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
index 7bcf86c61999..61e38e43ad1d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
@@ -270,7 +270,7 @@ int amdgpu_bo_list_ioctl(struct drm_device *dev, void *data,
 
r = amdgpu_bo_create_list_entry_array(>in, );
if (r)
-   goto error_free;
+   return r;
 
switch (args->in.operation) {
case AMDGPU_BO_LIST_OP_CREATE:
@@ -283,8 +283,7 @@ int amdgpu_bo_list_ioctl(struct drm_device *dev, void *data,
r = idr_alloc(>bo_list_handles, list, 1, 0, GFP_KERNEL);
mutex_unlock(>bo_list_lock);
if (r < 0) {
-   amdgpu_bo_list_put(list);
-   return r;
+   goto error_put_list;
}
 
handle = r;
@@ -306,9 +305,8 @@ int amdgpu_bo_list_ioctl(struct drm_device *dev, void *data,
mutex_unlock(>bo_list_lock);
 
if (IS_ERR(old)) {
-   amdgpu_bo_list_put(list);
r = PTR_ERR(old);
-   goto error_free;
+   goto error_put_list;
}
 
amdgpu_bo_list_put(old);
@@ -325,8 +323,10 @@ int amdgpu_bo_list_ioctl(struct drm_device *dev, void 
*data,
 
return 0;
 
+error_put_list:
+   amdgpu_bo_list_put(list);
+
 error_free:
-   if (info)
-   kvfree(info);
+   kvfree(info);
return r;
 }
-- 
2.23.0

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: fix memory leak in wait_all_fence

[zhoucm1]

On 2017年04月07日 16:55, Christian König wrote:

Am 07.04.2017 um 10:46 schrieb zhoucm1:



On 2017年04月07日 16:36, Christian König wrote:

Am 07.04.2017 um 05:36 schrieb Chunming Zhou:

Change-Id: Ib3e271e00e49f10152c1b3eace981a6bf78820de
Signed-off-by: Chunming Zhou 


NAK, that will allocate an array for the fence again, which we 
wanted to avoid.
I don't got your means, the **array just to store fence temporary, 
freed at the end.


Yeah, but that is unnecessary. We created this function to just avoid 
that.






We should just drop the fence reference directly after waiting for it.

How to handle err case in the middle.


just put a fence_put() directly after fence_wait_timeout(). We don't 
need to keep a reference to the fence till the end.

I see your mean, will send V2 soon.

Regards,
David Zhou


Regards,
Christian.



Regards,
David Zhou


Regards,
Christian.


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 25 
+++--

  1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c

index de1c4c3..d842452 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -1216,22 +1216,28 @@ static int amdgpu_cs_wait_all_fences(struct 
amdgpu_device *adev,

   struct drm_amdgpu_fence *fences)
  {
  uint32_t fence_count = wait->in.fence_count;
+struct fence **array;
  unsigned int i;
  long r = 1;
  +array = kcalloc(fence_count, sizeof(struct fence *), 
GFP_KERNEL);

+
+if (array == NULL)
+return -ENOMEM;
  for (i = 0; i < fence_count; i++) {
  struct fence *fence;
  unsigned long timeout = 
amdgpu_gem_timeout(wait->in.timeout_ns);

fence = amdgpu_cs_get_fence(adev, filp, [i]);
-if (IS_ERR(fence))
-return PTR_ERR(fence);
-else if (!fence)
+if (IS_ERR(fence)) {
+r = PTR_ERR(fence);
+goto err;
+} else if (!fence)
  continue;
-
+array[i] = fence;
  r = kcl_fence_wait_timeout(fence, true, timeout);
  if (r < 0)
-return r;
+goto err;
if (r == 0)
  break;
@@ -1240,7 +1246,14 @@ static int amdgpu_cs_wait_all_fences(struct 
amdgpu_device *adev,

  memset(wait, 0, sizeof(*wait));
  wait->out.status = (r > 0);
  -return 0;
+r = 0;
+
+err:
+for (i = 0; i < fence_count; i++)
+fence_put(array[i]);
+kfree(array);
+
+return r;
  }
/**





___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx





___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: fix memory leak in wait_all_fence

[Christian König]

Am 07.04.2017 um 10:46 schrieb zhoucm1:



On 2017年04月07日 16:36, Christian König wrote:

Am 07.04.2017 um 05:36 schrieb Chunming Zhou:

Change-Id: Ib3e271e00e49f10152c1b3eace981a6bf78820de
Signed-off-by: Chunming Zhou 


NAK, that will allocate an array for the fence again, which we wanted 
to avoid.
I don't got your means, the **array just to store fence temporary, 
freed at the end.


Yeah, but that is unnecessary. We created this function to just avoid that.





We should just drop the fence reference directly after waiting for it.

How to handle err case in the middle.


just put a fence_put() directly after fence_wait_timeout(). We don't 
need to keep a reference to the fence till the end.


Regards,
Christian.



Regards,
David Zhou


Regards,
Christian.


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 25 +++--
  1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c

index de1c4c3..d842452 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -1216,22 +1216,28 @@ static int amdgpu_cs_wait_all_fences(struct 
amdgpu_device *adev,

   struct drm_amdgpu_fence *fences)
  {
  uint32_t fence_count = wait->in.fence_count;
+struct fence **array;
  unsigned int i;
  long r = 1;
  +array = kcalloc(fence_count, sizeof(struct fence *), 
GFP_KERNEL);

+
+if (array == NULL)
+return -ENOMEM;
  for (i = 0; i < fence_count; i++) {
  struct fence *fence;
  unsigned long timeout = 
amdgpu_gem_timeout(wait->in.timeout_ns);

fence = amdgpu_cs_get_fence(adev, filp, [i]);
-if (IS_ERR(fence))
-return PTR_ERR(fence);
-else if (!fence)
+if (IS_ERR(fence)) {
+r = PTR_ERR(fence);
+goto err;
+} else if (!fence)
  continue;
-
+array[i] = fence;
  r = kcl_fence_wait_timeout(fence, true, timeout);
  if (r < 0)
-return r;
+goto err;
if (r == 0)
  break;
@@ -1240,7 +1246,14 @@ static int amdgpu_cs_wait_all_fences(struct 
amdgpu_device *adev,

  memset(wait, 0, sizeof(*wait));
  wait->out.status = (r > 0);
  -return 0;
+r = 0;
+
+err:
+for (i = 0; i < fence_count; i++)
+fence_put(array[i]);
+kfree(array);
+
+return r;
  }
/**





___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: fix memory leak in wait_all_fence

[zhoucm1]

On 2017年04月07日 16:36, Christian König wrote:

Am 07.04.2017 um 05:36 schrieb Chunming Zhou:

Change-Id: Ib3e271e00e49f10152c1b3eace981a6bf78820de
Signed-off-by: Chunming Zhou 


NAK, that will allocate an array for the fence again, which we wanted 
to avoid.
I don't got your means, the **array just to store fence temporary, freed 
at the end.




We should just drop the fence reference directly after waiting for it.

How to handle err case in the middle.

Regards,
David Zhou


Regards,
Christian.


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 25 +++--
  1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c

index de1c4c3..d842452 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -1216,22 +1216,28 @@ static int amdgpu_cs_wait_all_fences(struct 
amdgpu_device *adev,

   struct drm_amdgpu_fence *fences)
  {
  uint32_t fence_count = wait->in.fence_count;
+struct fence **array;
  unsigned int i;
  long r = 1;
  +array = kcalloc(fence_count, sizeof(struct fence *), GFP_KERNEL);
+
+if (array == NULL)
+return -ENOMEM;
  for (i = 0; i < fence_count; i++) {
  struct fence *fence;
  unsigned long timeout = 
amdgpu_gem_timeout(wait->in.timeout_ns);

fence = amdgpu_cs_get_fence(adev, filp, [i]);
-if (IS_ERR(fence))
-return PTR_ERR(fence);
-else if (!fence)
+if (IS_ERR(fence)) {
+r = PTR_ERR(fence);
+goto err;
+} else if (!fence)
  continue;
-
+array[i] = fence;
  r = kcl_fence_wait_timeout(fence, true, timeout);
  if (r < 0)
-return r;
+goto err;
if (r == 0)
  break;
@@ -1240,7 +1246,14 @@ static int amdgpu_cs_wait_all_fences(struct 
amdgpu_device *adev,

  memset(wait, 0, sizeof(*wait));
  wait->out.status = (r > 0);
  -return 0;
+r = 0;
+
+err:
+for (i = 0; i < fence_count; i++)
+fence_put(array[i]);
+kfree(array);
+
+return r;
  }
/**





___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

答复: [PATCH] drm/amdgpu: fix memory leak in wait_all_fence

[Wang, Ken]

Reviewed-by: Ken Wang <qingqing.w...@amd.com>


发件人: amd-gfx <amd-gfx-boun...@lists.freedesktop.org> 代表 Chunming Zhou 
<david1.z...@amd.com>
发送时间: 2017年4月7日 11:36:18
收件人: amd-gfx@lists.freedesktop.org; Wang, Ken
抄送: Zhou, David(ChunMing)
主题: [PATCH] drm/amdgpu: fix memory leak in wait_all_fence

Change-Id: Ib3e271e00e49f10152c1b3eace981a6bf78820de
Signed-off-by: Chunming Zhou <david1.z...@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 25 +++--
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index de1c4c3..d842452 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -1216,22 +1216,28 @@ static int amdgpu_cs_wait_all_fences(struct 
amdgpu_device *adev,
  struct drm_amdgpu_fence *fences)
 {
 uint32_t fence_count = wait->in.fence_count;
+   struct fence **array;
 unsigned int i;
 long r = 1;

+   array = kcalloc(fence_count, sizeof(struct fence *), GFP_KERNEL);
+
+   if (array == NULL)
+   return -ENOMEM;
 for (i = 0; i < fence_count; i++) {
 struct fence *fence;
 unsigned long timeout = 
amdgpu_gem_timeout(wait->in.timeout_ns);

 fence = amdgpu_cs_get_fence(adev, filp, [i]);
-   if (IS_ERR(fence))
-   return PTR_ERR(fence);
-   else if (!fence)
+   if (IS_ERR(fence)) {
+   r = PTR_ERR(fence);
+   goto err;
+   } else if (!fence)
 continue;
-
+   array[i] = fence;
 r = kcl_fence_wait_timeout(fence, true, timeout);
 if (r < 0)
-   return r;
+   goto err;

 if (r == 0)
 break;
@@ -1240,7 +1246,14 @@ static int amdgpu_cs_wait_all_fences(struct 
amdgpu_device *adev,
 memset(wait, 0, sizeof(*wait));
 wait->out.status = (r > 0);

-   return 0;
+   r = 0;
+
+err:
+   for (i = 0; i < fence_count; i++)
+   fence_put(array[i]);
+   kfree(array);
+
+   return r;
 }

 /**
--
1.9.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amdgpu: fix memory leak in wait_all_fence

[Chunming Zhou]

Change-Id: Ib3e271e00e49f10152c1b3eace981a6bf78820de
Signed-off-by: Chunming Zhou 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 25 +++--
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index de1c4c3..d842452 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -1216,22 +1216,28 @@ static int amdgpu_cs_wait_all_fences(struct 
amdgpu_device *adev,
 struct drm_amdgpu_fence *fences)
 {
uint32_t fence_count = wait->in.fence_count;
+   struct fence **array;
unsigned int i;
long r = 1;
 
+   array = kcalloc(fence_count, sizeof(struct fence *), GFP_KERNEL);
+
+   if (array == NULL)
+   return -ENOMEM;
for (i = 0; i < fence_count; i++) {
struct fence *fence;
unsigned long timeout = amdgpu_gem_timeout(wait->in.timeout_ns);
 
fence = amdgpu_cs_get_fence(adev, filp, [i]);
-   if (IS_ERR(fence))
-   return PTR_ERR(fence);
-   else if (!fence)
+   if (IS_ERR(fence)) {
+   r = PTR_ERR(fence);
+   goto err;
+   } else if (!fence)
continue;
-
+   array[i] = fence;
r = kcl_fence_wait_timeout(fence, true, timeout);
if (r < 0)
-   return r;
+   goto err;
 
if (r == 0)
break;
@@ -1240,7 +1246,14 @@ static int amdgpu_cs_wait_all_fences(struct 
amdgpu_device *adev,
memset(wait, 0, sizeof(*wait));
wait->out.status = (r > 0);
 
-   return 0;
+   r = 0;
+
+err:
+   for (i = 0; i < fence_count; i++)
+   fence_put(array[i]);
+   kfree(array);
+
+   return r;
 }
 
 /**
-- 
1.9.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: fix memory leak in psp

[Huang Rui]

On Tue, Mar 28, 2017 at 05:16:09PM +0800, Ken Wang wrote:
> Change-Id: I6bea247f41ea405d11c1d75ca97b789f6970b191
> Signed-off-by: Ken Wang 

Reviewed-by: Huang Rui 

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 12 ++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> index 89d1d2f..4731015f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> @@ -132,6 +132,10 @@ psp_cmd_submit_buf(struct psp_context *psp,
>   msleep(1);
>   };
>  
> + amdgpu_bo_free_kernel(_buf_bo,
> +   _buf_mc_addr,
> +   (void **)_buf_mem);
> +
>   return ret;
>  }
>  
> @@ -174,6 +178,8 @@ static int psp_tmr_init(struct psp_context *psp)
>   if (ret)
>   goto failed_mem;
>  
> + kfree(cmd);
> +
>   return 0;
>  
>  failed_mem:
> @@ -241,6 +247,7 @@ static int psp_asd_load(struct psp_context *psp)
>  
>   amdgpu_bo_free_kernel(_bo, _mc_addr, _buf);
>   amdgpu_bo_free_kernel(_shared_bo, _shared_mc_addr, 
> _shared_buf);
> + kfree(cmd);
>  
>   return 0;
>  
> @@ -289,11 +296,11 @@ static int psp_load_fw(struct amdgpu_device *adev)
>  
>   ret = psp_tmr_init(psp);
>   if (ret)
> - goto failed;
> + goto failed_mem;
>  
>   ret = psp_asd_load(psp);
>   if (ret)
> - goto failed;
> + goto failed_mem;
>  
>   for (i = 0; i < adev->firmware.max_ucodes; i++) {
>   ucode = >firmware.ucode[i];
> @@ -322,6 +329,7 @@ static int psp_load_fw(struct amdgpu_device *adev)
>  
>   amdgpu_bo_free_kernel(>fence_buf_bo,
> >fence_buf_mc_addr, >fence_buf);
> + kfree(cmd);
>  
>   return 0;
>  
> -- 
> 2.7.4
> 
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: fix memory leak in psp

[Christian König]

Am 28.03.2017 um 11:16 schrieb Ken Wang:

Change-Id: I6bea247f41ea405d11c1d75ca97b789f6970b191
Signed-off-by: Ken Wang 


I don't know the PSP code to well, but that looks correct to me on first 
glance.


Patch is Acked-by: Christian König .

Regards,
Christian.


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 12 ++--
  1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 89d1d2f..4731015f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -132,6 +132,10 @@ psp_cmd_submit_buf(struct psp_context *psp,
msleep(1);
};
  
+	amdgpu_bo_free_kernel(_buf_bo,

+ _buf_mc_addr,
+ (void **)_buf_mem);
+
return ret;
  }
  
@@ -174,6 +178,8 @@ static int psp_tmr_init(struct psp_context *psp)

if (ret)
goto failed_mem;
  
+	kfree(cmd);

+
return 0;
  
  failed_mem:

@@ -241,6 +247,7 @@ static int psp_asd_load(struct psp_context *psp)
  
  	amdgpu_bo_free_kernel(_bo, _mc_addr, _buf);

amdgpu_bo_free_kernel(_shared_bo, _shared_mc_addr, 
_shared_buf);
+   kfree(cmd);
  
  	return 0;
  
@@ -289,11 +296,11 @@ static int psp_load_fw(struct amdgpu_device *adev)
  
  	ret = psp_tmr_init(psp);

if (ret)
-   goto failed;
+   goto failed_mem;
  
  	ret = psp_asd_load(psp);

if (ret)
-   goto failed;
+   goto failed_mem;
  
  	for (i = 0; i < adev->firmware.max_ucodes; i++) {

ucode = >firmware.ucode[i];
@@ -322,6 +329,7 @@ static int psp_load_fw(struct amdgpu_device *adev)
  
  	amdgpu_bo_free_kernel(>fence_buf_bo,

  >fence_buf_mc_addr, >fence_buf);
+   kfree(cmd);
  
  	return 0;
  



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amdgpu: fix memory leak in psp

[Ken Wang]

Change-Id: I6bea247f41ea405d11c1d75ca97b789f6970b191
Signed-off-by: Ken Wang 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 89d1d2f..4731015f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -132,6 +132,10 @@ psp_cmd_submit_buf(struct psp_context *psp,
msleep(1);
};
 
+   amdgpu_bo_free_kernel(_buf_bo,
+ _buf_mc_addr,
+ (void **)_buf_mem);
+
return ret;
 }
 
@@ -174,6 +178,8 @@ static int psp_tmr_init(struct psp_context *psp)
if (ret)
goto failed_mem;
 
+   kfree(cmd);
+
return 0;
 
 failed_mem:
@@ -241,6 +247,7 @@ static int psp_asd_load(struct psp_context *psp)
 
amdgpu_bo_free_kernel(_bo, _mc_addr, _buf);
amdgpu_bo_free_kernel(_shared_bo, _shared_mc_addr, 
_shared_buf);
+   kfree(cmd);
 
return 0;
 
@@ -289,11 +296,11 @@ static int psp_load_fw(struct amdgpu_device *adev)
 
ret = psp_tmr_init(psp);
if (ret)
-   goto failed;
+   goto failed_mem;
 
ret = psp_asd_load(psp);
if (ret)
-   goto failed;
+   goto failed_mem;
 
for (i = 0; i < adev->firmware.max_ucodes; i++) {
ucode = >firmware.ucode[i];
@@ -322,6 +329,7 @@ static int psp_load_fw(struct amdgpu_device *adev)
 
amdgpu_bo_free_kernel(>fence_buf_bo,
  >fence_buf_mc_addr, >fence_buf);
+   kfree(cmd);
 
return 0;
 
-- 
2.7.4

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx