[PATCH] drm/ttm: Add cgroup memory accounting for GTT memory

[PATCH] drm/ttm: Add cgroup memory accounting for GTT memory

[Mukul Joshi]

Make sure we do not overflow the memory limits set for a cgroup when doing
GTT memory allocations.

Suggested-by: Philip Yang <philip.yang@amd.com>
Signed-off-by: Mukul Joshi <mukul.joshi@amd.com>
---
 drivers/gpu/drm/ttm/ttm_pool.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c
index 6e1fd6985ffc..59e1accdef08 100644
--- a/drivers/gpu/drm/ttm/ttm_pool.c
+++ b/drivers/gpu/drm/ttm/ttm_pool.c
@@ -91,7 +91,7 @@ static struct page *ttm_pool_alloc_page(struct ttm_pool *pool, gfp_t gfp_flags,
 	 */
 	if (order)
 		gfp_flags |= __GFP_NOMEMALLOC | __GFP_NORETRY | __GFP_NOWARN |
-			__GFP_KSWAPD_RECLAIM;
+			__GFP_KSWAPD_RECLAIM | __GFP_ACCOUNT;
 
 	if (!pool->use_dma_alloc) {
 		p = alloc_pages_node(pool->nid, gfp_flags, order);
-- 
2.35.1

Re: [PATCH] drm/ttm: Add cgroup memory accounting for GTT memory

[Christian König]

Am 06.06.24 um 21:22 schrieb Mukul Joshi:
> Make sure we do not overflow the memory limits set for a cgroup when doing
> GTT memory allocations.

NAK, That's intentionally not done like that.

Please see the cgroup discussion on memory management on the public 
mailing list.

Regards,
Christian.

>
> Suggested-by: Philip Yang <philip.yang@amd.com>
> Signed-off-by: Mukul Joshi <mukul.joshi@amd.com>
> ---
>   drivers/gpu/drm/ttm/ttm_pool.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c
> index 6e1fd6985ffc..59e1accdef08 100644
> --- a/drivers/gpu/drm/ttm/ttm_pool.c
> +++ b/drivers/gpu/drm/ttm/ttm_pool.c
> @@ -91,7 +91,7 @@ static struct page *ttm_pool_alloc_page(struct ttm_pool *pool, gfp_t gfp_flags,
>   	 */
>   	if (order)
>   		gfp_flags |= __GFP_NOMEMALLOC | __GFP_NORETRY | __GFP_NOWARN |
> -			__GFP_KSWAPD_RECLAIM;
> +			__GFP_KSWAPD_RECLAIM | __GFP_ACCOUNT;
>   
>   	if (!pool->use_dma_alloc) {
>   		p = alloc_pages_node(pool->nid, gfp_flags, order);

RE: [PATCH] drm/ttm: Add cgroup memory accounting for GTT memory

[Joshi, Mukul]

[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: Koenig, Christian <Christian.Koenig@amd.com>
> Sent: Friday, June 7, 2024 3:26 AM
> To: Joshi, Mukul <Mukul.Joshi@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Kuehling, Felix <Felix.Kuehling@amd.com>; Bhardwaj, Rajneesh
> <Rajneesh.Bhardwaj@amd.com>; Yang, Philip <Philip.Yang@amd.com>
> Subject: Re: [PATCH] drm/ttm: Add cgroup memory accounting for GTT
> memory
>
> Am 06.06.24 um 21:22 schrieb Mukul Joshi:
> > Make sure we do not overflow the memory limits set for a cgroup when
> > doing GTT memory allocations.
>
> NAK, That's intentionally not done like that.
>
> Please see the cgroup discussion on memory management on the public
> mailing list.
>
Can you please point us to that discussion?

Thanks,
Mukul

> Regards,
> Christian.
>
> >
> > Suggested-by: Philip Yang <philip.yang@amd.com>
> > Signed-off-by: Mukul Joshi <mukul.joshi@amd.com>
> > ---
> >   drivers/gpu/drm/ttm/ttm_pool.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/ttm/ttm_pool.c
> > b/drivers/gpu/drm/ttm/ttm_pool.c index 6e1fd6985ffc..59e1accdef08
> > 100644
> > --- a/drivers/gpu/drm/ttm/ttm_pool.c
> > +++ b/drivers/gpu/drm/ttm/ttm_pool.c
> > @@ -91,7 +91,7 @@ static struct page *ttm_pool_alloc_page(struct
> ttm_pool *pool, gfp_t gfp_flags,
> >      */
> >     if (order)
> >             gfp_flags |= __GFP_NOMEMALLOC | __GFP_NORETRY |
> __GFP_NOWARN |
> > -                   __GFP_KSWAPD_RECLAIM;
> > +                   __GFP_KSWAPD_RECLAIM | __GFP_ACCOUNT;
> >
> >     if (!pool->use_dma_alloc) {
> >             p = alloc_pages_node(pool->nid, gfp_flags, order);

Re: [PATCH] drm/ttm: Add cgroup memory accounting for GTT memory

[Christian König]

Am 07.06.24 um 16:43 schrieb Joshi, Mukul:
> [AMD Official Use Only - AMD Internal Distribution Only]
>
>> -----Original Message-----
>> From: Koenig, Christian <Christian.Koenig@amd.com>
>> Sent: Friday, June 7, 2024 3:26 AM
>> To: Joshi, Mukul <Mukul.Joshi@amd.com>; amd-gfx@lists.freedesktop.org
>> Cc: Kuehling, Felix <Felix.Kuehling@amd.com>; Bhardwaj, Rajneesh
>> <Rajneesh.Bhardwaj@amd.com>; Yang, Philip <Philip.Yang@amd.com>
>> Subject: Re: [PATCH] drm/ttm: Add cgroup memory accounting for GTT
>> memory
>>
>> Am 06.06.24 um 21:22 schrieb Mukul Joshi:
>>> Make sure we do not overflow the memory limits set for a cgroup when
>>> doing GTT memory allocations.
>> NAK, That's intentionally not done like that.
>>
>> Please see the cgroup discussion on memory management on the public
>> mailing list.
>>
> Can you please point us to that discussion?

IIRC search the mailing list for mails prefixed with "drm/cgroup:".

At some point I stopped looking into this mail thread, but the idea of 
using __GFP_ACCOUNT was abandoned rather early in the thread.

For adding it in the TTM pool the whole idea falls apart when you 
consider that the process calling this is usually not the process 
actually using the memory.

For example when a process causes VRAM pressure it allocates memory to 
swap out the BOs from other processes to system memory.

Regards,
Christian.

>
> Thanks,
> Mukul
>
>> Regards,
>> Christian.
>>
>>> Suggested-by: Philip Yang <philip.yang@amd.com>
>>> Signed-off-by: Mukul Joshi <mukul.joshi@amd.com>
>>> ---
>>>    drivers/gpu/drm/ttm/ttm_pool.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/ttm/ttm_pool.c
>>> b/drivers/gpu/drm/ttm/ttm_pool.c index 6e1fd6985ffc..59e1accdef08
>>> 100644
>>> --- a/drivers/gpu/drm/ttm/ttm_pool.c
>>> +++ b/drivers/gpu/drm/ttm/ttm_pool.c
>>> @@ -91,7 +91,7 @@ static struct page *ttm_pool_alloc_page(struct
>> ttm_pool *pool, gfp_t gfp_flags,
>>>       */
>>>      if (order)
>>>              gfp_flags |= __GFP_NOMEMALLOC | __GFP_NORETRY |
>> __GFP_NOWARN |
>>> -                   __GFP_KSWAPD_RECLAIM;
>>> +                   __GFP_KSWAPD_RECLAIM | __GFP_ACCOUNT;
>>>
>>>      if (!pool->use_dma_alloc) {
>>>              p = alloc_pages_node(pool->nid, gfp_flags, order);