[PATCH 1/9] DRI2: Free DRI2 drawable references in DRI2DestroyDrawable

[Christopher James Halse Rogers]

On Thu, 2011-02-03 at 19:48 +0200, Pauli wrote:
> From: Pauli Nieminen <ext-pauli.nieminen at nokia.com>
> 
> If client calls DRI2CreateDrawable multiple times for same drawable
> DRI2 creates multiple references. Multiple references cause DRI2 send
> multiple invalidate events for same client.
> 
> Problem is easy to see in xtrace. Like following filtered snippet from
> problematic client:
> 000:<:0b85: 16: DRI2-Request(132,5): GetBuffers drawable=0x00800011
> attachments={attachment=BackLeft(0x00000001)};
> 000:<:0b89: 32: DRI2-Request(132,8): SwapBuffers drawable=0x00800011
> target_msc_hi=0 target_msc_lo=0 divisor_hi=0 divisor_lo=0 remainder_hi=0
> remainder_lo=0
> 000:>:0b89: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
> 000:>:0b89: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
> 000:>:0b89: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
> 000:>:0b89: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
> 000:>:0b89: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
> 000:>:0b89: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
> 000:>:0b89: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
> 000:>:0b89: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
> 000:>:0b89: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
> 000:>:0b89: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
> 000:>:0b89: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
> 000:>:0b89: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
> 000:>:0b89: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
> 000:>:0b89: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
> 000:<:0b8e: 16: DRI2-Request(132,5): GetBuffers drawable=0x00800011
> attachments={attachment=BackLeft(0x00000001)};
> 000:<:0b8f: 32: DRI2-Request(132,8): SwapBuffers drawable=0x00800011
> target_msc_hi=0 target_msc_lo=0 divisor_hi=0 divisor_lo=0 remainder_hi=0
> remainder_lo=0
> 000:>:0b8f: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
> 000:>:0b8f: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
> 000:>:0b8f: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
> 000:>:0b8f: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
> 000:>:0b8f: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
> 000:>:0b8f: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
> 000:>:0b8f: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
> 000:>:0b8f: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
> 000:>:0b8f: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
> 000:>:0b8f: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
> 000:>:0b8f: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
> 000:>:0b8f: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
> 000:>:0b8f: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
> 000:>:0b8f: Event DRI2-InvalidateBuffers(67) drawable=0x00800011
> 000:<:0bc4: 16: DRI2-Request(132,5): GetBuffers drawable=0x00800011
> attachments={attachment=BackLeft(0x00000001)};
> 000:<:0bc5: 32: DRI2-Request(132,8): SwapBuffers drawable=0x00800011
> target_msc_hi=0 target_msc_lo=0 divisor_hi=0 divisor_lo=0 remainder_hi=0
> remainder_lo=0
> 
> Problem is triggered because client side EGL implementation is using
> DRI2 directly.
> 
> DRI2 code in server doesn't handle DRI2DestroyDrawable that leaks
> references. If client recreates rendering target EGL destroys and
> creates DRI2 drawable. In that case DRI2DestroyDrawable leaks DRI2
> drawable references.
> 
> To fix this memory leak/performance problem server has to free the
> reference when client requests for it.
> 
> Signed-off-by: Pauli Nieminen <ext-pauli.nieminen at nokia.com>
> ---
>  hw/xfree86/dri2/dri2.c    |   31 +++++++++++++++++++++++++++++++
>  hw/xfree86/dri2/dri2.h    |    4 +++-
>  hw/xfree86/dri2/dri2ext.c |    2 +-
>  3 files changed, 35 insertions(+), 2 deletions(-)

Perhaps the xtrace log could be left out of the commit message?  It's
rather long, and I think the rest of the commit message is
comprehensible without it.

Otherwise,
Reviewed-By: Christopher James Halse Rogers
<christopher.halse.rogers at canonical.com>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: This is a digitally signed message part
URL: <http://lists.x.org/archives/xorg-devel/attachments/20110204/68a91017/attachment.pgp>