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.
diff -r -u xf86-video-qxl-0.1.1.orig/src/qxl_image.c xf86-video-qxl-0.1.1/src/qxl_image.c
--- xf86-video-qxl-0.1.1.orig/src/qxl_image.c 2013-10-20 10:53:29.000000000 -0400
+++ xf86-video-qxl-0.1.1/src/qxl_image.c 2014-05-14 09:10:43.435444196 -0400
@@ -140,6 +140,7 @@
struct qxl_bo *image_bo;
int dest_stride = (width * Bpp + 3) & (~3);
int h;
+ int chunk_size;
data += y * stride + x * Bpp;
@@ -155,9 +156,23 @@
hash = 0;
h = height;
+
+ chunk_size = MAX (512 * 512, dest_stride);
+
+ /* ensure we will not create too many pieces and overflow
+ * the command buffer (MAX_RELOCS). if so, increase the chunk_size.
+ * each loop creates at least 2 cmd buffer entries, and
+ * we have to leave room when we're done.
+ */
+ if (height / (chunk_size / dest_stride) > (MAX_RELOCS / 4)) {
+ chunk_size = height / (MAX_RELOCS/4) * dest_stride;
+#if 1
+ ErrorF ("adjusted chunk_size to %d\n", chunk_size);
+#endif
+ }
+
while (h)
{
- int chunk_size = MAX (512 * 512, dest_stride);
int n_lines = MIN ((chunk_size / dest_stride), h);
struct qxl_bo *bo = qxl->bo_funcs->bo_alloc (qxl, sizeof (QXLDataChunk) + n_lines * dest_stride, "image data");
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel