diff mbox series

[v4,05/33] drm/xe/bo: Introduce xe_bo_put_async

Message ID 20250129195212.745731-6-matthew.brost@intel.com (mailing list archive)
State New, archived
Headers show
Series Introduce GPU SVM and Xe SVM implementation | expand

Commit Message

Matthew Brost Jan. 29, 2025, 7:51 p.m. UTC 
From: Thomas Hellström <thomas.hellstrom@linux.intel.com>

Introduce xe_bo_put_async to put a bo where the context is such that
the bo destructor can't run due to lockdep problems or atomic context.

If the put is the final put, freeing will be done from a work item.

Signed-off-by: Matthew Brost <matthew.brost@intel.com>
Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
---
 drivers/gpu/drm/xe/xe_bo.c           | 25 +++++++++++++++++++++++++
 drivers/gpu/drm/xe/xe_bo.h           | 13 +++++++++++++
 drivers/gpu/drm/xe/xe_device.c       |  3 +++
 drivers/gpu/drm/xe/xe_device_types.h |  8 ++++++++
 4 files changed, 49 insertions(+)

Comments

Thomas Hellström Jan. 30, 2025, 8:49 a.m. UTC | #1 
On Wed, 2025-01-29 at 11:51 -0800, Matthew Brost wrote:
> From: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> 
> Introduce xe_bo_put_async to put a bo where the context is such that
> the bo destructor can't run due to lockdep problems or atomic
> context.
> 
> If the put is the final put, freeing will be done from a work item.
> 
> Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> ---
>  drivers/gpu/drm/xe/xe_bo.c           | 25 +++++++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_bo.h           | 13 +++++++++++++
>  drivers/gpu/drm/xe/xe_device.c       |  3 +++
>  drivers/gpu/drm/xe/xe_device_types.h |  8 ++++++++
>  4 files changed, 49 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> index fb1629d9d566..e914a60b8afc 100644
> --- a/drivers/gpu/drm/xe/xe_bo.c
> +++ b/drivers/gpu/drm/xe/xe_bo.c
> @@ -2544,6 +2544,31 @@ void xe_bo_put_commit(struct llist_head
> *deferred)
>  		drm_gem_object_free(&bo->ttm.base.refcount);
>  }
>  
> +static void xe_bo_dev_work_func(struct work_struct *work)
> +{
> +	struct xe_bo_dev *bo_dev = container_of(work,
> typeof(*bo_dev), async_free);
> +
> +	xe_bo_put_commit(&bo_dev->async_list);
> +}
> +
> +/**
> + * xe_bo_dev_init() - Initialize BO dev to manage async BO freeing
> + * @bo_dev: The BO dev structure
> + */
> +void xe_bo_dev_init(struct xe_bo_dev *bo_dev)
> +{
> +	INIT_WORK(&bo_dev->async_free, xe_bo_dev_work_func);
> +}
> +
> +/**
> + * xe_bo_dev_fini() - Finalize BO dev managing async BO freeing
> + * @bo_dev: The BO dev structure
> + */
> +void xe_bo_dev_fini(struct xe_bo_dev *bo_dev)
> +{
> +	flush_work(&bo_dev->async_free);
> +}
> +
>  void xe_bo_put(struct xe_bo *bo)
>  {
>  	struct xe_tile *tile;
> diff --git a/drivers/gpu/drm/xe/xe_bo.h b/drivers/gpu/drm/xe/xe_bo.h
> index 04995c5ced32..ce55a2bb13f6 100644
> --- a/drivers/gpu/drm/xe/xe_bo.h
> +++ b/drivers/gpu/drm/xe/xe_bo.h
> @@ -317,6 +317,19 @@ xe_bo_put_deferred(struct xe_bo *bo, struct
> llist_head *deferred)
>  
>  void xe_bo_put_commit(struct llist_head *deferred);
>  
> +static inline void
> +xe_bo_put_async(struct xe_bo *bo)

Needs kerneldoc. I will rebase my multi-device series on this one, Let
me know if you'll add that or if I should do it when rebasing my multi-
device series on this one.

> +{
> +	struct xe_bo_dev *bo_device = &xe_bo_device(bo)->bo_device;
> +
> +	if (xe_bo_put_deferred(bo, &bo_device->async_list))
> +		schedule_work(&bo_device->async_free);
> +}
> +
> +void xe_bo_dev_init(struct xe_bo_dev *bo_device);
> +
> +void xe_bo_dev_fini(struct xe_bo_dev *bo_device);
> +
>  struct sg_table *xe_bo_sg(struct xe_bo *bo);
>  
>  /*
> diff --git a/drivers/gpu/drm/xe/xe_device.c
> b/drivers/gpu/drm/xe/xe_device.c
> index 8fedc72e9db4..5fac3d40cc8e 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -387,6 +387,8 @@ static void xe_device_destroy(struct drm_device
> *dev, void *dummy)
>  {
>  	struct xe_device *xe = to_xe_device(dev);
>  
> +	xe_bo_dev_fini(&xe->bo_device);
> +
>  	if (xe->preempt_fence_wq)
>  		destroy_workqueue(xe->preempt_fence_wq);
>  
> @@ -424,6 +426,7 @@ struct xe_device *xe_device_create(struct pci_dev
> *pdev,
>  	if (WARN_ON(err))
>  		goto err;
>  
> +	xe_bo_dev_init(&xe->bo_device);
>  	err = drmm_add_action_or_reset(&xe->drm, xe_device_destroy,
> NULL);
>  	if (err)
>  		goto err;
> diff --git a/drivers/gpu/drm/xe/xe_device_types.h
> b/drivers/gpu/drm/xe/xe_device_types.h
> index 89f532b67bc4..71151532e28f 100644
> --- a/drivers/gpu/drm/xe/xe_device_types.h
> +++ b/drivers/gpu/drm/xe/xe_device_types.h
> @@ -519,6 +519,14 @@ struct xe_device {
>  		int mode;
>  	} wedged;
>  
> +	/** @bo_device: Struct to control async free of BOs */
> +	struct xe_bo_dev {
> +		/** @async_free: Free worker */
> +		struct work_struct async_free;
> +		/** @async_list: List of BOs to be freed */
> +		struct llist_head async_list;
> +	} bo_device;
> +
>  	/** @pmu: performance monitoring unit */
>  	struct xe_pmu pmu;
>
Matthew Brost Jan. 30, 2025, 4:26 p.m. UTC | #2 
On Thu, Jan 30, 2025 at 09:49:54AM +0100, Thomas Hellström wrote:
> On Wed, 2025-01-29 at 11:51 -0800, Matthew Brost wrote:
> > From: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> > 
> > Introduce xe_bo_put_async to put a bo where the context is such that
> > the bo destructor can't run due to lockdep problems or atomic
> > context.
> > 
> > If the put is the final put, freeing will be done from a work item.
> > 
> > Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> > Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_bo.c           | 25 +++++++++++++++++++++++++
> >  drivers/gpu/drm/xe/xe_bo.h           | 13 +++++++++++++
> >  drivers/gpu/drm/xe/xe_device.c       |  3 +++
> >  drivers/gpu/drm/xe/xe_device_types.h |  8 ++++++++
> >  4 files changed, 49 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> > index fb1629d9d566..e914a60b8afc 100644
> > --- a/drivers/gpu/drm/xe/xe_bo.c
> > +++ b/drivers/gpu/drm/xe/xe_bo.c
> > @@ -2544,6 +2544,31 @@ void xe_bo_put_commit(struct llist_head
> > *deferred)
> >  		drm_gem_object_free(&bo->ttm.base.refcount);
> >  }
> >  
> > +static void xe_bo_dev_work_func(struct work_struct *work)
> > +{
> > +	struct xe_bo_dev *bo_dev = container_of(work,
> > typeof(*bo_dev), async_free);
> > +
> > +	xe_bo_put_commit(&bo_dev->async_list);
> > +}
> > +
> > +/**
> > + * xe_bo_dev_init() - Initialize BO dev to manage async BO freeing
> > + * @bo_dev: The BO dev structure
> > + */
> > +void xe_bo_dev_init(struct xe_bo_dev *bo_dev)
> > +{
> > +	INIT_WORK(&bo_dev->async_free, xe_bo_dev_work_func);
> > +}
> > +
> > +/**
> > + * xe_bo_dev_fini() - Finalize BO dev managing async BO freeing
> > + * @bo_dev: The BO dev structure
> > + */
> > +void xe_bo_dev_fini(struct xe_bo_dev *bo_dev)
> > +{
> > +	flush_work(&bo_dev->async_free);
> > +}
> > +
> >  void xe_bo_put(struct xe_bo *bo)
> >  {
> >  	struct xe_tile *tile;
> > diff --git a/drivers/gpu/drm/xe/xe_bo.h b/drivers/gpu/drm/xe/xe_bo.h
> > index 04995c5ced32..ce55a2bb13f6 100644
> > --- a/drivers/gpu/drm/xe/xe_bo.h
> > +++ b/drivers/gpu/drm/xe/xe_bo.h
> > @@ -317,6 +317,19 @@ xe_bo_put_deferred(struct xe_bo *bo, struct
> > llist_head *deferred)
> >  
> >  void xe_bo_put_commit(struct llist_head *deferred);
> >  
> > +static inline void
> > +xe_bo_put_async(struct xe_bo *bo)
> 
> Needs kerneldoc. I will rebase my multi-device series on this one, Let
> me know if you'll add that or if I should do it when rebasing my multi-
> device series on this one.
> 

Yep. Added kernel for structures / exported functions but missed this
inline. I should be able to write something here.

Matt

> > +{
> > +	struct xe_bo_dev *bo_device = &xe_bo_device(bo)->bo_device;
> > +
> > +	if (xe_bo_put_deferred(bo, &bo_device->async_list))
> > +		schedule_work(&bo_device->async_free);
> > +}
> > +
> > +void xe_bo_dev_init(struct xe_bo_dev *bo_device);
> > +
> > +void xe_bo_dev_fini(struct xe_bo_dev *bo_device);
> > +
> >  struct sg_table *xe_bo_sg(struct xe_bo *bo);
> >  
> >  /*
> > diff --git a/drivers/gpu/drm/xe/xe_device.c
> > b/drivers/gpu/drm/xe/xe_device.c
> > index 8fedc72e9db4..5fac3d40cc8e 100644
> > --- a/drivers/gpu/drm/xe/xe_device.c
> > +++ b/drivers/gpu/drm/xe/xe_device.c
> > @@ -387,6 +387,8 @@ static void xe_device_destroy(struct drm_device
> > *dev, void *dummy)
> >  {
> >  	struct xe_device *xe = to_xe_device(dev);
> >  
> > +	xe_bo_dev_fini(&xe->bo_device);
> > +
> >  	if (xe->preempt_fence_wq)
> >  		destroy_workqueue(xe->preempt_fence_wq);
> >  
> > @@ -424,6 +426,7 @@ struct xe_device *xe_device_create(struct pci_dev
> > *pdev,
> >  	if (WARN_ON(err))
> >  		goto err;
> >  
> > +	xe_bo_dev_init(&xe->bo_device);
> >  	err = drmm_add_action_or_reset(&xe->drm, xe_device_destroy,
> > NULL);
> >  	if (err)
> >  		goto err;
> > diff --git a/drivers/gpu/drm/xe/xe_device_types.h
> > b/drivers/gpu/drm/xe/xe_device_types.h
> > index 89f532b67bc4..71151532e28f 100644
> > --- a/drivers/gpu/drm/xe/xe_device_types.h
> > +++ b/drivers/gpu/drm/xe/xe_device_types.h
> > @@ -519,6 +519,14 @@ struct xe_device {
> >  		int mode;
> >  	} wedged;
> >  
> > +	/** @bo_device: Struct to control async free of BOs */
> > +	struct xe_bo_dev {
> > +		/** @async_free: Free worker */
> > +		struct work_struct async_free;
> > +		/** @async_list: List of BOs to be freed */
> > +		struct llist_head async_list;
> > +	} bo_device;
> > +
> >  	/** @pmu: performance monitoring unit */
> >  	struct xe_pmu pmu;
> >  
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
index fb1629d9d566..e914a60b8afc 100644
--- a/drivers/gpu/drm/xe/xe_bo.c
+++ b/drivers/gpu/drm/xe/xe_bo.c
@@ -2544,6 +2544,31 @@  void xe_bo_put_commit(struct llist_head *deferred)
 		drm_gem_object_free(&bo->ttm.base.refcount);
 }
 
+static void xe_bo_dev_work_func(struct work_struct *work)
+{
+	struct xe_bo_dev *bo_dev = container_of(work, typeof(*bo_dev), async_free);
+
+	xe_bo_put_commit(&bo_dev->async_list);
+}
+
+/**
+ * xe_bo_dev_init() - Initialize BO dev to manage async BO freeing
+ * @bo_dev: The BO dev structure
+ */
+void xe_bo_dev_init(struct xe_bo_dev *bo_dev)
+{
+	INIT_WORK(&bo_dev->async_free, xe_bo_dev_work_func);
+}
+
+/**
+ * xe_bo_dev_fini() - Finalize BO dev managing async BO freeing
+ * @bo_dev: The BO dev structure
+ */
+void xe_bo_dev_fini(struct xe_bo_dev *bo_dev)
+{
+	flush_work(&bo_dev->async_free);
+}
+
 void xe_bo_put(struct xe_bo *bo)
 {
 	struct xe_tile *tile;
diff --git a/drivers/gpu/drm/xe/xe_bo.h b/drivers/gpu/drm/xe/xe_bo.h
index 04995c5ced32..ce55a2bb13f6 100644
--- a/drivers/gpu/drm/xe/xe_bo.h
+++ b/drivers/gpu/drm/xe/xe_bo.h
@@ -317,6 +317,19 @@  xe_bo_put_deferred(struct xe_bo *bo, struct llist_head *deferred)
 
 void xe_bo_put_commit(struct llist_head *deferred);
 
+static inline void
+xe_bo_put_async(struct xe_bo *bo)
+{
+	struct xe_bo_dev *bo_device = &xe_bo_device(bo)->bo_device;
+
+	if (xe_bo_put_deferred(bo, &bo_device->async_list))
+		schedule_work(&bo_device->async_free);
+}
+
+void xe_bo_dev_init(struct xe_bo_dev *bo_device);
+
+void xe_bo_dev_fini(struct xe_bo_dev *bo_device);
+
 struct sg_table *xe_bo_sg(struct xe_bo *bo);
 
 /*
diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
index 8fedc72e9db4..5fac3d40cc8e 100644
--- a/drivers/gpu/drm/xe/xe_device.c
+++ b/drivers/gpu/drm/xe/xe_device.c
@@ -387,6 +387,8 @@  static void xe_device_destroy(struct drm_device *dev, void *dummy)
 {
 	struct xe_device *xe = to_xe_device(dev);
 
+	xe_bo_dev_fini(&xe->bo_device);
+
 	if (xe->preempt_fence_wq)
 		destroy_workqueue(xe->preempt_fence_wq);
 
@@ -424,6 +426,7 @@  struct xe_device *xe_device_create(struct pci_dev *pdev,
 	if (WARN_ON(err))
 		goto err;
 
+	xe_bo_dev_init(&xe->bo_device);
 	err = drmm_add_action_or_reset(&xe->drm, xe_device_destroy, NULL);
 	if (err)
 		goto err;
diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
index 89f532b67bc4..71151532e28f 100644
--- a/drivers/gpu/drm/xe/xe_device_types.h
+++ b/drivers/gpu/drm/xe/xe_device_types.h
@@ -519,6 +519,14 @@  struct xe_device {
 		int mode;
 	} wedged;
 
+	/** @bo_device: Struct to control async free of BOs */
+	struct xe_bo_dev {
+		/** @async_free: Free worker */
+		struct work_struct async_free;
+		/** @async_list: List of BOs to be freed */
+		struct llist_head async_list;
+	} bo_device;
+
 	/** @pmu: performance monitoring unit */
 	struct xe_pmu pmu;