Re: [PATCH 1/2] drm/ttm: Allow direct reclaim to allocate local memory

Re: [PATCH 1/2] drm/ttm: Allow direct reclaim to allocate local memory

[Alex Deucher]

+ dri-devel

On Tue, Jul 2, 2024 at 1:40 PM Rajneesh Bhardwaj
<rajneesh.bhardwaj@amd.com> wrote:
>
> Limiting the allocation of higher order pages to the closest NUMA node
> and enabling direct memory reclaim provides not only failsafe against
> situations when memory becomes too much fragmented and the allocator is
> not able to satisfy the request from the local node but falls back to
> remote pages (HUGEPAGE) but also offers performance improvement.
> Accessing remote pages suffers due to bandwidth limitations and could be
> avoided if memory becomes defragmented and in most cases without using
> manual compation.
>
> Signed-off-by: Rajneesh Bhardwaj <rajneesh.bhardwaj@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..cc27d5c7afe8 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_RECLAIM | __GFP_THISNODE;
>
>         if (!pool->use_dma_alloc) {
>                 p = alloc_pages_node(pool->nid, gfp_flags, order);
> --
> 2.34.1
>

Re: [PATCH 2/2] drm/ttm: Make ttm shrinkers NUMA aware

[Alex Deucher]

+ dri-devel

On Tue, Jul 2, 2024 at 1:40 PM Rajneesh Bhardwaj
<rajneesh.bhardwaj@amd.com> wrote:
>
> Otherwise the nid is always passed as 0 during memory reclaim so
> make TTM shrinkers NUMA aware.
>
> Signed-off-by: Rajneesh Bhardwaj <rajneesh.bhardwaj@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 cc27d5c7afe8..f93ac9089a60 100644
> --- a/drivers/gpu/drm/ttm/ttm_pool.c
> +++ b/drivers/gpu/drm/ttm/ttm_pool.c
> @@ -812,7 +812,7 @@ int ttm_pool_mgr_init(unsigned long num_pages)
>                             &ttm_pool_debugfs_shrink_fops);
>  #endif
>
> -       mm_shrinker = shrinker_alloc(0, "drm-ttm_pool");
> +       mm_shrinker = shrinker_alloc(SHRINKER_NUMA_AWARE, "drm-ttm_pool");
>         if (!mm_shrinker)
>                 return -ENOMEM;
>
> --
> 2.34.1
>

Re: [PATCH 2/2] drm/ttm: Make ttm shrinkers NUMA aware

[Christian König]

Am 02.07.24 um 20:20 schrieb Alex Deucher:
> + dri-devel
>
> On Tue, Jul 2, 2024 at 1:40 PM Rajneesh Bhardwaj
> <rajneesh.bhardwaj@amd.com> wrote:
>> Otherwise the nid is always passed as 0 during memory reclaim so
>> make TTM shrinkers NUMA aware.
>>
>> Signed-off-by: Rajneesh Bhardwaj <rajneesh.bhardwaj@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 cc27d5c7afe8..f93ac9089a60 100644
>> --- a/drivers/gpu/drm/ttm/ttm_pool.c
>> +++ b/drivers/gpu/drm/ttm/ttm_pool.c
>> @@ -812,7 +812,7 @@ int ttm_pool_mgr_init(unsigned long num_pages)
>>                              &ttm_pool_debugfs_shrink_fops);
>>   #endif
>>
>> -       mm_shrinker = shrinker_alloc(0, "drm-ttm_pool");
>> +       mm_shrinker = shrinker_alloc(SHRINKER_NUMA_AWARE, "drm-ttm_pool");

You also need to make ttm_pool_shrink() actually use the nid.

Just setting the flag won't really help us.

Regards,
Christian.

>>          if (!mm_shrinker)
>>                  return -ENOMEM;
>>
>> --
>> 2.34.1
>>

RE: [PATCH 2/2] drm/ttm: Make ttm shrinkers NUMA aware

[Bhardwaj, Rajneesh]

[AMD Official Use Only - AMD Internal Distribution Only]

-----Original Message-----
From: Koenig, Christian <Christian.Koenig@amd.com>
Sent: Tuesday, July 2, 2024 2:25 PM
To: Alex Deucher <alexdeucher@gmail.com>; Bhardwaj, Rajneesh <Rajneesh.Bhardwaj@amd.com>; Maling list - DRI developers <dri-devel@lists.freedesktop.org>
Cc: amd-gfx@lists.freedesktop.org; Kuehling, Felix <Felix.Kuehling@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com>
Subject: Re: [PATCH 2/2] drm/ttm: Make ttm shrinkers NUMA aware



Am 02.07.24 um 20:20 schrieb Alex Deucher:
> + dri-devel
>
> On Tue, Jul 2, 2024 at 1:40 PM Rajneesh Bhardwaj
> <rajneesh.bhardwaj@amd.com> wrote:
>> Otherwise the nid is always passed as 0 during memory reclaim so make
>> TTM shrinkers NUMA aware.
>>
>> Signed-off-by: Rajneesh Bhardwaj <rajneesh.bhardwaj@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 cc27d5c7afe8..f93ac9089a60
>> 100644
>> --- a/drivers/gpu/drm/ttm/ttm_pool.c
>> +++ b/drivers/gpu/drm/ttm/ttm_pool.c
>> @@ -812,7 +812,7 @@ int ttm_pool_mgr_init(unsigned long num_pages)
>>                              &ttm_pool_debugfs_shrink_fops);
>>   #endif
>>
>> -       mm_shrinker = shrinker_alloc(0, "drm-ttm_pool");
>> +       mm_shrinker = shrinker_alloc(SHRINKER_NUMA_AWARE,
>> + "drm-ttm_pool");

You also need to make ttm_pool_shrink() actually use the nid.

Yeah, Did you mean setting the nid of the shrinker control structure from something like ttm_global_init() -passes NUMA node id dev_to_node(dev) to ttm_pool_mgr_init and use it to set the mm_shrinker->sc.nid ?

Just setting the flag won't really help us.

Regards,
Christian.

>>          if (!mm_shrinker)
>>                  return -ENOMEM;
>>
>> --
>> 2.34.1
>>

Re: [PATCH 2/2] drm/ttm: Make ttm shrinkers NUMA aware

[Christian König]

Am 02.07.24 um 23:54 schrieb Bhardwaj, Rajneesh:
> [AMD Official Use Only - AMD Internal Distribution Only]
>
> -----Original Message-----
> From: Koenig, Christian <Christian.Koenig@amd.com>
> Sent: Tuesday, July 2, 2024 2:25 PM
> To: Alex Deucher <alexdeucher@gmail.com>; Bhardwaj, Rajneesh <Rajneesh.Bhardwaj@amd.com>; Maling list - DRI developers <dri-devel@lists.freedesktop.org>
> Cc: amd-gfx@lists.freedesktop.org; Kuehling, Felix <Felix.Kuehling@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com>
> Subject: Re: [PATCH 2/2] drm/ttm: Make ttm shrinkers NUMA aware
>
>
>
> Am 02.07.24 um 20:20 schrieb Alex Deucher:
>> + dri-devel
>>
>> On Tue, Jul 2, 2024 at 1:40 PM Rajneesh Bhardwaj
>> <rajneesh.bhardwaj@amd.com> wrote:
>>> Otherwise the nid is always passed as 0 during memory reclaim so make
>>> TTM shrinkers NUMA aware.
>>>
>>> Signed-off-by: Rajneesh Bhardwaj <rajneesh.bhardwaj@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 cc27d5c7afe8..f93ac9089a60
>>> 100644
>>> --- a/drivers/gpu/drm/ttm/ttm_pool.c
>>> +++ b/drivers/gpu/drm/ttm/ttm_pool.c
>>> @@ -812,7 +812,7 @@ int ttm_pool_mgr_init(unsigned long num_pages)
>>>                               &ttm_pool_debugfs_shrink_fops);
>>>    #endif
>>>
>>> -       mm_shrinker = shrinker_alloc(0, "drm-ttm_pool");
>>> +       mm_shrinker = shrinker_alloc(SHRINKER_NUMA_AWARE,
>>> + "drm-ttm_pool");
> You also need to make ttm_pool_shrink() actually use the nid.
>
> Yeah, Did you mean setting the nid of the shrinker control structure from something like ttm_global_init() -passes NUMA node id dev_to_node(dev) to ttm_pool_mgr_init and use it to set the mm_shrinker->sc.nid ?

No, the nid needs to be passed in as parameter to ttm_pool_shrink(). See 
function ttm_pool_shrinker_scan()

Regards,
Christian.

>
> Just setting the flag won't really help us.
>
> Regards,
> Christian.
>
>>>           if (!mm_shrinker)
>>>                   return -ENOMEM;
>>>
>>> --
>>> 2.34.1
>>>