[Mesa-dev] [PATCH 6/7] nir: add shader reference counting

[Kenneth Graunke]

On Saturday, October 24, 2015 01:07:59 PM Rob Clark wrote:
> From: Rob Clark <robclark at freedesktop.org>
> 
> For gallium, at least, we'll need this to manage shader's lifetimes,
> since in some cases both the driver and the state tracker will need
> to hold on to a reference for variant managing.
> 
> Use nir_shader_mutable() before doing any IR opt/lowering/etc, to
> ensure you are not modifying a copy someone else is also holding a
> reference to.  In this way, unnecessary nir_shader_clone()s are
> avoided whenever possible.
> 
> TODO: Any places missed to s/ralloc_free()/nir_shader_unref()/ outside
> of freedreno/ir3?
> 
> Signed-off-by: Rob Clark <robclark at freedesktop.org>

I'm pretty uncomfortable with mixing ralloc and reference counting.

A ralloc context is essentially a linked list of child allocations.
Freeing an item alters the parent context's linked list.  As such,
this is inherently not threadsafe at all.  (It works great if you
keep memory contexts within a single thread...)

I'm afraid that if a NIR shader is used in multiple threads, with
reference counting...and allocated out of a parent context...then
we could corrupt the parent memory context's list, which would be
an awful bug to track down.

If reference counting is useful, then perhaps the best solution is
to alter nir_shader_create() to *not* take a mem_ctx parameter, and
simply do:

   nir_shader *shader = ralloc(NULL, nir_shader);

Then, we're guaranteed that nir_shaders won't have a parent memory
context, avoiding the potential threading fiasco.  We can still use
ralloc safely within a shader, but manage whole shaders entirely with
reference counting...

--Ken
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20151105/9096c79b/attachment.sig>