Re: [PATCH] drm/nouveau: nouveau_sched_fini() should check for init failure

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]<

 



On 1/31/24 22:39, Timur Tabi wrote:
If initialization fails, Nouveau can still call the _fini() function
to clean up, with the expectation that the function can handle if its
corresponding _init() function was never called or exited with error.

Such is not the case with nouveau_sched_fini(), which still attempts
to wait for jobs to finish even if the underlying data structures were
never initialized.

Good catch!


Fixes: 5f03a507b29e ("drm/nouveau: implement 1:1 scheduler - entity relationship")
Signed-off-by: Timur Tabi <ttabi@xxxxxxxxxx>
---
  drivers/gpu/drm/nouveau/nouveau_sched.c | 5 +++++
  drivers/gpu/drm/nouveau/nouveau_sched.h | 2 ++
  2 files changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/nouveau_sched.c b/drivers/gpu/drm/nouveau/nouveau_sched.c
index dd98f6910f9c..9c771bc0e332 100644
--- a/drivers/gpu/drm/nouveau/nouveau_sched.c
+++ b/drivers/gpu/drm/nouveau/nouveau_sched.c
@@ -443,6 +443,8 @@ nouveau_sched_init(struct nouveau_sched *sched, struct nouveau_drm *drm,
  	INIT_LIST_HEAD(&sched->job.list.head);
  	init_waitqueue_head(&sched->job.wq);
+ sched->initialized = true;

I wonder if we should allocate struct nouveau_sched dynamically instead and just
check for NULL in the corresponding *_fini() functions.

Actually, in nouveau_abi16_ioctl_channel_alloc() we can omit creating a scheduler
instance entirely if !nouveau_cli_uvmm().

- Danilo

+
  	return 0;
fail_sched:
@@ -459,6 +461,9 @@ nouveau_sched_fini(struct nouveau_sched *sched)
  	struct drm_gpu_scheduler *drm_sched = &sched->base;
  	struct drm_sched_entity *entity = &sched->entity;
+ if (!sched->initialized)
+		return;
+
  	rmb(); /* for list_empty to work without lock */
  	wait_event(sched->job.wq, list_empty(&sched->job.list.head));
diff --git a/drivers/gpu/drm/nouveau/nouveau_sched.h b/drivers/gpu/drm/nouveau/nouveau_sched.h
index a6528f5981e6..351931c706aa 100644
--- a/drivers/gpu/drm/nouveau/nouveau_sched.h
+++ b/drivers/gpu/drm/nouveau/nouveau_sched.h
@@ -109,6 +109,8 @@ struct nouveau_sched {
  		} list;
  		struct wait_queue_head wq;
  	} job;
+
+	bool initialized;
  };
int nouveau_sched_init(struct nouveau_sched *sched, struct nouveau_drm *drm,




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux