[igt-dev] [PATCH i-g-t v33 40/40] lib/intel_bufops: address review comments (clarify buffer ownership rules)

Zbigniew Kempczyński zbigniew.kempczynski at intel.com
Fri Sep 11 05:55:50 UTC 2020


On Thu, Sep 10, 2020 at 02:33:02PM +0100, Chris Wilson wrote:
> Quoting Zbigniew Kempczyński (2020-08-31 14:30:49)
> > To avoid intel_buf handle leakage we need to clarify of buffer ownership
> > rules. So we currently have:
> > 
> > 1. intel_buf_init(), intel_buf_create() which create handle and take
> >    ownership of such handle.
> > 2. intel_buf_init_using_handle(), intel_buf_create_using_handle() which
> >    take bo handle from the caller and doesn't take ownership.
> > 
> > intel_buf_close()/intel_buf_destroy() will honour ownership and skip
> > closing handle if buffer is not owned.
> > 
> > To take/release buffer ownership intel_buf_set_ownership() can be used.
> > 
> > Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
> > Cc: Dominik Grzegorzek <dominik.grzegorzek at intel.com>
> > Cc: Chris Wilson <chris at chris-wilson.co.uk>
> 
> Ok, the changes look fine, I didn't find anything else to complain about
> and the only test that failed in local testing was gem_concurrent_blit,
> so have a
> Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
> for the series (with the exception of the changes to
> gem_concurrent_all.c)
> 
> Do you want to squash the fixups and drop the unready patch for merging?
> -Chris

Problem with dropping the patch for gem_concurrent_blit makes it also
unsuable because rendercopy now uses intel_bb instead of intel_batchbuffer.
Other fixups of course can be squashed - I added them on stack of changes
to easily review - doing review of few thousand lines is not comfortable
(I'm trying to handle that keeping previous branch and do copy-compare
in git-difftool, if there's better way to do that please share).

Thanks for the review, please decide what we're doing with gem_concurrent_blit.

--
Zbigniew


More information about the igt-dev mailing list