| Message ID | 20250116193528.2297487-1-ingyujang25@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm/i915/gt: Handle INTEL_WAKEREF_DEF return value in gen8_ggtt_bind_get_ce | expand |
Hi Ingyu, > diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c > index d60a6ca0cae5..8d22d8f2243d 100644 > --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c > +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c > @@ -311,7 +311,7 @@ static struct intel_context *gen8_ggtt_bind_get_ce(struct i915_ggtt *ggtt, intel > * doing rpm_resume(). > */ > *wakeref = intel_gt_pm_get_if_awake(gt); > - if (!*wakeref) > + if (!*wakeref || *wakeref == INTEL_WAKEREF_DEF) INTEL_WAKEREF_DEF is a wrapper for an error pointer - how about IS_ERR_OR_NULL() macro? Without going a bit deeper into the code it is not apparent that INTEL_WAKEREF_DEF is indicating an error. Nice catch nevertheless. Krzysztof > return NULL; > > intel_engine_pm_get(ce->engine); > -- > 2.34.1 >
On Wed, 22 Jan 2025, Krzysztof Karas <krzysztof.karas@intel.com> wrote: > Hi Ingyu, > >> diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c >> index d60a6ca0cae5..8d22d8f2243d 100644 >> --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c >> +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c >> @@ -311,7 +311,7 @@ static struct intel_context *gen8_ggtt_bind_get_ce(struct i915_ggtt *ggtt, intel >> * doing rpm_resume(). >> */ >> *wakeref = intel_gt_pm_get_if_awake(gt); >> - if (!*wakeref) >> + if (!*wakeref || *wakeref == INTEL_WAKEREF_DEF) > INTEL_WAKEREF_DEF is a wrapper for an error pointer - how about > IS_ERR_OR_NULL() macro? Without going a bit deeper into the code > it is not apparent that INTEL_WAKEREF_DEF is indicating an error. > > Nice catch nevertheless. It's not a nice catch. It's wrong [1]. BR, Jani. [1] https://lore.kernel.org/r/87cyglg9w2.fsf@intel.com > > Krzysztof > >> return NULL; >> >> intel_engine_pm_get(ce->engine); >> -- >> 2.34.1 >>
diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c index d60a6ca0cae5..8d22d8f2243d 100644 --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c @@ -311,7 +311,7 @@ static struct intel_context *gen8_ggtt_bind_get_ce(struct i915_ggtt *ggtt, intel * doing rpm_resume(). */ *wakeref = intel_gt_pm_get_if_awake(gt); - if (!*wakeref) + if (!*wakeref || *wakeref == INTEL_WAKEREF_DEF) return NULL; intel_engine_pm_get(ce->engine);
The function 'gen8_ggtt_bind_get_ce' previously did not handle the case where 'intel_gt_pm_get_if_awake()' returns 'INTEL_WAKEREF_DEF'. This value is returned when the 'intel_ref_tracker_alloc()' call within 'intel_gt_pm_get_if_awake()' fails to allocate due to memory pressure or other constraints. 'intel_ref_tracker_alloc()' uses 'ref_tracker_alloc()' with the 'GFP_NOWAIT' flag, which can return 'NULL' if allocation fails. In this case, the function explicitly returns 'INTEL_WAKEREF_DEF'. This patch adds a check for 'INTEL_WAKEREF_DEF' in 'gen8_ggtt_bind_get_ce' to ensure that such errors are properly handled. If 'INTEL_WAKEREF_DEF' is returned, the function returns 'NULL' . Signed-off-by: Ingyu Jang <ingyujang25@gmail.com> --- drivers/gpu/drm/i915/gt/intel_ggtt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)