[Mesa-dev] [PATCH 2/9] i965: Check last known busy status on bo before asking the kernel

Chris Wilson chris at chris-wilson.co.uk
Thu Jun 15 08:41:39 UTC 2017


Quoting Kenneth Graunke (2017-06-14 22:49:01)
> On Friday, June 9, 2017 6:01:33 AM PDT Chris Wilson wrote:
> > If we know the bo is idle (that is we have no submitted a command buffer
> > referencing this bo since the last query) we can skip asking the kernel.
> > Note this may report a false negative if the target is being shared
> > between processes (exported via dmabuf or flink). To allow the caller
> > control over using the last known flag, the query is split into two.
> 
> I'm not crazy about exposing __brw_bo_busy and brw_bo_busy, with slightly
> different semantics.  Why not just make brw_bo_busy do:
> 
>    if (bo->idle && bo->reusable)
>       return false;
> 
>    /* otherwise query the kernel */
> 
> These days, it appears that bo->reusable is false for any buffers that
> have been imported/exported via dmabuf or flink, and true otherwise.
> (We might want to rename it to bo->foreign or such.)

I set bo->reusable to false on snooped buffers as we don't yet handle
mixed modes in the bo cache. To offset that I was thinking of having a
specific cache for qbo. We do have flags we pass to bo_alloc, so
supporting a cache of snoopable, or even uncached on llc, within bufmgr
isn't hard.

An alternative is to have bo->exported, which is useful if we ever want
to disregard our state tracking.
 
> With that change, brw_bo_busy should bypass the ioctl for most BOs, but
> would still work for foreign BOs, without the caller having to worry
> about it.

Just not qbo right now :)

> >  /**
> > - * Returns 1 if mapping the buffer for write could cause the process
> > - * to block, due to the object being active in the GPU.
> > + * Returns 0 if mapping the buffer is not in active use by the gpu.
> > + * If non-zero, any mapping for for write could cause the process
> > + * to block, due to the object being active in the GPU. If the lower
> > + * 16 bits are zero, then we can map for read without stalling.
> > + *
> > + * The last-known busy status of the brw_bo is checked first. This may be
> > + * stale if the brw_bo has been exported to a foriegn process. If used on an
> > + * exported bo, call __brw_bo_busy() directly to bypass the local check.
> >   */
> > -int brw_bo_busy(struct brw_bo *bo);
> > +static inline int brw_bo_busy(struct brw_bo *bo)
> > +{
> > +   if (bo->idle) /* Note this may be stale if the bo is exported */
> > +      return 0;
> > +
> > +   return __brw_bo_busy(bo);
> > +}
> 
> I'd rather keep this as a boolean result, rather than an integer with
> certain bits having particular meanings.  Bonus points for changing the
> return type to "bool".

The different bits are significant, especially in the case where you do
want to distinguish ready to read vs ready to write. Such as reading
back a qbo that might also be still in use for GPU queries.
-Chris


More information about the mesa-dev mailing list