mattmacy
commented
8 years ago This looks like a double free on a page. It appears that sometimes we do want a hold count > 1.
Closed dumbbell closed 7 years ago
mattmacy
commented
8 years ago This looks like a double free on a page. It appears that sometimes we do want a hold count > 1.
dumbbell
commented
8 years ago FYI, here is another very similar panic which happens more frequently than the previously reported one:
#1 doadump (textdump=0)
at /mnt/home/dumbbell/Projects/freebsd/src/GIT/sys/kern/kern_shutdown.c:298
#2 0xffffffff829f8719 in vt_kms_postswitch (arg=<optimized out>)
at /mnt/home/dumbbell/Projects/freebsd/src/GIT/sys/modules/drm/drm/../../../dev/drm/linux_fb.c:82
#3 0xffffffff808e252b in vt_window_switch (
vw=0xffffffff8172b410 <vt_conswindow>)
at /mnt/home/dumbbell/Projects/freebsd/src/GIT/sys/dev/vt/vt_core.c:540
#4 0xffffffff808dff90 in vtterm_cngrab (tm=<optimized out>)
at /mnt/home/dumbbell/Projects/freebsd/src/GIT/sys/dev/vt/vt_core.c:1465
#5 0xffffffff809f9812 in cngrab ()
at /mnt/home/dumbbell/Projects/freebsd/src/GIT/sys/kern/kern_cons.c:368
#6 0xffffffff80a53e26 in vpanic (
fmt=0xffffffff8297ebc6 "Assertion %s failed at %s:%d",
ap=0xfffffe04551f23a0)
at /mnt/home/dumbbell/Projects/freebsd/src/GIT/sys/kern/kern_shutdown.c:745
#7 0xffffffff80a53d16 in kassert_panic (
fmt=0xffffffff8297ebc6 "Assertion %s failed at %s:%d")
at /mnt/home/dumbbell/Projects/freebsd/src/GIT/sys/kern/kern_shutdown.c:649
#8 0xffffffff828b71a4 in __free_hot_cold_page (vm_page=0xfffff8043035b6e8)
at /mnt/home/dumbbell/Projects/freebsd/src/GIT/sys/modules/drm/i915/i915kms/../../../../compat/linuxkpi/common/include/linux/gfp.h:102
#9 0xffffffff828b6189 in release_pages (cold=false, pages=<optimized out>,
nr=<optimized out>)
at /mnt/home/dumbbell/Projects/freebsd/src/GIT/sys/modules/drm/i915/i915kms/../../../../compat/linuxkpi/common/include/linux/pagemap.h:84
#10 i915_gem_userptr_get_pages (obj=<optimized out>)
at /mnt/home/dumbbell/Projects/freebsd/src/GIT/sys/modules/drm/i915/i915kms/../../../../dev/drm/i915/i915_gem_userptr.c:700
#11 0xffffffff82896d68 in i915_gem_object_get_pages (obj=0xfffff801c726ad80)
at /mnt/home/dumbbell/Projects/freebsd/src/GIT/sys/modules/drm/i915/i915kms/../../../../dev/drm/i915/i915_gem.c:2451
#12 0xffffffff828997ac in i915_gem_object_set_to_gtt_domain (
obj=0xfffff801c726ad80, write=false)
at /mnt/home/dumbbell/Projects/freebsd/src/GIT/sys/modules/drm/i915/i915kms/../../../../dev/drm/i915/i915_gem.c:3854
#13 0xffffffff8289939f in i915_gem_set_domain_ioctl (dev=<optimized out>,
data=<optimized out>, linux_file=<optimized out>)
at /mnt/home/dumbbell/Projects/freebsd/src/GIT/sys/modules/drm/i915/i915kms/../../../../dev/drm/i915/i915_gem.c:1632
#14 0xffffffff829e6341 in drm_ioctl (filp=<optimized out>, cmd=2148295775,
arg=<optimized out>)
at /mnt/home/dumbbell/Projects/freebsd/src/GIT/sys/modules/drm/drm/../../../dev/drm/drm_ioctl.c:714
#15 0xffffffff82842fba in linux_dev_ioctl (dev=<optimized out>,
cmd=2148295775, data=<unavailable>, fflag=<optimized out>,
td=<unavailable>)
at /mnt/home/dumbbell/Projects/freebsd/src/GIT/sys/modules/linuxkpi/../../compat/linuxkpi/common/src/linux_compat.c:927
#16 0xffffffff80928673 in devfs_ioctl (ap=0xfffffe04551f2770)
at /mnt/home/dumbbell/Projects/freebsd/src/GIT/sys/fs/devfs/devfs_vnops.c:831
#17 0xffffffff80fdc7a0 in VOP_IOCTL_APV (vop=<optimized out>,
a=<optimized out>) at vnode_if.c:1067
#18 0xffffffff80b2754c in VOP_IOCTL (vp=0xfffff80184608588,
command=<optimized out>, data=0xfffffe04551f2880, fflag=<unavailable>,
cred=0xfffff8004312c500, td=<unavailable>) at ./vnode_if.h:448
#19 vn_ioctl (fp=0xfffff80011a24af0, com=<optimized out>,
data=0xfffffe04551f2880, active_cred=0xfffff8004312c500, td=<unavailable>)
at /mnt/home/dumbbell/Projects/freebsd/src/GIT/sys/kern/vfs_vnops.c:1498
#20 0xffffffff80928d8f in devfs_ioctl_f (fp=<unavailable>, com=<unavailable>,
data=<unavailable>, cred=<unavailable>, td=0xfffff80184ed4510)
at /mnt/home/dumbbell/Projects/freebsd/src/GIT/sys/fs/devfs/devfs_vnops.c:789
#21 0xffffffff80ab9f96 in fo_ioctl (fp=0xfffff80011a24af0, com=<unavailable>,
data=<unavailable>, active_cred=<optimized out>, td=<optimized out>)
at /mnt/home/dumbbell/Projects/freebsd/src/GIT/sys/sys/file.h:328
#22 kern_ioctl (td=<optimized out>, fd=<optimized out>, com=<optimized out>,
data=<optimized out>)
at /mnt/home/dumbbell/Projects/freebsd/src/GIT/sys/kern/sys_generic.c:839
#23 0xffffffff80ab9ce1 in sys_ioctl (td=<optimized out>,
uap=0xfffffe04551f29d0)
at /mnt/home/dumbbell/Projects/freebsd/src/GIT/sys/kern/sys_generic.c:748
#24 0xffffffff80ecddd4 in syscallenter (td=0xfffff80184ed4510,
sa=<optimized out>)
at /mnt/home/dumbbell/Projects/freebsd/src/GIT/sys/amd64/amd64/../../kern/subr_syscall.c:135
#25 amd64_syscall (td=0xfffff80184ed4510, traced=0)
at /mnt/home/dumbbell/Projects/freebsd/src/GIT/sys/amd64/amd64/trap.c:938
#26 <signal handler called>I created the patch mentionned above to support hold counts greater than one. For now, the laptop survived more than a minute (I'm even writing this comment using it), so I'm hopeful. I will watch for page leaks, just in case.
dumbbell
commented
8 years ago Hi!
So the patch mentionned above fixes the immediate panic, but there is a larger issue causing memory leaks and perhaps runtime problems.
In i915_gem_release_mmap(), the driver indirectly calls unmap_mapping_range() to unmap all mappings in a given range in all processes having one (same in TTM). unmap_mapping_range() is given ...->anon_inode->i_mapping which, in Linux, is a struct address_space holding all pages and all mappings (all struct vm_area_structs). This struct address_space is shared by all character devices of a given device (ie. /dev/dri/card0, /dev/dri/controlD64 and /dev/dri/renderD128 all share the same list of pages and mappings, so that all processes see the same content).
In FreeBSD, anon_inode->i_mapping is never initialized (see alloc_anon_inode()). Therefore, unmap_mapping_range() is given a NULL pointer and is basically a no-op: no vm_object/pager is deallocated and the pager's dtor is never called. This leads a leak of all mapped data.
Even if the pointer was correct, unmap_mapping_range() assumes the given pointer is the handle passed to cdev_pager_allocate(). This is not the case: we use the struct vm_area_struct pointer there.
Regarding the sharing of pages, I didn't look at the code of the fault handler yet, but I believe (though not 100% sure) it's correct because the pages are provided by the GEM object. So even if, unlike Linux, FreeBSD doesn't share a common list of pages (pages in the central shared struct address_space in Linux vs. pages attached to each vm_object in FreeBSD), they point to the same physical address. Yet, I'm wondering if there could be issues because the list of pages are private to each mapping.
I didn't look yet at the other ways to map and share graphics buffers, but they are probably affected by the "incompatible" implementation of struct address_space in FreeBSD.
Now, I'm working on the possible solutions, but I don't know yet how to fix the whole problem. We could implement the real struct address_space, but I have the feeling we would duplicate a lot of the job of the VM.
Do you have a suggestion to do the global unmap (ie. accross all mappings in that range from all character devices in all processes) without maintaining our own accounting of mappings in linuxkpi?
mattmacy
commented
7 years ago @markjdb any comment?
Panic hit while playing a video using Flash (?) with SNA as the acceleration method.