On Tue, 2025-10-28 at 18:11 -0700, Matthew Brost wrote: > 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. > > > > General commit for the series — could we add some kernel > documentation, > ideally in xe_svm.c, that explains the reference counting scheme used > for drm_pagemap? > > For example: > > - An SVM VM holds a drm_pagemap reference to local pagemaps. > - madvise VMAs hold a reference to the preferred location pagemap. > - Allocated device pages hold a reference to the pagemap. > - The pagemap itself holds a reference to the device/module. > > Reference counting schemes can be difficult to reverse-engineer and > easy > to forget, so it would be best to document them clearly.
Sure. Good idea. /Thomas > > Matt > > > 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 > >
