[Intel-gfx] [PATCH 03/25] drm/i915/gem: Update context name on closing

Chris Wilson chris at chris-wilson.co.uk
Mon Nov 11 10:58:50 UTC 2019


Quoting Mika Kuoppala (2019-11-11 10:47:00)
> Chris Wilson <chris at chris-wilson.co.uk> writes:
> 
> > Update the context.name on closing so that the persistent requests are
> > clear in debug prints.
> >
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/gem/i915_gem_context.c | 18 ++++++++++++++++++
> >  1 file changed, 18 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > index 982770e8163d..72d389afa28a 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > @@ -461,11 +461,29 @@ static void kill_context(struct i915_gem_context *ctx)
> >       }
> >  }
> >  
> > +static void set_closed_name(struct i915_gem_context *ctx)
> > +{
> > +     char *s;
> > +
> > +     /* Replace '[]' with '<>' to indicate closed in debug prints */
> > +
> > +     s = strrchr(ctx->name, '[');
> > +     if (!s)
> > +             return;
> > +
> > +     *s = '<';
> > +
> > +     s = strchr(s + 1, ']');
> 
> I can't think of a way for s+1 to be NULL as the TASKCOM_LEN + 8
> makes the [pid] appear at the end.
> 
> With extending the buffer, one could have gone with 
> += "(closed)". To be more readable.

Yeah, [] vs <> is subtle. It's debug so if we want to make it more
obvious later, or encode more information in there, I do not foresee
much problem in doing so.

> But would bloat the buffer more.

What's a few bytes more when we waste pages per context ;)
 
> Which leads to thinking that perhaps we should grab only
> the taskname/pid and then construct the name on the fly.
> 
> That needs buffer for callers, which might be nontrivial
> due to usage on error situations.

Exactly, and we can't allocate there either.
-Chris


More information about the Intel-gfx mailing list