On Sat, Oct 25, 2025 at 02:03:58PM +0200, Thomas Hellström wrote: > With the end goal of being able to free unused pagemaps > and allocate them on demand, add a refcount to struct drm_pagemap, > remove the xe embedded drm_pagemap, allocating and freeing it > explicitly. >
I think techincally we can leak a drm_pagemap in this patch by itself, but since the ref counting scheme is refined later in the series, likely not worth adding drmm/devm handler to drop this reference. Anyways, patch itself look good to me: Reviewed-by: Matthew Brost <[email protected]> > Signed-off-by: Thomas Hellström <[email protected]> > --- > drivers/gpu/drm/drm_pagemap.c | 51 ++++++++++++++++++++++++++++++ > drivers/gpu/drm/xe/xe_svm.c | 26 ++++++++++----- > drivers/gpu/drm/xe/xe_vram_types.h | 2 +- > include/drm/drm_pagemap.h | 36 +++++++++++++++++++++ > 4 files changed, 106 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/drm_pagemap.c b/drivers/gpu/drm/drm_pagemap.c > index 22c44807e3fe..4b8692f0b2a2 100644 > --- a/drivers/gpu/drm/drm_pagemap.c > +++ b/drivers/gpu/drm/drm_pagemap.c > @@ -538,6 +538,57 @@ static int drm_pagemap_migrate_populate_ram_pfn(struct > vm_area_struct *vas, > return -ENOMEM; > } > > +static void drm_pagemap_release(struct kref *ref) > +{ > + struct drm_pagemap *dpagemap = container_of(ref, typeof(*dpagemap), > ref); > + > + kfree(dpagemap); > +} > + > +/** > + * drm_pagemap_create() - Create a struct drm_pagemap. > + * @dev: Pointer to a struct device providing the device-private memory. > + * @pagemap: Pointer to a pre-setup struct dev_pagemap providing the struct > pages. > + * @ops: Pointer to the struct drm_pagemap_ops. > + * > + * Allocate and initialize a struct drm_pagemap. > + * > + * Return: A refcounted pointer to a struct drm_pagemap on success. > + * Error pointer on error. > + */ > +struct drm_pagemap * > +drm_pagemap_create(struct device *dev, > + struct dev_pagemap *pagemap, > + const struct drm_pagemap_ops *ops) > +{ > + struct drm_pagemap *dpagemap = kzalloc(sizeof(*dpagemap), GFP_KERNEL); > + > + if (!dpagemap) > + return ERR_PTR(-ENOMEM); > + > + kref_init(&dpagemap->ref); > + dpagemap->dev = dev; > + dpagemap->ops = ops; > + dpagemap->pagemap = pagemap; > + > + return dpagemap; > +} > +EXPORT_SYMBOL(drm_pagemap_create); > + > +/** > + * drm_pagemap_put() - Put a struct drm_pagemap reference > + * @dpagemap: Pointer to a struct drm_pagemap object. > + * > + * Puts a struct drm_pagemap reference and frees the drm_pagemap object > + * if the refount reaches zero. > + */ > +void drm_pagemap_put(struct drm_pagemap *dpagemap) > +{ > + if (likely(dpagemap)) > + kref_put(&dpagemap->ref, drm_pagemap_release); > +} > +EXPORT_SYMBOL(drm_pagemap_put); > + > /** > * drm_pagemap_evict_to_ram() - Evict GPU SVM range to RAM > * @devmem_allocation: Pointer to the device memory allocation > diff --git a/drivers/gpu/drm/xe/xe_svm.c b/drivers/gpu/drm/xe/xe_svm.c > index 129e7818565c..6d2c6c144315 100644 > --- a/drivers/gpu/drm/xe/xe_svm.c > +++ b/drivers/gpu/drm/xe/xe_svm.c > @@ -861,7 +861,7 @@ static int xe_drm_pagemap_populate_mm(struct drm_pagemap > *dpagemap, > struct mm_struct *mm, > unsigned long timeslice_ms) > { > - struct xe_vram_region *vr = container_of(dpagemap, typeof(*vr), > dpagemap); > + struct xe_vram_region *vr = container_of(dpagemap->pagemap, > typeof(*vr), pagemap); > struct xe_device *xe = vr->xe; > struct device *dev = xe->drm.dev; > struct drm_buddy_block *block; > @@ -1372,7 +1372,7 @@ u8 xe_svm_ranges_zap_ptes_in_range(struct xe_vm *vm, > u64 start, u64 end) > > static struct drm_pagemap *tile_local_pagemap(struct xe_tile *tile) > { > - return &tile->mem.vram->dpagemap; > + return tile->mem.vram->dpagemap; > } > > /** > @@ -1482,6 +1482,15 @@ int xe_devm_add(struct xe_tile *tile, struct > xe_vram_region *vr) > return ret; > } > > + vr->dpagemap = drm_pagemap_create(dev, &vr->pagemap, > + &xe_drm_pagemap_ops); > + if (IS_ERR(vr->dpagemap)) { > + drm_err(&xe->drm, "Failed to create drm_pagemap tile %d memory: > %pe\n", > + tile->id, vr->dpagemap); > + ret = PTR_ERR(vr->dpagemap); > + goto out_no_dpagemap; > + } > + > vr->pagemap.type = MEMORY_DEVICE_PRIVATE; > vr->pagemap.range.start = res->start; > vr->pagemap.range.end = res->end; > @@ -1489,22 +1498,23 @@ int xe_devm_add(struct xe_tile *tile, struct > xe_vram_region *vr) > vr->pagemap.ops = drm_pagemap_pagemap_ops_get(); > vr->pagemap.owner = xe_svm_devm_owner(xe); > addr = devm_memremap_pages(dev, &vr->pagemap); > - > - vr->dpagemap.dev = dev; > - vr->dpagemap.ops = &xe_drm_pagemap_ops; > - > if (IS_ERR(addr)) { > - devm_release_mem_region(dev, res->start, resource_size(res)); > ret = PTR_ERR(addr); > drm_err(&xe->drm, "Failed to remap tile %d memory, errno %pe\n", > tile->id, ERR_PTR(ret)); > - return ret; > + goto out_failed_memremap; > } > vr->hpa_base = res->start; > > drm_dbg(&xe->drm, "Added tile %d memory [%llx-%llx] to devm, remapped > to %pr\n", > tile->id, vr->io_start, vr->io_start + vr->usable_size, res); > return 0; > + > +out_failed_memremap: > + drm_pagemap_put(vr->dpagemap); > +out_no_dpagemap: > + devm_release_mem_region(dev, res->start, resource_size(res)); > + return ret; > } > #else > int xe_svm_alloc_vram(struct xe_tile *tile, > diff --git a/drivers/gpu/drm/xe/xe_vram_types.h > b/drivers/gpu/drm/xe/xe_vram_types.h > index 83772dcbf1af..c0d2c5ee8c10 100644 > --- a/drivers/gpu/drm/xe/xe_vram_types.h > +++ b/drivers/gpu/drm/xe/xe_vram_types.h > @@ -72,7 +72,7 @@ struct xe_vram_region { > * @dpagemap: The struct drm_pagemap of the ZONE_DEVICE memory > * pages of this tile. > */ > - struct drm_pagemap dpagemap; > + struct drm_pagemap *dpagemap; > /** > * @hpa_base: base host physical address > * > diff --git a/include/drm/drm_pagemap.h b/include/drm/drm_pagemap.h > index f6e7e234c089..2c7de928865b 100644 > --- a/include/drm/drm_pagemap.h > +++ b/include/drm/drm_pagemap.h > @@ -129,11 +129,15 @@ struct drm_pagemap_ops { > * struct drm_pagemap: Additional information for a struct dev_pagemap > * used for device p2p handshaking. > * @ops: The struct drm_pagemap_ops. > + * @ref: Reference count. > * @dev: The struct drevice owning the device-private memory. > + * @pagemap: Pointer to the underlying dev_pagemap. > */ > struct drm_pagemap { > const struct drm_pagemap_ops *ops; > + struct kref ref; > struct device *dev; > + struct dev_pagemap *pagemap; > }; > > struct drm_pagemap_devmem; > @@ -202,6 +206,37 @@ struct drm_pagemap_devmem_ops { > unsigned long npages); > }; > > +struct drm_pagemap *drm_pagemap_create(struct device *dev, > + struct dev_pagemap *pagemap, > + const struct drm_pagemap_ops *ops); > + > +#if IS_ENABLED(CONFIG_DRM_GPUSVM) > + > +void drm_pagemap_put(struct drm_pagemap *dpagemap); > + > +#else > + > +static inline void drm_pagemap_put(struct drm_pagemap *dpagemap) > +{ > +} > + > +#endif /* IS_ENABLED(CONFIG_DRM_GPUSVM) */ > + > +/** > + * drm_pagemap_get() - Obtain a reference on a struct drm_pagemap > + * @dpagemap: Pointer to the struct drm_pagemap. > + * > + * Return: Pointer to the struct drm_pagemap. > + */ > +static inline struct drm_pagemap * > +drm_pagemap_get(struct drm_pagemap *dpagemap) > +{ > + if (likely(dpagemap)) > + kref_get(&dpagemap->ref); > + > + return dpagemap; > +} > + > /** > * struct drm_pagemap_devmem - Structure representing a GPU SVM device > memory allocation > * > @@ -246,3 +281,4 @@ int drm_pagemap_populate_mm(struct drm_pagemap *dpagemap, > unsigned long timeslice_ms); > > #endif > + > -- > 2.51.0 >
