Re: [PATCH v3 9/9] drm/tegra: Support context isolation

[Mikko Perttunen]

On 2/19/22 20:35, Dmitry Osipenko wrote:
> 18.02.2022 14:39, Mikko Perttunen пишет:
> > +	if (context->memory_context && context->client->ops->get_streamid_offset) {
>              ^^^
> > +		int offset = context->client->ops->get_streamid_offset(context->client);
> > +
> > +		if (offset >= 0) {
> > +			job->context = context->memory_context;
> > +			job->engine_streamid_offset = offset;
> > +			host1x_context_get(job->context);
> > +		}
>
> You should bump refcount unconditionally or you'll get refcnt underflow
> on put, when offset < 0.

This refcount is intended to be dropped from 'release_job', where it's 
dropped if job->context is set, which it is from this path.

> > +	}
> > +
> >   	/*
> >   	 * job_data is now part of job reference counting, so don't release
> >   	 * it from here.
> > diff --git a/drivers/gpu/drm/tegra/uapi.c b/drivers/gpu/drm/tegra/uapi.c
> > index 9ab9179d2026..be33da54d12c 100644
> > --- a/drivers/gpu/drm/tegra/uapi.c
> > +++ b/drivers/gpu/drm/tegra/uapi.c
> > @@ -33,6 +33,9 @@ static void tegra_drm_channel_context_close(struct tegra_drm_context *context)
> >   	struct tegra_drm_mapping *mapping;
> >   	unsigned long id;
> >   
> > +	if (context->memory_context)
> > +		host1x_context_put(context->memory_context);
>
> The "if (context->memory_context &&
> context->client->ops->get_streamid_offset)" above doesn't match the "if
> (context->memory_context)". You'll get refcount underflow.

And this drop is for the refcount implicitly added when allocating the 
memory context through host1x_context_alloc; so these two places should 
be independent.

Please elaborate if I missed something.

Thanks,
Mikko