On 19.10.22 12:43, Oleksii Moisieiev wrote:
Greetings. I need your advise about the problem I'm facing right now: I'm working on the new type of dmabuf export implementation. This is going to be new ioctl to the gntdev and it's purpose is to use external buffer, provided by file descriptor as the backing storage during export to grant refs. Few words about the functionality I'm working on right now: My setup is based on IMX8QM (please see PS below if you need configuration details) When using dma-buf exporter to create dma-buf with backing storage and map it to the grant refs, provided from the domain, we've met a problem, that several HW (i.MX8 gpu in our case) do not support external buffer and requires backing storage to be created using it's native tools (eglCreateImageKHR returns EGL_NO_IMAGE_KHR for buffers, which were not created using gbm_bo_create). That's why new ioctls were added to be able to pass existing dma-buffer fd as input parameter and use it as backing storage to export to refs. Kernel version on IMX8QM board is 5.10.72 and itworks fine on this kernel version. New ioctls source code can be found here: https://github.com/oleksiimoisieiev/linux/tree/gntdev_map_buf_upstr_for-linus-6.1_2Now regarding the problem I've met when rebased those code on master:On my test stand I use DRM_IOCTL_MODE_CREATE_DUMB/DRM_IOCTL_MODE_DESTROY_DUMB ioctls to allocate buffer and I'm observing the following backtrace on DRM_IOCTL_MODE_DESTROY_DUMB: Unable to handle kernel paging request at virtual address 0000000387000098 Mem abort info: ESR = 0x0000000096000005 EC = 0x25: DABT (current EL), IL = 32 bits SET = 0, FnV = 0 EA = 0, S1PTW = 0 FSC = 0x05: level 1 translation fault Data abort info: ISV = 0, ISS = 0x00000005 CM = 0, WnR = 0 user pgtable: 4k pages, 48-bit VAs, pgdp=000000006df98000 [0000000387000098] pgd=0800000064f4f003, p4d=0800000064f4f003, pud=0000000000000000 Internal error: Oops: 96000005 [#1] PREEMPT SMP Modules linked in: xen_pciback overlay crct10dif_ce ip_tables x_tables ipv6 PU: 0 PID: 34 Comm: kworker/0:1 Not tainted 6.0.0 #85 Hardware name: linux,dummy-virt (DT) Workqueue: events virtio_gpu_dequeue_ctrl_func pstate: 000000c5 (nzcv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--) pc : check_move_unevictable_folios+0xb8/0x4d0 lr : check_move_unevictable_folios+0xb4/0x4d0 sp : ffff8000081a3ad0 x29: ffff8000081a3ad0 x28: ffff03856ac98800 x27: 0000000000000000 x26: ffffde7b168ee9d8 x25: ffff03856ae26008 x24: 0000000000000000 x23: ffffde7b1758d6c0 x22: 0000000000000001 x21: ffff8000081a3b68 x20: 0000000000000001 x19: fffffc0e15935040 x18: ffffffffffffffff x17: ffff250a68e3d000 x16: 0000000000000012 x15: ffff8000881a38d7 x14: 0000000000000000 x13: ffffde7b175a3150 x12: 0000000000002c55 x11: 0000000000000ec7 x10: ffffde7b176113f8 x9 : ffffde7b175a3150 x8 : 0000000100004ec7 x7 : ffffde7b175fb150 x6 : ffff8000081a3b70 x5 : 0000000000000001 x4 : 0000000000000000 x3 : ffff03856ac98850 x2 : 0000000000000000 x1 : 0000000000000000 x0 : 0000000387000000 Call trace: check_move_unevictable_folios+0xb8/0x4d0 check_move_unevictable_pages+0x8c/0x110 drm_gem_put_pages+0x118/0x198 drm_gem_shmem_put_pages_locked+0x4c/0x70 drm_gem_shmem_unpin+0x30/0x50 virtio_gpu_cleanup_object+0x84/0x130 virtio_gpu_cmd_unref_cb+0x18/0x2c virtio_gpu_dequeue_ctrl_func+0x124/0x290 process_one_work+0x1d0/0x320 worker_thread+0x14c/0x444 kthread+0x10c/0x110 ret_from_fork+0x10/0x20 Code: 97fc3fe1 aa1303e0 94003ac7 b4000080 (f9404c00) ---[ end trace 0000000000000000 ]--- After some investigation I think I've found the cause of the problem: This is the functionality, added in commit 3f9f1c67572f5e5e6dc84216d48d1480f3c4fcf6 which introduces safe mechanism to unmap grant pages which is waiting until page_count(page) = 1 before doing unmap. The problem is that DRM_IOCTL_MODE_CREATE_DUMB creates buffer where page_count(page) = 2. On my QEMU test stand I'm using Xen 4.16 (aarch64) with debian based Dom0 + DomU on the latest kernels. I've created some apps for testing: The first one is to allocate grant refs on DomU: https://github.com/oleksiimoisieiev/linux/tree/gntdev_map_buf_upstr_for-linus-6.1_2 The name is test.ko and it can be built using command: cd ./test; make NOTE: makefile expects kernel build to be present on ../../test-build directory. It should be run on DomU using command: insmod test.ko; echo "create" > /sys/kernel/etx_sysfs/etx_value Result will be the following: [ 126.104903] test: loading out-of-tree module taints kernel. [ 126.150586] Sysfs - Write!!! [ 126.150773] create [ 126.150773] [ 126.150888] Hello, World! [ 126.151203] Hello, World! [ 126.151324] gref 301 [ 126.151376] addr ffff00000883d000 [ 126.151431] gref 302 [ 126.151454] addr ffff00000883e000 [ 126.151478] gref 303 [ 126.151497] addr ffff00000883f000 [ 126.151525] gref 304 [ 126.151546] addr ffff000008840000 [ 126.151573] gref 305 [ 126.151593] addr ffff000008841000 The second is for dom0 and can be found here: https://github.com/oleksiimoisieiev/xen/tree/gntdev_fd How to build: make -C tools/console all Result: ./tools/console/gnt_test should be uploaded to Dom0 Start: sudo ./gnt_test_map 1 301 302 303 304 305 Where 1 is DomU ID and 301 302 303 304 305 - grefs from test.ko output This will create buffer using ioctls DRM_IOCTL_MODE_CREATE_DUMB them passes it as backing storage to gntdev and then destroys it using DRM_IOCTL_MODE_DESTROY_DUMB. The problem is that when dumb buffer is created we observe page_count(page) = 2. So when before buffer release I'm trying to unmap grant refs using unmap_grant_pages it is calling __gnttab_unmap_refs_async, which postpones actual unmapping to 5 ms because page_count(page) > 1. Which causes drm_gem_get_pages to try to free pages, which are still mapped. Also if I change in the following line: https://github.com/torvalds/linux/blob/bb1a1146467ad812bb65440696df0782e2bc63c8/drivers/xen/grant-table.c#L1313 change from page_count(item->pages[pc]) > 1 to page_count(item->pages[pc]) > 2 - everything works fine. The obvious way for fix this issue I see is to make the expected page_count for __gnttab_unmap_refs_async configurable for each buffer, but I'm now sure if this is the best solution. I would be happy to hear your thoughts and advises about how to fix this situation.
My first thought would be to save the page_count() of each page when doing the map operation, and then compare to that value. The natural place to store this count would be struct xen_page_foreign, but there are only 16 bits free for the 64-bit system case (it is using the struct page->private field for that purpose), so you'd need to bail out in case page_count() is > 65535. Juergen
Attachment:
OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature
