| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
|
|
whole thing except the header
|
|
|
|
|
|
already removes that amount
|
|
|
|
|
|
|
|
|
|
reset+resize
|
|
Instead of reset flush everything. primary doesn't exist.
TODO - this is not the solution after all, no need to evacuate anything,
merge from later.
|
|
Just bookkeeping - qxl_unmap_memory is called on initialization due to
the need to map twice (see the "hate" comment), and during
qxl_close_screen.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
send/not send destroy message
|
|
|
|
|
|
I don't have a stacktrace to show any segfault unfortunately.
|
|
Both results from ProcFreePixmap being called in unanticipated
circumstances:
cache->all_surfaces is NULL
surface->host_image is NULL
To reproduce the following scripts work, in tandem:
create xterms, destroy them
chvt
============ xterm_test ============
import os
import subprocess
import time
import atexit
env = os.environ
env['DISPLAY'] = ':0.0'
xterms = []
def kill_all():
print "killing xterms"
for x in xterms:
x.kill()
del xterms[:]
atexit.register(kill_all)
while True:
for i in range(10):
xterms.append(subprocess.Popen(['xterm', '+u8']))
time.sleep(1)
kill_all()
============= chvt_test_helper ============
XPID=`pgrep Xorg`
XTTY=`find /proc/$XPID/fd -lname "/dev/tty*"`
XTTY=`readlink $XTTY`
XTTY=${XTTY#/dev/tty}
echo "chvt 1 (from Xorg)"
chvt 1
sleep 2
echo "chvt $XTTY (to Xorg)"
chvt $XTTY
============== chvt_test =================
while true; do ./chvt-test ; sleep 3; done
|
|
In summary, on vt enter we still:
reset
recreate memory slots
clear our mspace allocators
and then do what switch mode below says
On vt leave we still:
reset (this is redundant since the first VGA access will trigger a
reset on the device side)
On switch mode however we only:
destroy primary surface
create primary surface (different size)
|
|
The primary surface, i.e. qxl->primary, the only surface with id==0, is
allocated in qxl_surface_cache_create_primary with prev==next==NULL.
Unlinking it was producing a wrong cache->free_surfaces == NULL. This
was not a problem because unlinking the primary only happened in
switch_host, which then called surface_cache_init. In a following commit
switch_host is simplified to destroy-primary+create-primary, so this bug
needs to be fixed first to avoid leaking surfaces and reaching a no
surface available situation.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
single point)
|
|
|
|
adds preprocessor definitions DEBUG_QXL_MEM & DEBUG_QXL_MEM_VERBOSE
|
|
Prevent access to freed memory when:
1. qxl_leave_vt/qxl_surface_cache_evacuate_all freed cache->all_surfaces
2. ProcRenderDispatch/damageDestroyPixmap/qxl_destroy_pixmap/qxl_surface_kill
access a surface that pointed inside the all_surfaces array
Solution in this patch:
1. never free all_surfaces
2. add an 'evacuated' field per surface, initialied to NULL, set during
evacuation.
3. on qxl_surface_kill, if surface->evacuated is set, don't destroy the
surface (it is already destroyed by this point via a reset in
qxl_surface_cache_evacuate_all's caller, qxl_leave_vt), just unref the
host pixmap, free the evacuated_surface_t and unlink it from the
evacuated linked list, so it isn't recreated later on
qxl_surface_cache_replace_all.
|
|
This is not enough to prevent any qxl_destroy_pixmap call during vt
switch, but it prevents those triggered by CursorDisplayCursor.
Note: a matching xf86_show_cursors call doesn't hurt, but is not
required, so not adding it.
It is still possible to access freed memory by the following trigger:
==4416== Invalid read of size 8
==4416== at 0x5D15EC1: unlink_surface (qxl_surface.c:685)
==4416== by 0x5D162F9: qxl_surface_kill (qxl_surface.c:799)
==4416== by 0x5D12688: qxl_destroy_pixmap (qxl_driver.c:928)
==4416== by 0x55730B: damageDestroyPixmap (damage.c:1556)
==4416== by 0x51C77B: ShmDestroyPixmap (shm.c:273)
==4416== by 0x54591B: FreePicture (picture.c:1465)
==4416== by 0x467A32: doFreeResource (resource.c:873)
==4416== by 0x467B7E: FreeResource (resource.c:903)
==4416== by 0x547742: ProcRenderFreePicture (render.c:661)
==4416== by 0x54B13A: ProcRenderDispatch (render.c:1988)
==4416== by 0x430670: Dispatch (dispatch.c:428)
==4416== by 0x492604: main (main.c:288)
==4416== Address 0x121031e0 is 116,960 bytes inside a block of size 122,880 free'd
==4416== at 0x4A079AE: free (vg_replace_malloc.c:427)
==4416== by 0x5D16BDA: qxl_surface_cache_evacuate_all (qxl_surface.c:1060)
==4416== by 0x5D13078: qxl_leave_vt (qxl_driver.c:1209)
==4416== by 0x4A4D4F: xf86VTSwitch (xf86Events.c:462)
==4416== by 0x4A4926: xf86Wakeup (xf86Events.c:285)
==4416== by 0x43E2E1: WakeupHandler (dixutils.c:421)
==4416== by 0x488A75: WaitForSomething (WaitFor.c:224)
==4416== by 0x4303CF: Dispatch (dispatch.c:357)
==4416== by 0x492604: main (main.c:288)
This is fixed by a following patch to not free all_surfaces, instead
keeping pointers from it to the evacuated list.
|
|
|
|
|
|
|
|
The QXL_IO_NOTIFY_OOM is intended exactly for handling occurrences of
lacking memory. The spice server tries to first release resources that
are no longer in the current tree (and thus, do not need rendering).
It renders drawables only as a last resort. And even then,
it does not update the whole primary surface, but rather renders
the oldest X drawables.
The call to update_area is redundant, and its effect on performance
is noticeable when playing full screen video.
Signed-off-by: Yonit Halperin <yhalperi@redhat.com>
|
|
|
|
At least in Fedora 17, the correct RPM name is xorg-x11-util-macros
|
|
|
|
qxl_surface.c:735:6: warning: declaration of 'i' shadows a previous
local [-Wshadow]
|
