[Spice-devel] [PATCH] repeatable xorg qxl crash inside qxl_image_create()

[David Mansfield]

On 05/13/2014 04:01 PM, David Mansfield wrote:
> Hi All:
>
> I'd like help tracking down the solution to a very repeatable xorg 
> crash.  I can cause the crash by either:
>
> 1) logging in and opening the second monitor without having removed 
> ~/.config/monitors.xml first.
>
> 2) using "shutter" (screen capture utility) to capture a "selection"
>
> I have a few coredumps with what seem like valid backtraces.  (see 
> below for an example).
>
> The xorg crash is caused by an assert is happening in frame#4, 
> function qxl_bo_output_bo_reloc():
>
>     if (qxl->cmds.n_reloc_bos >= MAX_RELOCS || qxl->cmds.n_relocs >= 
> MAX_RELOCS)
>       assert(0);
>
> According to GDB, both n_reloc_bos = 96 and n_relocs = 96. 
> (MAX_RELOCS=96).
>
> It seems that the "loop" in qxl_image_create (qxl_image.c) is 
> "chunking" the the create into pieces no bigger than "chunk_size" and 
> that this size is not big enough (or MAX_RELOCS is too small).
>
> From GDB (inside qxl_image_create() on or about line 174):
>
> (gdb) print h
> $13 = 31
> (gdb) print height
> $14 = 847
> (gdb) print chunk_size
> $15 = 262144
> (gdb) print n_lines
> $16 = 17
>
> So we have 31 lines left to go (out of 847), and we're copying 17 
> lines at a time.  Close but no cigar.
>
> Is the fix to increase the chunk_size or to increase MAX_RELOCS or is 
> something else broken here?
>

I decided to take a whack at this.  The attached patch dynamically 
increases the chunk_size based on an estimate of the number of chunks 
that will be needed.  I first tried changing MAX(512 * 512, 
dest_stride)  to MAX(5120 * 512, dest_stride), and that worked, but 
seemed to cause some increased latency and perhaps causing some audio 
dropouts, but that could be my imagination.

Anyway, this version of the patch only changes current behavior when an 
"assert()" is imminent anyway so if audio breaks up then so-be-it.

With this patch I see in Xorg.0.log:

[    49.444] adjusted chunk_size to 291840
[    49.450] adjusted chunk_size to 322560
[    50.087] adjusted chunk_size to 506880
[   122.838] adjusted chunk_size to 309260
[   122.849] adjusted chunk_size to 309260
[   122.926] adjusted chunk_size to 309260
[   122.939] adjusted chunk_size to 309260
[   124.006] adjusted chunk_size to 296100

So it may be a bit conservative (the target number of chunks is 
MAX_RELOCS / 4 because each iteration creates 2 entries, and we need 
some left over slots when we're done, as far as I can see).

And the crash scenarios which were 100% reproducible are fixed :-)

-- 
Thanks,
David Mansfield
Cobite, INC.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: qxl_image_create_bigger_chunks.patch
Type: text/x-patch
Size: 1184 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20140514/1071099a/attachment-0001.bin>