On 7/8/24 6:06 PM, Rajneesh Bhardwaj 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 compaction. (/proc/sys/vm/compact_memory) > > Note: On certain distros such as RHEL, the proactive compaction is > disabled. (https://tinyurl.com/4f32f7rs) > > Cc: Dave Airlie <airlied@xxxxxxxxxx> > Cc: Vlastimil Babka <vbabka@xxxxxxx> > Cc: Daniel Vetter <daniel.vetter@xxxxxxxx> > Reviewed-by: Christian König <christian.koenig@xxxxxxx> > Signed-off-by: Rajneesh Bhardwaj <rajneesh.bhardwaj@xxxxxxx> > --- > 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; __GFP_RECLAIM includes ___GFP_DIRECT_RECLAIM, what if the caller is a context that can't sleep? Then it would be a bugy to add that. OTOH the only caller I see is ttm_pool_alloc() which seems to start with GFP_USER and that already includes __GFP_RECLAIM, so either way I see no reason to be adding it here, other than it might be a potential bug in case other callers are added later and have more restricted context. As for __GFP_THISNODE addition that should be fine as this seems to be an opportunistic allocation with a fallback that's decreasing the attempted order. Also I note that the caller might be adding __GFP_RETRY_MAYFAIL if (ctx->gfp_retry_mayfail) gfp_flags |= __GFP_RETRY_MAYFAIL; But here adding __GFP_NORETRY makes __GFP_RETRY_MAYFAIL non-effective as __alloc_pages_slowpath() evaluates __GFP_NORETRY earlier to decide to give up, than evaluating __GFP_RETRY_MAYFAIL to decide to try a bit harder. That's not introduced by this patch but maybe something to look into, if __GFP_RETRY_MAYFAIL is expected to be useful here for trying harder. If it's instead intended only for the final fallback order-0 case where the gfp_flags adjustment above doesn't apply, then __GFP_RETRY_MAYFAIL will cause the allocation to fail instead of applying the infamous implicit "too small to fail" for order-0 allocation and invoking OOM. If that's the reason for it to be used here, all is fine and great. Vlastimil > > if (!pool->use_dma_alloc) { > p = alloc_pages_node(pool->nid, gfp_flags, order);
