| Message ID | 20241227111128.1546618-1-apoorva.singh@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm/i915/gt: Add GEM_BUG_ON to ensure at least one engine supports migration | expand |
On 27/12/2024 11:11, apoorva.singh@intel.com wrote: > From: Apoorva Singh <apoorva.singh@intel.com> > > Ensure GEM_BUG_ON verifies at least one engine supports migration. > Additionally, it prevents passing an uninitialized variable to > intel_context_create(). Under what circumstances can this happen ie. does it need a Fixes: tag? Also consider that if it can happen in production on some real hardware then GEM_BUG_ON does not prevent anything (GEM_BUG_ON is compiled out). On debug builds it also strictly doesn't prevent anything but crashes the machine. Consider this as an alternative (if the error scenario is real): diff --git a/drivers/gpu/drm/i915/gt/intel_migrate.c b/drivers/gpu/drm/i915/gt/intel_migrate.c index 6f7af4077135..83e6c20cb750 100644 --- a/drivers/gpu/drm/i915/gt/intel_migrate.c +++ b/drivers/gpu/drm/i915/gt/intel_migrate.c @@ -296,7 +296,9 @@ static struct intel_context *__migrate_engines(struct intel_gt *gt) engines[count++] = engine; } - return intel_context_create(engines[random_index(count)]); + return count ? + intel_context_create(engines[random_index(count)]) : + ERR_PTR(-ENOENT); } struct intel_context *intel_migrate_create_context(struct intel_migrate *m) But check if the whole error propagation chain is fine to handle it. Regards, Tvrtko > Cc: Chris Wilson <chris.p.wilson@linux.intel.com> > Signed-off-by: Apoorva Singh <apoorva.singh@intel.com> > --- > drivers/gpu/drm/i915/gt/intel_migrate.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/i915/gt/intel_migrate.c b/drivers/gpu/drm/i915/gt/intel_migrate.c > index 6f7af4077135..528a7cba3623 100644 > --- a/drivers/gpu/drm/i915/gt/intel_migrate.c > +++ b/drivers/gpu/drm/i915/gt/intel_migrate.c > @@ -296,6 +296,9 @@ static struct intel_context *__migrate_engines(struct intel_gt *gt) > engines[count++] = engine; > } > > + /* At least one engine must support migration! */ > + GEM_BUG_ON(!count); > + > return intel_context_create(engines[random_index(count)]); > } >
diff --git a/drivers/gpu/drm/i915/gt/intel_migrate.c b/drivers/gpu/drm/i915/gt/intel_migrate.c index 6f7af4077135..528a7cba3623 100644 --- a/drivers/gpu/drm/i915/gt/intel_migrate.c +++ b/drivers/gpu/drm/i915/gt/intel_migrate.c @@ -296,6 +296,9 @@ static struct intel_context *__migrate_engines(struct intel_gt *gt) engines[count++] = engine; } + /* At least one engine must support migration! */ + GEM_BUG_ON(!count); + return intel_context_create(engines[random_index(count)]); }