* [PATCH] drm/radeon: fix bo creation retry path
@ 2012-07-12 22:23 j.glisse
2012-07-13 5:54 ` Michel Dänzer
2012-07-13 7:43 ` Christian König
0 siblings, 2 replies; 5+ messages in thread
From: j.glisse @ 2012-07-12 22:23 UTC (permalink / raw)
To: dri-devel; +Cc: Jerome Glisse, stable
From: Jerome Glisse <jglisse@redhat.com>
Retry label was at wrong place in function leading to memory
leak.
Cc: <stable@vger.kernel.org>
Signed-off-by: Jerome Glisse <jglisse@redhat.com>
---
drivers/gpu/drm/radeon/radeon_object.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
index 6ecb200..f71e472 100644
--- a/drivers/gpu/drm/radeon/radeon_object.c
+++ b/drivers/gpu/drm/radeon/radeon_object.c
@@ -138,7 +138,6 @@ int radeon_bo_create(struct radeon_device *rdev,
acc_size = ttm_bo_dma_acc_size(&rdev->mman.bdev, size,
sizeof(struct radeon_bo));
-retry:
bo = kzalloc(sizeof(struct radeon_bo), GFP_KERNEL);
if (bo == NULL)
return -ENOMEM;
@@ -152,6 +151,8 @@ retry:
bo->surface_reg = -1;
INIT_LIST_HEAD(&bo->list);
INIT_LIST_HEAD(&bo->va);
+
+retry:
radeon_ttm_placement_from_domain(bo, domain);
/* Kernel allocation are uninterruptible */
down_read(&rdev->pm.mclk_lock);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/radeon: fix bo creation retry path
2012-07-12 22:23 [PATCH] drm/radeon: fix bo creation retry path j.glisse
@ 2012-07-13 5:54 ` Michel Dänzer
2012-08-21 7:03 ` Michel Dänzer
2012-07-13 7:43 ` Christian König
1 sibling, 1 reply; 5+ messages in thread
From: Michel Dänzer @ 2012-07-13 5:54 UTC (permalink / raw)
To: j.glisse; +Cc: dri-devel
On Don, 2012-07-12 at 18:23 -0400, j.glisse@gmail.com wrote:
> From: Jerome Glisse <jglisse@redhat.com>
>
> Retry label was at wrong place in function leading to memory
> leak.
>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Jerome Glisse <jglisse@redhat.com>
> ---
> drivers/gpu/drm/radeon/radeon_object.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
> index 6ecb200..f71e472 100644
> --- a/drivers/gpu/drm/radeon/radeon_object.c
> +++ b/drivers/gpu/drm/radeon/radeon_object.c
> @@ -138,7 +138,6 @@ int radeon_bo_create(struct radeon_device *rdev,
> acc_size = ttm_bo_dma_acc_size(&rdev->mman.bdev, size,
> sizeof(struct radeon_bo));
>
> -retry:
> bo = kzalloc(sizeof(struct radeon_bo), GFP_KERNEL);
> if (bo == NULL)
> return -ENOMEM;
> @@ -152,6 +151,8 @@ retry:
> bo->surface_reg = -1;
> INIT_LIST_HEAD(&bo->list);
> INIT_LIST_HEAD(&bo->va);
> +
> +retry:
> radeon_ttm_placement_from_domain(bo, domain);
> /* Kernel allocation are uninterruptible */
> down_read(&rdev->pm.mclk_lock);
Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Debian, X and DRI developer
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/radeon: fix bo creation retry path
2012-07-12 22:23 [PATCH] drm/radeon: fix bo creation retry path j.glisse
2012-07-13 5:54 ` Michel Dänzer
@ 2012-07-13 7:43 ` Christian König
1 sibling, 0 replies; 5+ messages in thread
From: Christian König @ 2012-07-13 7:43 UTC (permalink / raw)
To: j.glisse; +Cc: Jerome Glisse, stable, dri-devel
On 13.07.2012 00:23, j.glisse@gmail.com wrote:
> From: Jerome Glisse <jglisse@redhat.com>
>
> Retry label was at wrong place in function leading to memory
> leak.
>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Jerome Glisse <jglisse@redhat.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
> ---
> drivers/gpu/drm/radeon/radeon_object.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
> index 6ecb200..f71e472 100644
> --- a/drivers/gpu/drm/radeon/radeon_object.c
> +++ b/drivers/gpu/drm/radeon/radeon_object.c
> @@ -138,7 +138,6 @@ int radeon_bo_create(struct radeon_device *rdev,
> acc_size = ttm_bo_dma_acc_size(&rdev->mman.bdev, size,
> sizeof(struct radeon_bo));
>
> -retry:
> bo = kzalloc(sizeof(struct radeon_bo), GFP_KERNEL);
> if (bo == NULL)
> return -ENOMEM;
> @@ -152,6 +151,8 @@ retry:
> bo->surface_reg = -1;
> INIT_LIST_HEAD(&bo->list);
> INIT_LIST_HEAD(&bo->va);
> +
> +retry:
> radeon_ttm_placement_from_domain(bo, domain);
> /* Kernel allocation are uninterruptible */
> down_read(&rdev->pm.mclk_lock);
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/radeon: fix bo creation retry path
2012-07-13 5:54 ` Michel Dänzer
@ 2012-08-21 7:03 ` Michel Dänzer
2012-08-21 14:15 ` Alex Deucher
0 siblings, 1 reply; 5+ messages in thread
From: Michel Dänzer @ 2012-08-21 7:03 UTC (permalink / raw)
To: j.glisse; +Cc: dri-devel
On Fre, 2012-07-13 at 07:54 +0200, Michel Dänzer wrote:
> On Don, 2012-07-12 at 18:23 -0400, j.glisse@gmail.com wrote:
> > From: Jerome Glisse <jglisse@redhat.com>
> >
> > Retry label was at wrong place in function leading to memory
> > leak.
> >
> > Cc: <stable@vger.kernel.org>
> > Signed-off-by: Jerome Glisse <jglisse@redhat.com>
> > ---
> > drivers/gpu/drm/radeon/radeon_object.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
> > index 6ecb200..f71e472 100644
> > --- a/drivers/gpu/drm/radeon/radeon_object.c
> > +++ b/drivers/gpu/drm/radeon/radeon_object.c
> > @@ -138,7 +138,6 @@ int radeon_bo_create(struct radeon_device *rdev,
> > acc_size = ttm_bo_dma_acc_size(&rdev->mman.bdev, size,
> > sizeof(struct radeon_bo));
> >
> > -retry:
> > bo = kzalloc(sizeof(struct radeon_bo), GFP_KERNEL);
> > if (bo == NULL)
> > return -ENOMEM;
> > @@ -152,6 +151,8 @@ retry:
> > bo->surface_reg = -1;
> > INIT_LIST_HEAD(&bo->list);
> > INIT_LIST_HEAD(&bo->va);
> > +
> > +retry:
> > radeon_ttm_placement_from_domain(bo, domain);
> > /* Kernel allocation are uninterruptible */
> > down_read(&rdev->pm.mclk_lock);
>
> Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
Actually, this is wrong: ttm_bo_init() destroys the BO on failure. So
this patch makes the retry path work with freed memory. I see on IRC
that this is causing panics with some piglit tests for several people,
please submit a patch to revert this.
Sorry I didn't remember this when reviewing the patch. :(
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Debian, X and DRI developer
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/radeon: fix bo creation retry path
2012-08-21 7:03 ` Michel Dänzer
@ 2012-08-21 14:15 ` Alex Deucher
0 siblings, 0 replies; 5+ messages in thread
From: Alex Deucher @ 2012-08-21 14:15 UTC (permalink / raw)
To: Michel Dänzer; +Cc: dri-devel
On Tue, Aug 21, 2012 at 3:03 AM, Michel Dänzer <michel@daenzer.net> wrote:
> On Fre, 2012-07-13 at 07:54 +0200, Michel Dänzer wrote:
>> On Don, 2012-07-12 at 18:23 -0400, j.glisse@gmail.com wrote:
>> > From: Jerome Glisse <jglisse@redhat.com>
>> >
>> > Retry label was at wrong place in function leading to memory
>> > leak.
>> >
>> > Cc: <stable@vger.kernel.org>
>> > Signed-off-by: Jerome Glisse <jglisse@redhat.com>
>> > ---
>> > drivers/gpu/drm/radeon/radeon_object.c | 3 ++-
>> > 1 file changed, 2 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
>> > index 6ecb200..f71e472 100644
>> > --- a/drivers/gpu/drm/radeon/radeon_object.c
>> > +++ b/drivers/gpu/drm/radeon/radeon_object.c
>> > @@ -138,7 +138,6 @@ int radeon_bo_create(struct radeon_device *rdev,
>> > acc_size = ttm_bo_dma_acc_size(&rdev->mman.bdev, size,
>> > sizeof(struct radeon_bo));
>> >
>> > -retry:
>> > bo = kzalloc(sizeof(struct radeon_bo), GFP_KERNEL);
>> > if (bo == NULL)
>> > return -ENOMEM;
>> > @@ -152,6 +151,8 @@ retry:
>> > bo->surface_reg = -1;
>> > INIT_LIST_HEAD(&bo->list);
>> > INIT_LIST_HEAD(&bo->va);
>> > +
>> > +retry:
>> > radeon_ttm_placement_from_domain(bo, domain);
>> > /* Kernel allocation are uninterruptible */
>> > down_read(&rdev->pm.mclk_lock);
>>
>> Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
>
> Actually, this is wrong: ttm_bo_init() destroys the BO on failure. So
> this patch makes the retry path work with freed memory. I see on IRC
> that this is causing panics with some piglit tests for several people,
> please submit a patch to revert this.
>
> Sorry I didn't remember this when reviewing the patch. :(
Reverted in my -fixes queue.
Alex
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-08-21 14:15 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-12 22:23 [PATCH] drm/radeon: fix bo creation retry path j.glisse
2012-07-13 5:54 ` Michel Dänzer
2012-08-21 7:03 ` Michel Dänzer
2012-08-21 14:15 ` Alex Deucher
2012-07-13 7:43 ` Christian König
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.