diff mbox series

[1/2] drm/ttm: use the same attributes when freeing d_page->vaddr

Message ID 20190711031021.23512-1-huangfq.daxian@gmail.com (mailing list archive)
State New, archived
Headers show
Series [1/2] drm/ttm: use the same attributes when freeing d_page->vaddr | expand

Commit Message

Fuqian Huang July 11, 2019, 3:10 a.m. UTC
In function __ttm_dma_alloc_page(), d_page->addr is allocated
by dma_alloc_attrs() but freed with use dma_free_coherent() in
__ttm_dma_free_page().
Use the correct dma_free_attrs() to free d_page->vaddr.

Signed-off-by: Fuqian Huang <huangfq.daxian@gmail.com>
---
 drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Christian König July 16, 2019, 1:38 p.m. UTC | #1
Am 11.07.19 um 05:10 schrieb Fuqian Huang:
> In function __ttm_dma_alloc_page(), d_page->addr is allocated
> by dma_alloc_attrs() but freed with use dma_free_coherent() in
> __ttm_dma_free_page().
> Use the correct dma_free_attrs() to free d_page->vaddr.
>
> Signed-off-by: Fuqian Huang <huangfq.daxian@gmail.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

How do you want to upstream that? Should I pull it into our tree?

Thanks,
Christian.

> ---
>   drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
> index d594f7520b7b..7d78e6deac89 100644
> --- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
> +++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
> @@ -285,9 +285,13 @@ static int ttm_set_pages_caching(struct dma_pool *pool,
>   
>   static void __ttm_dma_free_page(struct dma_pool *pool, struct dma_page *d_page)
>   {
> +	unsigned long attrs = 0;
>   	dma_addr_t dma = d_page->dma;
>   	d_page->vaddr &= ~VADDR_FLAG_HUGE_POOL;
> -	dma_free_coherent(pool->dev, pool->size, (void *)d_page->vaddr, dma);
> +	if (pool->type & IS_HUGE)
> +		attrs = DMA_ATTR_NO_WARN;
> +
> +	dma_free_attrs(pool->dev, pool->size, (void *)d_page->vaddr, dma, attrs);
>   
>   	kfree(d_page);
>   	d_page = NULL;
Fuqian Huang July 18, 2019, 7:21 a.m. UTC | #2
Koenig, Christian <Christian.Koenig@amd.com> 於 2019年7月16日週二 下午9:38寫道:
>
> Am 11.07.19 um 05:10 schrieb Fuqian Huang:
> > In function __ttm_dma_alloc_page(), d_page->addr is allocated
> > by dma_alloc_attrs() but freed with use dma_free_coherent() in
> > __ttm_dma_free_page().
> > Use the correct dma_free_attrs() to free d_page->vaddr.
> >
> > Signed-off-by: Fuqian Huang <huangfq.daxian@gmail.com>
>
> Reviewed-by: Christian König <christian.koenig@amd.com>
>
> How do you want to upstream that? Should I pull it into our tree?

I just came across this misuse case accidentally.
I am not very clear about 'How to upstream that'.
Are there more than one way to upstream the code and fix the problem?

From my side, it is ok that you pull it into your tree and fix it or
fix it in other way.
:) It will be fine if the problem is fixed.

Thanks.

>
> Thanks,
> Christian.
>
> > ---
> >   drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 6 +++++-
> >   1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
> > index d594f7520b7b..7d78e6deac89 100644
> > --- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
> > +++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
> > @@ -285,9 +285,13 @@ static int ttm_set_pages_caching(struct dma_pool *pool,
> >
> >   static void __ttm_dma_free_page(struct dma_pool *pool, struct dma_page *d_page)
> >   {
> > +     unsigned long attrs = 0;
> >       dma_addr_t dma = d_page->dma;
> >       d_page->vaddr &= ~VADDR_FLAG_HUGE_POOL;
> > -     dma_free_coherent(pool->dev, pool->size, (void *)d_page->vaddr, dma);
> > +     if (pool->type & IS_HUGE)
> > +             attrs = DMA_ATTR_NO_WARN;
> > +
> > +     dma_free_attrs(pool->dev, pool->size, (void *)d_page->vaddr, dma, attrs);
> >
> >       kfree(d_page);
> >       d_page = NULL;
>
Christian König July 18, 2019, 7:56 a.m. UTC | #3
Am 18.07.19 um 09:21 schrieb Fuqian Huang:
> Koenig, Christian <Christian.Koenig@amd.com> 於 2019年7月16日週二 下午9:38寫道:
>> Am 11.07.19 um 05:10 schrieb Fuqian Huang:
>>> In function __ttm_dma_alloc_page(), d_page->addr is allocated
>>> by dma_alloc_attrs() but freed with use dma_free_coherent() in
>>> __ttm_dma_free_page().
>>> Use the correct dma_free_attrs() to free d_page->vaddr.
>>>
>>> Signed-off-by: Fuqian Huang <huangfq.daxian@gmail.com>
>> Reviewed-by: Christian König <christian.koenig@amd.com>
>>
>> How do you want to upstream that? Should I pull it into our tree?
> I just came across this misuse case accidentally.
> I am not very clear about 'How to upstream that'.
> Are there more than one way to upstream the code and fix the problem?

Well I can add it to the TTM tree which send to David or it could be 
pulled through some other way towards Linus.

>  From my side, it is ok that you pull it into your tree and fix it or
> fix it in other way.
> :) It will be fine if the problem is fixed.

Ok, fine with me :)

Christian.

>
> Thanks.
>
>> Thanks,
>> Christian.
>>
>>> ---
>>>    drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 6 +++++-
>>>    1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
>>> index d594f7520b7b..7d78e6deac89 100644
>>> --- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
>>> +++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
>>> @@ -285,9 +285,13 @@ static int ttm_set_pages_caching(struct dma_pool *pool,
>>>
>>>    static void __ttm_dma_free_page(struct dma_pool *pool, struct dma_page *d_page)
>>>    {
>>> +     unsigned long attrs = 0;
>>>        dma_addr_t dma = d_page->dma;
>>>        d_page->vaddr &= ~VADDR_FLAG_HUGE_POOL;
>>> -     dma_free_coherent(pool->dev, pool->size, (void *)d_page->vaddr, dma);
>>> +     if (pool->type & IS_HUGE)
>>> +             attrs = DMA_ATTR_NO_WARN;
>>> +
>>> +     dma_free_attrs(pool->dev, pool->size, (void *)d_page->vaddr, dma, attrs);
>>>
>>>        kfree(d_page);
>>>        d_page = NULL;
diff mbox series

Patch

diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
index d594f7520b7b..7d78e6deac89 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
@@ -285,9 +285,13 @@  static int ttm_set_pages_caching(struct dma_pool *pool,
 
 static void __ttm_dma_free_page(struct dma_pool *pool, struct dma_page *d_page)
 {
+	unsigned long attrs = 0;
 	dma_addr_t dma = d_page->dma;
 	d_page->vaddr &= ~VADDR_FLAG_HUGE_POOL;
-	dma_free_coherent(pool->dev, pool->size, (void *)d_page->vaddr, dma);
+	if (pool->type & IS_HUGE)
+		attrs = DMA_ATTR_NO_WARN;
+
+	dma_free_attrs(pool->dev, pool->size, (void *)d_page->vaddr, dma, attrs);
 
 	kfree(d_page);
 	d_page = NULL;