On 2017-06-26 15:51, Marc Dietrich wrote:
Am Montag, 26. Juni 2017, 15:35:15 CEST schrieb Grigori Goronzy:
On 2017-06-26 15:11, Marc Dietrich wrote:
> unfortunately, this change broke vmware/vmplayer here (bisected).
> Windows
> guest on linux host. Sig 11 in SVGA driver. All good if
> mesa_glthread=false.
Can you provide instructions how to reproduce this problem? A
backtrace
might help, too.
well, this is all proprietary software, so the backtrace doesn't really
tell
something.
I don't really get it, by the way. Isn't the SVGA driver for Linux
guests?
I think the windows driver is named the same. Here is a paste of
vmware.log:
https://pastebin.com/X3CS7rCP
I also have core dump, maybe only useful for VMWARE staff...
Hey Marc,
does the attached patch fix the crash?
Grigori
Best regards
Grigori
>> > Best regards
>> > Grigori
>> >
>> >> [1]
>> >> https://lists.freedesktop.org/archives/mesa-dev/2017-June/160329.html
>> >>
>> >> On 25/06/17 02:59, Grigori Goronzy wrote:
>> >>> These entry points are used by Alien Isolation and caused
>> >>> synchronization with glthread. The async marshalling implementation
>> >>> is similar to glBuffer(Sub)Data.
>> >>>
>> >>> Results in an approximately 6x drop in glthread synchronizations and
>> >>> a
>> >>> ~30% FPS jump in Alien Isolation (Medium preset, Athlon 860K, RX
>> >>> 480).
>> >>>
>> >>> This does not care about the EXTERNAL_VIRTUAL_MEMORY_BUFFER_AMD
>> >>> special
>> >>> case like the Buffer(Sub)Data marshalling functions.
>> >>> ---
>> >>> I'm not a fan of the code duplication and I'll try to address that in
>> >>> further changes to glthread/marshalling, but the improvement is so
>> >>> noticeable that I'd like to share it. Alien Isolation is now
>> >>> playable on
>> >>> my system while it wasn't before.
>> >>>
>> >>> src/mapi/glapi/gen/ARB_direct_state_access.xml | 4 +-
>> >>> src/mesa/main/marshal.c | 108
>> >>>
>> >>> +++++++++++++++++++++++++
>> >>>
>> >>> src/mesa/main/marshal.h | 18 +++++
>> >>> 3 files changed, 128 insertions(+), 2 deletions(-)
>> >>>
>> >>> diff --git a/src/mapi/glapi/gen/ARB_direct_state_access.xml
>> >>> b/src/mapi/glapi/gen/ARB_direct_state_access.xml
>> >>> index cb24d79..d3d2246 100644
>> >>> --- a/src/mapi/glapi/gen/ARB_direct_state_access.xml
>> >>> +++ b/src/mapi/glapi/gen/ARB_direct_state_access.xml
>> >>> @@ -61,14 +61,14 @@
>> >>>
>> >>> <param name="flags" type="GLbitfield" />
>> >>>
>> >>> </function>
>> >>>
>> >>> - <function name="NamedBufferData">
>> >>>
>> >>> + <function name="NamedBufferData" marshal="custom">
>> >>>
>> >>> <param name="buffer" type="GLuint" />
>> >>> <param name="size" type="GLsizeiptr" />
>> >>> <param name="data" type="const GLvoid *" />
>> >>> <param name="usage" type="GLenum" />
>> >>>
>> >>> </function>
>> >>>
>> >>> - <function name="NamedBufferSubData" no_error="true">
>> >>>
>> >>> + <function name="NamedBufferSubData" no_error="true"
>> >>> marshal="custom">
>> >>>
>> >>> <param name="buffer" type="GLuint" />
>> >>> <param name="offset" type="GLintptr" />
>> >>> <param name="size" type="GLsizeiptr" />
>> >>>
>> >>> diff --git a/src/mesa/main/marshal.c b/src/mesa/main/marshal.c
>> >>> index 4840f32..1fddf8e 100644
>> >>> --- a/src/mesa/main/marshal.c
>> >>> +++ b/src/mesa/main/marshal.c
>> >>> @@ -408,6 +408,114 @@ _mesa_marshal_BufferSubData(GLenum target,
>> >>> GLintptr offset, GLsizeiptr size,
>> >>>
>> >>> }
>> >>>
>> >>> }
>> >>> +/* NamedBufferData: marshalled asynchronously */
>> >>>
>> >>> +struct marshal_cmd_NamedBufferData
>> >>> +{
>> >>> + struct marshal_cmd_base cmd_base;
>> >>> + GLuint name;
>> >>> + GLsizei size;
>> >>> + GLenum usage;
>> >>> + /* Next size bytes are GLubyte data[size] */
>> >>> +};
>> >>> +
>> >>> +void
>> >>> +_mesa_unmarshal_NamedBufferData(struct gl_context *ctx,
>> >>> + const struct
>> >>> marshal_cmd_NamedBufferData *cmd)
>> >>> +{
>> >>> + const GLuint name = cmd->name;
>> >>> + const GLsizei size = cmd->size;
>> >>> + const GLenum usage = cmd->usage;
>> >>> + const void *data = (const void *) (cmd + 1);
>> >>> +
>> >>> + CALL_NamedBufferData(ctx->CurrentServerDispatch,
>> >>> + (name, size, data, usage));
>> >>> +}
>> >>> +
>> >>> +void GLAPIENTRY
>> >>> +_mesa_marshal_NamedBufferData(GLuint buffer, GLsizeiptr size,
>> >>> + const GLvoid * data, GLenum usage)
>> >>> +{
>> >>> + GET_CURRENT_CONTEXT(ctx);
>> >>> + size_t cmd_size = sizeof(struct marshal_cmd_NamedBufferData) +
>> >>> size;
>> >>> +
>> >>> + debug_print_marshal("NamedBufferData");
>> >>> + if (unlikely(size < 0)) {
>> >>> + _mesa_glthread_finish(ctx);
>> >>> + _mesa_error(ctx, GL_INVALID_VALUE, "NamedBufferData(size <
>> >>> 0)");
>> >>> + return;
>> >>> + }
>> >>> +
>> >>> + if (buffer > 0 && cmd_size <= MARSHAL_MAX_CMD_SIZE) {
>> >>> + struct marshal_cmd_NamedBufferData *cmd =
>> >>> + _mesa_glthread_allocate_command(ctx,
>> >>> DISPATCH_CMD_NamedBufferData,
>> >>> + cmd_size);
>> >>> + cmd->name = buffer;
>> >>> + cmd->size = size;
>> >>> + cmd->usage = usage;
>> >>> + char *variable_data = (char *) (cmd + 1);
>> >>> + memcpy(variable_data, data, size);
>> >>> + _mesa_post_marshal_hook(ctx);
>> >>> + } else {
>> >>> + _mesa_glthread_finish(ctx);
>> >>> + CALL_NamedBufferData(ctx->CurrentServerDispatch,
>> >>> + (buffer, size, data, usage));
>> >>> + }
>> >>> +}
>> >>> +
>> >>> +/* NamedBufferSubData: marshalled asynchronously */
>> >>> +struct marshal_cmd_NamedBufferSubData
>> >>> +{
>> >>> + struct marshal_cmd_base cmd_base;
>> >>> + GLuint name;
>> >>> + GLintptr offset;
>> >>> + GLsizei size;
>> >>> + /* Next size bytes are GLubyte data[size] */
>> >>> +};
>> >>> +
>> >>> +void
>> >>> +_mesa_unmarshal_NamedBufferSubData(struct gl_context *ctx,
>> >>> + const struct
>> >>> marshal_cmd_NamedBufferSubData *cmd)
>> >>> +{
>> >>> + const GLuint name = cmd->name;
>> >>> + const GLintptr offset = cmd->offset;
>> >>> + const GLsizei size = cmd->size;
>> >>> + const void *data = (const void *) (cmd + 1);
>> >>> +
>> >>> + CALL_NamedBufferSubData(ctx->CurrentServerDispatch,
>> >>> + (name, offset, size, data));
>> >>> +}
>> >>> +
>> >>> +void GLAPIENTRY
>> >>> +_mesa_marshal_NamedBufferSubData(GLuint buffer, GLintptr offset,
>> >>> + GLsizeiptr size, const GLvoid *
>> >>> data)
>> >>> +{
>> >>> + GET_CURRENT_CONTEXT(ctx);
>> >>> + size_t cmd_size = sizeof(struct marshal_cmd_NamedBufferSubData)
>> >>> + size;
>> >>> +
>> >>> + debug_print_marshal("NamedBufferSubData");
>> >>> + if (unlikely(size < 0)) {
>> >>> + _mesa_glthread_finish(ctx);
>> >>> + _mesa_error(ctx, GL_INVALID_VALUE, "NamedBufferSubData(size <
>> >>> 0)");
>> >>> + return;
>> >>> + }
>> >>> +
>> >>> + if (buffer > 0 && cmd_size <= MARSHAL_MAX_CMD_SIZE) {
>> >>> + struct marshal_cmd_NamedBufferSubData *cmd =
>> >>> + _mesa_glthread_allocate_command(ctx,
>> >>> DISPATCH_CMD_NamedBufferSubData,
>> >>> + cmd_size);
>> >>> + cmd->name = buffer;
>> >>> + cmd->offset = offset;
>> >>> + cmd->size = size;
>> >>> + char *variable_data = (char *) (cmd + 1);
>> >>> + memcpy(variable_data, data, size);
>> >>> + _mesa_post_marshal_hook(ctx);
>> >>> + } else {
>> >>> + _mesa_glthread_finish(ctx);
>> >>> + CALL_NamedBufferSubData(ctx->CurrentServerDispatch,
>> >>> + (buffer, offset, size, data));
>> >>> + }
>> >>> +}
>> >>> +
>> >>>
>> >>> /* ClearBufferfv: marshalled asynchronously */
>> >>> struct marshal_cmd_ClearBufferfv
>> >>> {
>> >>>
>> >>> diff --git a/src/mesa/main/marshal.h b/src/mesa/main/marshal.h
>> >>> index ddfa834..999c75e 100644
>> >>> --- a/src/mesa/main/marshal.h
>> >>> +++ b/src/mesa/main/marshal.h
>> >>> @@ -180,6 +180,8 @@ struct marshal_cmd_Flush;
>> >>>
>> >>> struct marshal_cmd_BindBuffer;
>> >>> struct marshal_cmd_BufferData;
>> >>> struct marshal_cmd_BufferSubData;
>> >>>
>> >>> +struct marshal_cmd_NamedBufferData;
>> >>> +struct marshal_cmd_NamedBufferSubData;
>> >>>
>> >>> struct marshal_cmd_ClearBufferfv;
>> >>>
>> >>> void
>> >>>
>> >>> @@ -228,6 +230,22 @@ _mesa_marshal_BufferSubData(GLenum target,
>> >>> GLintptr offset, GLsizeiptr size,
>> >>>
>> >>> const GLvoid * data);
>> >>>
>> >>> void
>> >>>
>> >>> +_mesa_unmarshal_NamedBufferData(struct gl_context *ctx,
>> >>> + const struct
>> >>> marshal_cmd_NamedBufferData *cmd);
>> >>> +
>> >>> +void GLAPIENTRY
>> >>> +_mesa_marshal_NamedBufferData(GLuint buffer, GLsizeiptr size,
>> >>> + const GLvoid * data, GLenum usage);
>> >>> +
>> >>> +void
>> >>> +_mesa_unmarshal_NamedBufferSubData(struct gl_context *ctx,
>> >>> + const struct
>> >>> marshal_cmd_NamedBufferSubData *cmd);
>> >>> +
>> >>> +void GLAPIENTRY
>> >>> +_mesa_marshal_NamedBufferSubData(GLuint buffer, GLintptr offset,
>> >>> GLsizeiptr size,
>> >>> + const GLvoid * data);
>> >>> +
>> >>> +void
>> >>>
>> >>> _mesa_unmarshal_ClearBufferfv(struct gl_context *ctx,
>> >>>
>> >>> const struct
>> >>>
>> >>> marshal_cmd_ClearBufferfv *cmd);
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
From 477b1f7b906382024252c513e32799b9b858a042 Mon Sep 17 00:00:00 2001
From: Grigori Goronzy <g...@chown.ath.cx>
Date: Mon, 10 Jul 2017 01:55:52 +0200
Subject: [PATCH] mesa/marshal: fix glNamedBufferData with NULL data
The semantics are similar to glBufferData.
---
src/mesa/main/marshal.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/src/mesa/main/marshal.c b/src/mesa/main/marshal.c
index 5fc733f..97e3e73 100644
--- a/src/mesa/main/marshal.c
+++ b/src/mesa/main/marshal.c
@@ -415,6 +415,7 @@ struct marshal_cmd_NamedBufferData
GLuint name;
GLsizei size;
GLenum usage;
+ bool data_null; /* If set, no data follows for "data" */
/* Next size bytes are GLubyte data[size] */
};
@@ -425,7 +426,12 @@ _mesa_unmarshal_NamedBufferData(struct gl_context *ctx,
const GLuint name = cmd->name;
const GLsizei size = cmd->size;
const GLenum usage = cmd->usage;
- const void *data = (const void *) (cmd + 1);
+ const void *data;
+
+ if (cmd->data_null)
+ data = NULL;
+ else
+ data = (const void *) (cmd + 1);
CALL_NamedBufferData(ctx->CurrentServerDispatch,
(name, size, data, usage));
@@ -436,7 +442,7 @@ _mesa_marshal_NamedBufferData(GLuint buffer, GLsizeiptr
size,
const GLvoid * data, GLenum usage)
{
GET_CURRENT_CONTEXT(ctx);
- size_t cmd_size = sizeof(struct marshal_cmd_NamedBufferData) + size;
+ size_t cmd_size = sizeof(struct marshal_cmd_NamedBufferData) + (data ? size
: 0);
debug_print_marshal("NamedBufferData");
if (unlikely(size < 0)) {
@@ -452,8 +458,11 @@ _mesa_marshal_NamedBufferData(GLuint buffer, GLsizeiptr
size,
cmd->name = buffer;
cmd->size = size;
cmd->usage = usage;
- char *variable_data = (char *) (cmd + 1);
- memcpy(variable_data, data, size);
+ cmd->data_null = !data;
+ if (data) {
+ char *variable_data = (char *) (cmd + 1);
+ memcpy(variable_data, data, size);
+ }
_mesa_post_marshal_hook(ctx);
} else {
_mesa_glthread_finish(ctx);
--
2.7.4
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
