[Mesa-dev] questions about assertion hit in vmwgfx/svga driver

[Brian Paul]

On 10/11/2012 03:00 PM, Chris Fester wrote:
> On Wed, Oct 10, 2012 at 5:14 PM, Brian Paul<brianp at vmware.com>  wrote:
>> On 10/10/2012 04:03 PM, Chris Fester wrote:
>>>
>>> Hi all,
>>>
>>> I'm working on a project involving virtualizing a rather old Linux
>>> distribution with 3D support.  Some of the details of the setup:
>>>
>>> Host: Ubuntu 12.04 AMD64, fglrx-updates package installed
>>> -- Vmware guest: Ubuntu 12.04 32 bit, stock packages
>>> ---- chrooted sandbox environment: Fedora Core 2 with gcc 3.3.3 (many
>>> modifications)
>>>
>>> The chrooted environment properly mounts up /proc, /dev/shm, /dev/pts,
>>> /sys, etc.  It also bind mounts /tmp so we get the socket for X11
>>> communication, as well as /dev/dri.
>>>
>>> It has been a long process getting all the dependencies to compile,
>>> writing a little assembly to mimic compiler built-ins, using
>>> libatomicops instead of compiler built-ins, and finally getting mesa
>>> to build.  But I am now able to run glxgears from within the chrooted
>>> sandbox at about 100fps.  Also glxinfo reports that I do have direct
>>> rendering.  Woohoo!
>>>
>>> All of the source that I have built has been obtained with apt-get
>>> source<ubuntu package name>.  I wanted to be sure that everything
>>> being compiled in the chroot was exactly the same version as what I
>>> was running in the guest OS.  Specifically, the Ubuntu version of the
>>> mesa source package: mesa_8.0.3+8.0.2-0ubuntu3.2.
>>>
>>> So the down side... I have hit an assertion in
>>> src/gallium/drivers/svga/svga_resource_buffer_upload.c while running
>>> my company's GL application.  The assertion details:
>>>
>>> src/gallium/drivers/svga/svga_resource_buffer_upload.c:591
>>> assert(!sbuf->head.prev&&   !sbuf->head.next);
>>>
>>>
>>> The prev and next pointers happen to be the same value.  They're also
>>> identical to the prev and next pointers in svga->dirty_buffers.  And
>>> topping it all off, the pointers all point to the location of
>>> &svga->dirty_buffers.
>>>
>>> Below I've pasted more details from GDB.  My question is regarding the
>>> linked list handling.  I've noticed in the Linux doubly-linked-list
>>> implementation, the following happens on a deletion (from
>>> /usr/include/linux/list.h):
>>>
>>> static __inline__ void list_del(struct list_head *entry)
>>> {
>>>           __list_del(entry->prev, entry->next);
>>>           entry->next = entry->prev = 0;  //<------------------- set to 0!
>>> }
>>>
>>> And the code from src/gallium/auxiliary/util/u_double_list.h:
>>>
>>> static INLINE void list_del(struct list_head *item)
>>> {
>>>       item->prev->next = item->next;
>>>       item->next->prev = item->prev;
>>> }
>>>
>>> Note that next and prev are not zero-ed out here.
>>>
>>> I also noticed this bit of code:
>>> svga_resource_buffer_upload.c:290 (in svga_buffer_upload_flush()):
>>>      LIST_DEL(&sbuf->head);
>>> #ifdef DEBUG
>>>      sbuf->head.next = sbuf->head.prev = NULL;
>>> #endif
>>>
>>> Why the #ifdef DEBUG here?  Should the setting of next and prev to
>>> NULL take place all the time?  Or should they be set to NULL in the
>>> u_double_list.h header's list_del()?
>>>
>>> I realize that my setup is pretty unique and this assertion may be my
>>> responsibility.  If the svga/list code as it stands is correct, then
>>> I'd appreciate any advice on what may be incorrect in my setup  that
>>> could cause the error.  If I remove the assert, the application
>>> continues on happily, although I need to give it more soak time.
>>>
>>> Thanks for your efforts and your time!
>>> Chris Fester
>>>
>
> (snipping out the GDB info)
>
>>
>> You might want to try the latest Mesa sources from git.  It should be easy
>> to build/install in your guest if you've got the other pieces in place.
>>
>> Otherwise, if your app is proprietary, maybe you could make an apitrace of
>> the GL calls and send it to me for testing.
>>
>> https://github.com/apitrace/apitrace
>>
>> -Brian
>
> Hi Brian,
>
> I was able to build the latest git sources.  I hit the same assertion
> with the list pointers.  The patch below fixes it for me.  Let me know
> what you think.  If I get some time I'll attempt to build/run apitrace
> and get you a trace as well.  That will take some time as I will need
> to remove some of my company's artwork/images.
>
> Chris
>
> diff --git a/src/gallium/auxiliary/util/u_double_list.h b/src/gallium/auxiliary/
> index 9d1129b..408c26d 100644
> --- a/src/gallium/auxiliary/util/u_double_list.h
> +++ b/src/gallium/auxiliary/util/u_double_list.h
> @@ -82,6 +82,7 @@ static INLINE void list_del(struct list_head *item)
>   {
>       item->prev->next = item->next;
>       item->next->prev = item->prev;
> +    item->prev = item->next = NULL;
>   }
>
>   static INLINE void list_delinit(struct list_head *item)
>
>

This looks OK, and I'll commit the change.  The #ifdef DEBUG code you 
pointed out above should probably go away then.  I'll do that too and 
run some tests before committing.  Thanks!

-Brian