Re: [PATCH 0/2] stability for dual head

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 06/09/2014 09:29 AM, Alon Levy wrote:
On 06/09/2014 04:18 PM, David Mansfield wrote:
On 06/09/2014 07:18 AM, Alon Levy wrote:
On 06/03/2014 04:14 PM, David Mansfield wrote:
Bump. I'll make it easy.  This is a multiple choice response form.
Anyone reading this can respond with one letter so save time and effort.

a) "We're too busy with RHEL 7/paying clients, come back in a month/some
timeframe"
b) "There's an SEP field on these problems, everyone who understands
that code has moved on"
c) "Go away"
d) "Oops, I've been meaning to get back to you but I keep forgetting and
life is hectic..."
e) "Didn't you hear? SPICE is dead."
f) "Other." Please elaborate using the space provided below:
The first patch looks good (just adjusting the #if to disable the
print). I'll pick it up, thanks, you deserved a faster response.

No idea what SEP is.
Hi Alon,

I followed Marc-André's advice and sent these out to DRI ond xorg
mailing lists, respectively.  The qxl.ko patch was picked up by Dave
Airlie and committed to drm-next branch.

The second is still without a home.

(BTW: An SEP is a "somebody else's problem" effect, see
http://en.wikipedia.org/wiki/Somebody_Else%27s_Problem, popularized in
Douglas Adams' Hitchhiker's Guide novel.  Very funny concept.)
Missed that.

Any possibility of help with issue #2, the xorg-devel list is silent on
this  one and I don't know who the maintainer is specifically. Without
this patch xorg-qxl is trivially crashable when using dual head at
1920x1200 resolution (or potentially lower resolution).

96 relocs with 512x512 chunks - what do you do to crash it?

Soren Sandmann is the maintainer, but I did a release once, I can commit
it once I'm done testing (need to allow large resolutions which by
default are limited to surface0_area_size).

Good morning (evening?) Alon,

Anything else you need from me on this? I've attached the patch from git-format-patch that should have all of the correct signed-off etc. and a proper commit description.

Just a friendly ping.

--
Thanks,
David Mansfield,
Cobite, INC.


>From c68706dcd3c868d9baff2461413cadf6201eee8a Mon Sep 17 00:00:00 2001
From: David Mansfield <spice@xxxxxxxxxxxxx>
Date: Tue, 3 Jun 2014 10:05:42 -0400
Subject: [PATCH qxl] Dynamically adjust chunk size to avoid command buffer
 overflow.

The maximum number of "commands" that can be queued at once is
fixed at compile time at MAX_RELOCS. However, during the creation
of an image object in qxl_image_create(), the image is split into
commands of maximum size 512*512. For a large dual-head system,
it is easy to create an image for which the number of chunks will
result in an overflow of MAX_RELOCS number of "commands".

Identify this scenario and dynamically increase the chunk size to
avoid the overflow, and the resulting assert() which crashes Xorg.

Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=79317
Signed-off-by: David Mansfield <spice@xxxxxxxxxxxxx>
---
 src/qxl_image.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/src/qxl_image.c b/src/qxl_image.c
index 0a0ca30..396aa0f 100644
--- a/src/qxl_image.c
+++ b/src/qxl_image.c
@@ -140,6 +140,7 @@ qxl_image_create (qxl_screen_t *qxl, const uint8_t *data,
 	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 @@ qxl_image_create (qxl_screen_t *qxl, const uint8_t *data,
 
 	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 0
+		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");
 
-- 
1.9.0


_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel

[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]