[Mesa-dev] [PATCH v2 2/2] panfrost: protect access to shared bo cache and transient pool

[Daniel Stone]

Hi Rohan,

On Fri, 30 Aug 2019 at 17:00, Rohan Garg <rohan.garg at collabora.com> wrote:
> Both the BO cache and the transient pool are shared across
> context's. Protect access to these with mutexes.

These fixes seem right to me, and (minus the issues Boris pointed
out), both are:
Reviewed-by: Daniel Stone <daniels at collabora.com>

I think it might be worthwhile pushing both of these to the context in
the longer term. I wonder if we should also rename the 'BO cache' to
something else. (If only so I can stop saying the wrong thing about
'the BO cache'! Maybe we could call it 'BO reuse cache' or similar?)

Another use of BO caching (usually 'BO handle cache') is to handle the
exclusivity requirement around GEM handles. For any given kernel
buffer (wherever it comes from: a panfrost_gem_object allocated by us,
a panfrost_gem_object allocated by another process, a V4L2 buffer,
etc), for every DRM connection (a drm_file represents one open() of
the DRM device), there can only be one GEM handle referring to that
buffer.

drmPrimeFDToHandle() is where you will most often bump into this. If a
client imports the same underlying buffer twice, drmPrimeFDToHandle()
may return a GEM handle which you already have open. Since creating
and destroying GEM handles is not refcounted, this means that you may
get two panfrost_bo's referring to the same GEM handle. When you close
one of those, the other will be left hanging: either you cannot use it
(because the handle is invalid), or you will end up overlapping with
another buffer which later comes to use the same GEM handle. Neither
result is good.

Given that, on the screen (not on the context, since it needs to be
scoped to every user of the same DRM FD), we need a BO handle cache.
The screen would have a list of every panfrost_bo for that device, and
when panfrost_drm_import_bo() receives a GEM handle back from the
kernel, we need to walk the list of all BOs. If we find a panfrost_bo
with the same handle, we just bump the refcount and return that. This
would need a screen-wide lock on the BO handle list.

Would you be able to take a look at that please?

Cheers,
Daniel