Hi, Christian, On Mon, 2026-11-02 at 11:50 +0100, Christian König wrote: > First of all please add Natalie and Arun on TTM patches, they are the > maintainers of that component now. > > On 11/2/26 11:18, Himal Prasad Ghimiray wrote: > > ttm_pool_alloc() always allocates on the pool's default node, > > although > > the internal TTM helpers (__ttm_pool_alloc(), > > ttm_pool_alloc_page(), > > ttm_pool_type_take()) already take a nid. Add ttm_pool_alloc_node() > > to > > let a driver pass that nid for a specific tt. > > > > ttm_pool_alloc() passes NUMA_NO_NODE, keeping current behaviour. An > > explicit node on a dma_alloc pool returns -EOPNOTSUPP, as > > dma_alloc_attrs() can't honor it. > > We already have an interface for NUMA node aware allocations in the > TTM pool. > > I don't mind changing/improving that when necessary, but that here is > only adding a second path and therfore a clear NO-GO from my side.
What we were looking at moving forward is to possibly support interleaving modes (and that would mean interleaving per folio). > > If you want to change the nid from ttm_pool_init() to > ttm_pool_alloc() then change all existing users. Point taken. Any specific reason ttm_pool_init() was initially chosen? Thanks, Thomas > > Regards, > Christian. > > > Signed-off-by: Himal Prasad Ghimiray > > <[email protected]> > > --- > > drivers/gpu/drm/ttm/ttm_pool.c | 59 +++++++++++++++++++++++++++++- > > ---- > > include/drm/ttm/ttm_pool.h | 2 ++ > > 2 files changed, 53 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/gpu/drm/ttm/ttm_pool.c > > b/drivers/gpu/drm/ttm/ttm_pool.c > > index 1bf37023fed6..54d2f6b5c229 100644 > > --- a/drivers/gpu/drm/ttm/ttm_pool.c > > +++ b/drivers/gpu/drm/ttm/ttm_pool.c > > @@ -158,9 +158,9 @@ static int ttm_pool_nid(struct ttm_pool *pool) > > return nid; > > } > > > > -/* Allocate pages of size 1 << order with the given gfp_flags */ > > +/* Allocate pages of size 1 << order with the given gfp_flags on > > node @nid */ > > static struct page *ttm_pool_alloc_page(struct ttm_pool *pool, > > gfp_t gfp_flags, > > - unsigned int order) > > + unsigned int order, int > > nid) > > { > > const unsigned int beneficial_order = > > ttm_pool_beneficial_order(pool); > > unsigned long attr = DMA_ATTR_FORCE_CONTIGUOUS; > > @@ -189,7 +189,7 @@ static struct page *ttm_pool_alloc_page(struct > > ttm_pool *pool, gfp_t gfp_flags, > > } > > > > if (!ttm_pool_uses_dma_alloc(pool)) { > > - p = alloc_pages_node(pool->nid, gfp_flags, order); > > + p = alloc_pages_node(nid, gfp_flags, order); > > if (p) { > > p->private = order; > > mod_lruvec_page_state(p, NR_GPU_ACTIVE, 1 > > << order); > > @@ -783,7 +783,8 @@ static unsigned int > > ttm_pool_alloc_find_order(unsigned int highest, > > static int __ttm_pool_alloc(struct ttm_pool *pool, struct ttm_tt > > *tt, > > const struct ttm_operation_ctx *ctx, > > struct ttm_pool_alloc_state *alloc, > > - struct ttm_pool_tt_restore *restore) > > + struct ttm_pool_tt_restore *restore, > > + int nid) > > { > > enum ttm_caching page_caching; > > gfp_t gfp_flags = GFP_USER; > > @@ -818,7 +819,7 @@ static int __ttm_pool_alloc(struct ttm_pool > > *pool, struct ttm_tt *tt, > > p = NULL; > > pt = ttm_pool_select_type(pool, page_caching, > > order); > > if (pt && allow_pools) > > - p = ttm_pool_type_take(pt, > > ttm_pool_nid(pool)); > > + p = ttm_pool_type_take(pt, nid); > > > > /* > > * If that fails or previously failed, allocate > > from system. > > @@ -829,7 +830,7 @@ static int __ttm_pool_alloc(struct ttm_pool > > *pool, struct ttm_tt *tt, > > if (!p) { > > page_caching = ttm_cached; > > allow_pools = false; > > - p = ttm_pool_alloc_page(pool, gfp_flags, > > order); > > + p = ttm_pool_alloc_page(pool, gfp_flags, > > order, nid); > > } > > /* If that fails, lower the order if possible and > > retry. */ > > if (!p) { > > @@ -900,10 +901,52 @@ int ttm_pool_alloc(struct ttm_pool *pool, > > struct ttm_tt *tt, > > > > ttm_pool_alloc_state_init(tt, &alloc); > > > > - return __ttm_pool_alloc(pool, tt, ctx, &alloc, NULL); > > + return __ttm_pool_alloc(pool, tt, ctx, &alloc, NULL, > > ttm_pool_nid(pool)); > > } > > EXPORT_SYMBOL(ttm_pool_alloc); > > > > +/** > > + * ttm_pool_alloc_node - Fill a ttm_tt object, placing pages on a > > given node > > + * > > + * @pool: ttm_pool to use > > + * @tt: ttm_tt object to fill > > + * @ctx: operation context > > + * @nid: NUMA node id to allocate the backing system pages from, > > or > > + * NUMA_NO_NODE to use the pool's default node (@ttm_pool.nid) > > + * > > + * Like ttm_pool_alloc(), but allows the caller to override the > > NUMA node the > > + * backing system pages are allocated from. This only affects the > > + * non-coherent (non dma-alloc) allocation path; coherent/dma > > pools continue > > + * to use the device-local node. Intended for drivers that expose > > a > > + * user-selectable host memory NUMA placement. > > + * > > + * Returns: 0 on success, -EOPNOTSUPP if @nid is specified on a > > coherent/dma > > + * pool, negative error code otherwise. > > + */ > > +int ttm_pool_alloc_node(struct ttm_pool *pool, struct ttm_tt *tt, > > + struct ttm_operation_ctx *ctx, int nid) > > +{ > > + struct ttm_pool_alloc_state alloc; > > + > > + if (WARN_ON(ttm_tt_is_backed_up(tt))) > > + return -EINVAL; > > + > > + /* > > + * Coherent/dma pools allocate via dma_alloc_attrs(), > > which places pages > > + * per device DMA constraints and cannot honor a caller- > > requested node. > > + */ > > + if (nid != NUMA_NO_NODE && ttm_pool_uses_dma_alloc(pool)) > > + return -EOPNOTSUPP; > > + > > + if (nid == NUMA_NO_NODE) > > + nid = ttm_pool_nid(pool); > > + > > + ttm_pool_alloc_state_init(tt, &alloc); > > + > > + return __ttm_pool_alloc(pool, tt, ctx, &alloc, NULL, nid); > > +} > > +EXPORT_SYMBOL(ttm_pool_alloc_node); > > + > > /** > > * ttm_pool_restore_and_alloc - Fill a ttm_tt, restoring > > previously backed-up > > * content. > > @@ -964,7 +1007,7 @@ int ttm_pool_restore_and_alloc(struct ttm_pool > > *pool, struct ttm_tt *tt, > > } > > } > > > > - return __ttm_pool_alloc(pool, tt, ctx, &alloc, restore); > > + return __ttm_pool_alloc(pool, tt, ctx, &alloc, restore, > > ttm_pool_nid(pool)); > > } > > > > /** > > diff --git a/include/drm/ttm/ttm_pool.h > > b/include/drm/ttm/ttm_pool.h > > index 26ee592e1994..6a0b14aca092 100644 > > --- a/include/drm/ttm/ttm_pool.h > > +++ b/include/drm/ttm/ttm_pool.h > > @@ -79,6 +79,8 @@ struct ttm_pool { > > > > int ttm_pool_alloc(struct ttm_pool *pool, struct ttm_tt *tt, > > struct ttm_operation_ctx *ctx); > > +int ttm_pool_alloc_node(struct ttm_pool *pool, struct ttm_tt *tt, > > + struct ttm_operation_ctx *ctx, int nid); > > void ttm_pool_free(struct ttm_pool *pool, struct ttm_tt *tt); > > > > void ttm_pool_init(struct ttm_pool *pool, struct device *dev,
