On Fri, 17 Jan 2025, Ingyu Jang <ingyujang25@xxxxxxxxx> wrote: > 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' . No. The callers should only handle NULL vs. non-NULL, and never make any other assumptions about the value. If intel_ref_tracker_alloc() fails, we'll only lose the wakeref tracking debug aid, but everything else goes fine. The patch at hand conflates the returned "asleep" (NULL) with "ref tracker fail", and that's wrong. See how intel_ref_tracker_free() handles INTEL_WAKEREF_DEF. BR, Jani. > > Signed-off-by: Ingyu Jang <ingyujang25@xxxxxxxxx> > --- > drivers/gpu/drm/i915/gt/intel_ggtt.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > 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); -- Jani Nikula, Intel
