Linaro-mm-sig February 2022

linaro-mm-sig@lists.linaro.org

24 participants
17 discussions

[syzbot] inconsistent lock state in sync_info_debugfs_show

by syzbot

Hello, syzbot found the following issue on: HEAD commit: 1c52283265a4 Merge branch 'akpm' (patches from Andrew) git tree: upstream console output: https://syzkaller.appspot.com/x/log.txt?x=1224663fb00000 kernel config: https://syzkaller.appspot.com/x/.config?x=75bc179af0ff0457 dashboard link: https://syzkaller.appspot.com/bug?extid=007bfe0f3330f6e1e7d1 compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2 Unfortunately, I don't have any reproducer for this issue yet. IMPORTANT: if you fix the issue, please add the following tag to the commit: Reported-by: syzbot+007bfe0f3330f6e1e7d1(a)syzkaller.appspotmail.com ================================ WARNING: inconsistent lock state 5.16.0-syzkaller #0 Not tainted -------------------------------- inconsistent {IN-HARDIRQ-W} -> {HARDIRQ-ON-W} usage. syz-executor.2/18360 [HC0[0]:SC0[0]:HE0:SE1] takes: ffffffff8c712cf8 (sync_timeline_list_lock){?...}-{2:2}, at: spin_lock_irq include/linux/spinlock.h:374 [inline] ffffffff8c712cf8 (sync_timeline_list_lock){?...}-{2:2}, at: sync_info_debugfs_show+0x2d/0x200 drivers/dma-buf/sync_debug.c:147 {IN-HARDIRQ-W} state was registered at: lock_acquire kernel/locking/lockdep.c:5639 [inline] lock_acquire+0x1ab/0x510 kernel/locking/lockdep.c:5604 __raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline] _raw_spin_lock_irqsave+0x39/0x50 kernel/locking/spinlock.c:162 sync_timeline_debug_remove+0x25/0x190 drivers/dma-buf/sync_debug.c:31 sync_timeline_free drivers/dma-buf/sw_sync.c:104 [inline] kref_put include/linux/kref.h:65 [inline] sync_timeline_put drivers/dma-buf/sw_sync.c:116 [inline] timeline_fence_release+0x263/0x340 drivers/dma-buf/sw_sync.c:144 dma_fence_release+0x2ee/0x590 drivers/dma-buf/dma-fence.c:549 kref_put include/linux/kref.h:65 [inline] dma_fence_put include/linux/dma-fence.h:276 [inline] dma_fence_array_release+0x1e4/0x2b0 drivers/dma-buf/dma-fence-array.c:120 dma_fence_release+0x2ee/0x590 drivers/dma-buf/dma-fence.c:549 kref_put include/linux/kref.h:65 [inline] dma_fence_put include/linux/dma-fence.h:276 [inline] irq_dma_fence_array_work+0xa5/0xd0 drivers/dma-buf/dma-fence-array.c:52 irq_work_single+0x120/0x270 kernel/irq_work.c:211 irq_work_run_list+0x91/0xc0 kernel/irq_work.c:242 irq_work_run+0x54/0xd0 kernel/irq_work.c:251 __sysvec_irq_work+0x95/0x3d0 arch/x86/kernel/irq_work.c:22 sysvec_irq_work+0x8e/0xc0 arch/x86/kernel/irq_work.c:17 asm_sysvec_irq_work+0x12/0x20 arch/x86/include/asm/idtentry.h:664 __raw_spin_unlock_irq include/linux/spinlock_api_smp.h:160 [inline] _raw_spin_unlock_irq+0x25/0x40 kernel/locking/spinlock.c:202 spin_unlock_irq include/linux/spinlock.h:399 [inline] sw_sync_debugfs_release+0x160/0x240 drivers/dma-buf/sw_sync.c:321 __fput+0x286/0x9f0 fs/file_table.c:311 task_work_run+0xdd/0x1a0 kernel/task_work.c:164 exit_task_work include/linux/task_work.h:32 [inline] do_exit+0xb29/0x2a30 kernel/exit.c:806 do_group_exit+0xd2/0x2f0 kernel/exit.c:935 get_signal+0x4b0/0x28c0 kernel/signal.c:2862 arch_do_signal_or_restart+0x2a9/0x1c40 arch/x86/kernel/signal.c:868 handle_signal_work kernel/entry/common.c:148 [inline] exit_to_user_mode_loop kernel/entry/common.c:172 [inline] exit_to_user_mode_prepare+0x17d/0x290 kernel/entry/common.c:207 __syscall_exit_to_user_mode_work kernel/entry/common.c:289 [inline] syscall_exit_to_user_mode+0x19/0x60 kernel/entry/common.c:300 ret_from_fork+0x15/0x30 arch/x86/entry/entry_64.S:288 irq event stamp: 124 hardirqs last enabled at (123): [<ffffffff894fd980>] __raw_spin_unlock_irqrestore include/linux/spinlock_api_smp.h:151 [inline] hardirqs last enabled at (123): [<ffffffff894fd980>] _raw_spin_unlock_irqrestore+0x50/0x70 kernel/locking/spinlock.c:194 hardirqs last disabled at (124): [<ffffffff894fd6e1>] __raw_spin_lock_irq include/linux/spinlock_api_smp.h:117 [inline] hardirqs last disabled at (124): [<ffffffff894fd6e1>] _raw_spin_lock_irq+0x41/0x50 kernel/locking/spinlock.c:170 softirqs last enabled at (116): [<ffffffff81465513>] invoke_softirq kernel/softirq.c:432 [inline] softirqs last enabled at (116): [<ffffffff81465513>] __irq_exit_rcu+0x123/0x180 kernel/softirq.c:637 softirqs last disabled at (97): [<ffffffff81465513>] invoke_softirq kernel/softirq.c:432 [inline] softirqs last disabled at (97): [<ffffffff81465513>] __irq_exit_rcu+0x123/0x180 kernel/softirq.c:637 other info that might help us debug this: Possible unsafe locking scenario: CPU0 ---- lock(sync_timeline_list_lock); <Interrupt> lock(sync_timeline_list_lock); *** DEADLOCK *** 3 locks held by syz-executor.2/18360: #0: ffff88801e30c0f0 (&f->f_pos_lock){+.+.}-{3:3}, at: __fdget_pos+0xe9/0x100 fs/file.c:1034 #1: ffff88807a26dd58 (&p->lock){+.+.}-{3:3}, at: seq_read_iter+0xdf/0x1280 fs/seq_file.c:182 #2: ffffffff8c712cf8 (sync_timeline_list_lock){?...}-{2:2}, at: spin_lock_irq include/linux/spinlock.h:374 [inline] #2: ffffffff8c712cf8 (sync_timeline_list_lock){?...}-{2:2}, at: sync_info_debugfs_show+0x2d/0x200 drivers/dma-buf/sync_debug.c:147 stack backtrace: CPU: 0 PID: 18360 Comm: syz-executor.2 Not tainted 5.16.0-syzkaller #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Call Trace: <TASK> __dump_stack lib/dump_stack.c:88 [inline] dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:106 print_usage_bug kernel/locking/lockdep.c:203 [inline] valid_state kernel/locking/lockdep.c:3945 [inline] mark_lock_irq kernel/locking/lockdep.c:4148 [inline] mark_lock.cold+0x61/0x8e kernel/locking/lockdep.c:4605 mark_held_locks+0x9f/0xe0 kernel/locking/lockdep.c:4206 __trace_hardirqs_on_caller kernel/locking/lockdep.c:4224 [inline] lockdep_hardirqs_on_prepare kernel/locking/lockdep.c:4292 [inline] lockdep_hardirqs_on_prepare+0x135/0x400 kernel/locking/lockdep.c:4244 trace_hardirqs_on+0x5b/0x1c0 kernel/trace/trace_preemptirq.c:49 __raw_spin_unlock_irq include/linux/spinlock_api_smp.h:159 [inline] _raw_spin_unlock_irq+0x1f/0x40 kernel/locking/spinlock.c:202 spin_unlock_irq include/linux/spinlock.h:399 [inline] sync_print_obj drivers/dma-buf/sync_debug.c:118 [inline] sync_info_debugfs_show+0xeb/0x200 drivers/dma-buf/sync_debug.c:153 seq_read_iter+0x4f5/0x1280 fs/seq_file.c:230 seq_read+0x3e8/0x5c0 fs/seq_file.c:162 vfs_read+0x1b5/0x600 fs/read_write.c:479 ksys_read+0x12d/0x250 fs/read_write.c:619 do_syscall_x64 arch/x86/entry/common.c:50 [inline] do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80 entry_SYSCALL_64_after_hwframe+0x44/0xae RIP: 0033:0x7f781fb4b059 Code: ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b8 ff ff ff f7 d8 64 89 01 48 RSP: 002b:00007f781e4c0168 EFLAGS: 00000246 ORIG_RAX: 0000000000000000 RAX: ffffffffffffffda RBX: 00007f781fc5df60 RCX: 00007f781fb4b059 RDX: 0000000000000008 RSI: 0000000020000140 RDI: 0000000000000003 RBP: 00007f781fba508d R08: 0000000000000000 R09: 0000000000000000 R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000 R13: 00007fff0ff6575f R14: 00007f781e4c0300 R15: 0000000000022000 </TASK> --- This report is generated by a bot. It may contain errors. See https://goo.gl/tpsmEJ for more information about syzbot. syzbot engineers can be reached at syzkaller(a)googlegroups.com. syzbot will keep track of this issue. See: https://goo.gl/tpsmEJ#status for how to communicate with syzbot.

2 years

DMA-buf and uncached system memory

by Christian König

Hi guys, we are currently working an Freesync and direct scan out from system memory on AMD APUs in A+A laptops. On problem we stumbled over is that our display hardware needs to scan out from uncached system memory and we currently don't have a way to communicate that through DMA-buf. For our specific use case at hand we are going to implement something driver specific, but the question is should we have something more generic for this? After all the system memory access pattern is a PCIe extension and as such something generic. Regards, Christian.

2 years, 4 months

Re: [syzbot] WARNING in __dma_map_sg_attrs

by Daniel Vetter

On Sat, Feb 05, 2022 at 12:18:23PM -0800, syzbot wrote: > syzbot has found a reproducer for the following issue on: > > HEAD commit: 0457e5153e0e Merge tag 'for-linus' of git://git.kernel.org.. > git tree: upstream > console output: https://syzkaller.appspot.com/x/log.txt?x=11b2637c700000 > kernel config: https://syzkaller.appspot.com/x/.config?x=6f043113811433a5 > dashboard link: https://syzkaller.appspot.com/bug?extid=10e27961f4da37c443b2 > compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2 > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=11c65542700000 > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1163f480700000 > > IMPORTANT: if you fix the issue, please add the following tag to the commit: > Reported-by: syzbot+10e27961f4da37c443b2(a)syzkaller.appspotmail.com Adding Gerd, since this seems to blow up in udmabuf. I wonder why syzbot didn't figure this out, since it seems to have correctly added both dma-api and dma-buf people. Just not the maintainer for the begin_cpu_udmabuf function in the middle of the backtrace? -Daniel > > ------------[ cut here ]------------ > WARNING: CPU: 1 PID: 3595 at kernel/dma/mapping.c:188 __dma_map_sg_attrs+0x181/0x1f0 kernel/dma/mapping.c:188 > Modules linked in: > CPU: 0 PID: 3595 Comm: syz-executor249 Not tainted 5.17.0-rc2-syzkaller-00316-g0457e5153e0e #0 > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 > RIP: 0010:__dma_map_sg_attrs+0x181/0x1f0 kernel/dma/mapping.c:188 > Code: 00 00 00 00 00 fc ff df 48 c1 e8 03 80 3c 10 00 75 71 4c 8b 3d c0 83 b5 0d e9 db fe ff ff e8 b6 0f 13 00 0f 0b e8 af 0f 13 00 <0f> 0b 45 31 e4 e9 54 ff ff ff e8 a0 0f 13 00 49 8d 7f 50 48 b8 00 > RSP: 0018:ffffc90002a07d68 EFLAGS: 00010293 > RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000 > RDX: ffff88807e25e2c0 RSI: ffffffff81649e91 RDI: ffff88801b848408 > RBP: ffff88801b848000 R08: 0000000000000002 R09: ffff88801d86c74f > R10: ffffffff81649d72 R11: 0000000000000001 R12: 0000000000000002 > R13: ffff88801d86c680 R14: 0000000000000001 R15: 0000000000000000 > FS: 0000555556e30300(0000) GS:ffff8880b9d00000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 00000000200000cc CR3: 000000001d74a000 CR4: 00000000003506e0 > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > Call Trace: > <TASK> > dma_map_sgtable+0x70/0xf0 kernel/dma/mapping.c:264 > get_sg_table.isra.0+0xe0/0x160 drivers/dma-buf/udmabuf.c:72 > begin_cpu_udmabuf+0x130/0x1d0 drivers/dma-buf/udmabuf.c:126 > dma_buf_begin_cpu_access+0xfd/0x1d0 drivers/dma-buf/dma-buf.c:1164 > dma_buf_ioctl+0x259/0x2b0 drivers/dma-buf/dma-buf.c:363 > vfs_ioctl fs/ioctl.c:51 [inline] > __do_sys_ioctl fs/ioctl.c:874 [inline] > __se_sys_ioctl fs/ioctl.c:860 [inline] > __x64_sys_ioctl+0x193/0x200 fs/ioctl.c:860 > do_syscall_x64 arch/x86/entry/common.c:50 [inline] > do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80 > entry_SYSCALL_64_after_hwframe+0x44/0xae > RIP: 0033:0x7f62fcf530f9 > Code: 28 c3 e8 2a 14 00 00 66 2e 0f 1f 84 00 00 00 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 c0 ff ff ff f7 d8 64 89 01 48 > RSP: 002b:00007ffe3edab9b8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010 > RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f62fcf530f9 > RDX: 0000000020000200 RSI: 0000000040086200 RDI: 0000000000000006 > RBP: 00007f62fcf170e0 R08: 0000000000000000 R09: 0000000000000000 > R10: 0000000000000000 R11: 0000000000000246 R12: 00007f62fcf17170 > R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000 > </TASK> > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch

2 years, 7 months

Re: [syzbot] WARNING in drm_gem_shmem_vm_open

by syzbot

syzbot suspects this issue was fixed by commit: commit 0499f419b76f94ede08304aad5851144813ac55c Author: Javier Martinez Canillas <javierm(a)redhat.com> Date: Mon Jan 10 09:56:25 2022 +0000 video: vga16fb: Only probe for EGA and VGA 16 color graphic cards bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=126571e0700000 start commit: 5d6ab0bb408f Merge tag 'xtensa-20211008' of git://github.c.. git tree: upstream kernel config: https://syzkaller.appspot.com/x/.config?x=32e6048063923b7b dashboard link: https://syzkaller.appspot.com/bug?extid=91525b2bd4b5dff71619 syz repro: https://syzkaller.appspot.com/x/repro.syz?x=11073300b00000 If the result looks correct, please mark the issue as fixed by replying with: #syz fix: video: vga16fb: Only probe for EGA and VGA 16 color graphic cards For information about bisection process see: https://goo.gl/tpsmEJ#bisection

2 years, 7 months

[PATCH v2 00/12] iio: buffer-dma: write() and new DMABUF based API

by Paul Cercueil

Hi Jonathan, This is the V2 of my patchset that introduces a new userspace interface based on DMABUF objects to complement the fileio API, and adds write() support to the existing fileio API. Changes since v1: - the patches that were merged in v1 have been (obviously) dropped from this patchset; - the patch that was setting the write-combine cache setting has been dropped as well, as it was simply not useful. - [01/12]: * Only remove the outgoing queue, and keep the incoming queue, as we want the buffer to start streaming data as soon as it is enabled. * Remove IIO_BLOCK_STATE_DEQUEUED, since it is now functionally the same as IIO_BLOCK_STATE_DONE. - [02/12]: * Fix block->state not being reset in iio_dma_buffer_request_update() for output buffers. * Only update block->bytes_used once and add a comment about why we update it. * Add a comment about why we're setting a different state for output buffers in iio_dma_buffer_request_update() * Remove useless cast to bool (!!) in iio_dma_buffer_io() - [05/12]: Only allow the new IOCTLs on the buffer FD created with IIO_BUFFER_GET_FD_IOCTL(). - [12/12]: * Explicitly state that the new interface is optional and is not implemented by all drivers. * The IOCTLs can now only be called on the buffer FD returned by IIO_BUFFER_GET_FD_IOCTL. * Move the page up a bit in the index since it is core stuff and not driver-specific. The patches not listed here have not been modified since v1. Cheers, -Paul Alexandru Ardelean (1): iio: buffer-dma: split iio_dma_buffer_fileio_free() function Paul Cercueil (11): iio: buffer-dma: Get rid of outgoing queue iio: buffer-dma: Enable buffer write support iio: buffer-dmaengine: Support specifying buffer direction iio: buffer-dmaengine: Enable write support iio: core: Add new DMABUF interface infrastructure iio: buffer-dma: Use DMABUFs instead of custom solution iio: buffer-dma: Implement new DMABUF based userspace API iio: buffer-dmaengine: Support new DMABUF based userspace API iio: core: Add support for cyclic buffers iio: buffer-dmaengine: Add support for cyclic buffers Documentation: iio: Document high-speed DMABUF based API Documentation/driver-api/dma-buf.rst | 2 + Documentation/iio/dmabuf_api.rst | 94 +++ Documentation/iio/index.rst | 2 + drivers/iio/adc/adi-axi-adc.c | 3 +- drivers/iio/buffer/industrialio-buffer-dma.c | 610 ++++++++++++++---- .../buffer/industrialio-buffer-dmaengine.c | 42 +- drivers/iio/industrialio-buffer.c | 60 ++ include/linux/iio/buffer-dma.h | 38 +- include/linux/iio/buffer-dmaengine.h | 5 +- include/linux/iio/buffer_impl.h | 8 + include/uapi/linux/iio/buffer.h | 30 + 11 files changed, 749 insertions(+), 145 deletions(-) create mode 100644 Documentation/iio/dmabuf_api.rst -- 2.34.1

2 years, 8 months

completely rework the dma_resv semantic

by Christian König

Hi Daniel, just a gentle ping that you wanted to take a look at this. Not much changed compared to the last version, only a minor bugfix in the dma_resv_get_singleton error handling. Regards, Christian.

2 years, 9 months

Re: [Linaro-mm-sig] [PATCH 18/24] dma-buf: add enum dma_resv_usage v3

by Daniel Vetter

On Tue, Dec 07, 2021 at 01:34:05PM +0100, Christian König wrote: > This change adds the dma_resv_usage enum and allows us to specify why a > dma_resv object is queried for its containing fences. > > Additional to that a dma_resv_usage_rw() helper function is added to aid > retrieving the fences for a read or write userspace submission. > > This is then deployed to the different query functions of the dma_resv > object and all of their users. When the write paratermer was previously > true we now use DMA_RESV_USAGE_WRITE and DMA_RESV_USAGE_READ otherwise. > > v2: add KERNEL/OTHER in separate patch > v3: some kerneldoc suggestions by Daniel > > Signed-off-by: Christian König <christian.koenig(a)amd.com> Just commenting on the kerneldoc here. > diff --git a/include/linux/dma-resv.h b/include/linux/dma-resv.h > index 40ac9d486f8f..d96d8ca9af56 100644 > --- a/include/linux/dma-resv.h > +++ b/include/linux/dma-resv.h > @@ -49,6 +49,49 @@ extern struct ww_class reservation_ww_class; > > struct dma_resv_list; > > +/** > + * enum dma_resv_usage - how the fences from a dma_resv obj are used > + * > + * This enum describes the different use cases for a dma_resv object and > + * controls which fences are returned when queried. We need to link here to both dma_buf.resv and from there to here. Also we had a fair amount of text in the old dma_resv fields which should probably be included here. > + */ > +enum dma_resv_usage { > + /** > + * @DMA_RESV_USAGE_WRITE: Implicit write synchronization. > + * > + * This should only be used for userspace command submissions which add > + * an implicit write dependency. > + */ > + DMA_RESV_USAGE_WRITE, > + > + /** > + * @DMA_RESV_USAGE_READ: Implicit read synchronization. > + * > + * This should only be used for userspace command submissions which add > + * an implicit read dependency. I think the above would benefit from at least a link each to &dma_buf.resv for further discusion. Plus the READ flag needs a huge warning that in general it does _not_ guarantee that neither there's no writes possible, nor that the writes can be assumed mistakes and dropped (on buffer moves e.g.). Drivers can only make further assumptions for driver-internal dma_resv objects (e.g. on vm/pagetables) or when the fences are all fences of the same driver (e.g. the special sync rules amd has that takes the fence owner into account). We have this documented in the dma_buf.resv rules, but since it came up again in a discussion with Thomas H. somewhere, it's better to hammer this in a few more time. Specically in generally ignoring READ fences for buffer moves (well the copy job, memory freeing still has to wait for all of them) is a correctness bug. Maybe include a big warning that really the difference between READ and WRITE should only matter for implicit sync, and _not_ for anything else the kernel does. I'm assuming the actual replacement is all mechanical, so I skipped that one for now, that's for next year :-) -Daniel > + */ > + DMA_RESV_USAGE_READ, > +}; > + > +/** > + * dma_resv_usage_rw - helper for implicit sync > + * @write: true if we create a new implicit sync write > + * > + * This returns the implicit synchronization usage for write or read accesses, > + * see enum dma_resv_usage. > + */ > +static inline enum dma_resv_usage dma_resv_usage_rw(bool write) > +{ > + /* This looks confusing at first sight, but is indeed correct. > + * > + * The rational is that new write operations needs to wait for the > + * existing read and write operations to finish. > + * But a new read operation only needs to wait for the existing write > + * operations to finish. > + */ > + return write ? DMA_RESV_USAGE_READ : DMA_RESV_USAGE_WRITE; > +} > + > /** > * struct dma_resv - a reservation object manages fences for a buffer > * > @@ -147,8 +190,8 @@ struct dma_resv_iter { > /** @obj: The dma_resv object we iterate over */ > struct dma_resv *obj; > > - /** @all_fences: If all fences should be returned */ > - bool all_fences; > + /** @usage: Controls which fences are returned */ > + enum dma_resv_usage usage; > > /** @fence: the currently handled fence */ > struct dma_fence *fence; > @@ -178,14 +221,14 @@ struct dma_fence *dma_resv_iter_next(struct dma_resv_iter *cursor); > * dma_resv_iter_begin - initialize a dma_resv_iter object > * @cursor: The dma_resv_iter object to initialize > * @obj: The dma_resv object which we want to iterate over > - * @all_fences: If all fences should be returned or just the exclusive one > + * @usage: controls which fences to include, see enum dma_resv_usage. > */ > static inline void dma_resv_iter_begin(struct dma_resv_iter *cursor, > struct dma_resv *obj, > - bool all_fences) > + enum dma_resv_usage usage) > { > cursor->obj = obj; > - cursor->all_fences = all_fences; > + cursor->usage = usage; > cursor->fence = NULL; > } > > @@ -242,7 +285,7 @@ static inline bool dma_resv_iter_is_restarted(struct dma_resv_iter *cursor) > * dma_resv_for_each_fence - fence iterator > * @cursor: a struct dma_resv_iter pointer > * @obj: a dma_resv object pointer > - * @all_fences: true if all fences should be returned > + * @usage: controls which fences to return > * @fence: the current fence > * > * Iterate over the fences in a struct dma_resv object while holding the > @@ -251,8 +294,8 @@ static inline bool dma_resv_iter_is_restarted(struct dma_resv_iter *cursor) > * valid as long as the lock is held and so no extra reference to the fence is > * taken. > */ > -#define dma_resv_for_each_fence(cursor, obj, all_fences, fence) \ > - for (dma_resv_iter_begin(cursor, obj, all_fences), \ > +#define dma_resv_for_each_fence(cursor, obj, usage, fence) \ > + for (dma_resv_iter_begin(cursor, obj, usage), \ > fence = dma_resv_iter_first(cursor); fence; \ > fence = dma_resv_iter_next(cursor)) > > @@ -419,14 +462,14 @@ void dma_resv_add_shared_fence(struct dma_resv *obj, struct dma_fence *fence); > void dma_resv_replace_fences(struct dma_resv *obj, uint64_t context, > struct dma_fence *fence); > void dma_resv_add_excl_fence(struct dma_resv *obj, struct dma_fence *fence); > -int dma_resv_get_fences(struct dma_resv *obj, bool write, > +int dma_resv_get_fences(struct dma_resv *obj, enum dma_resv_usage usage, > unsigned int *num_fences, struct dma_fence ***fences); > -int dma_resv_get_singleton(struct dma_resv *obj, bool write, > +int dma_resv_get_singleton(struct dma_resv *obj, enum dma_resv_usage usage, > struct dma_fence **fence); > int dma_resv_copy_fences(struct dma_resv *dst, struct dma_resv *src); > -long dma_resv_wait_timeout(struct dma_resv *obj, bool wait_all, bool intr, > - unsigned long timeout); > -bool dma_resv_test_signaled(struct dma_resv *obj, bool test_all); > +long dma_resv_wait_timeout(struct dma_resv *obj, enum dma_resv_usage usage, > + bool intr, unsigned long timeout); > +bool dma_resv_test_signaled(struct dma_resv *obj, enum dma_resv_usage usage); > void dma_resv_describe(struct dma_resv *obj, struct seq_file *seq); > > #endif /* _LINUX_RESERVATION_H */ > -- > 2.25.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch

2 years, 9 months

[PATCH] dma-fance: Add dma_fence_assert_in_signalling_section

by Daniel Vetter

Useful for checking for dma-fence signalling annotations since they don't quite nest as freely as we'd like to. Cc: Matthew Brost <matthew.brost(a)intel.com> Signed-off-by: Daniel Vetter <daniel.vetter(a)intel.com> Cc: Sumit Semwal <sumit.semwal(a)linaro.org> Cc: Gustavo Padovan <gustavo(a)padovan.org> Cc: "Christian König" <christian.koenig(a)amd.com> Cc: linux-media(a)vger.kernel.org Cc: linaro-mm-sig(a)lists.linaro.org --- drivers/dma-buf/dma-fence.c | 19 +++++++++++++++++++ include/linux/dma-fence.h | 2 ++ 2 files changed, 21 insertions(+) diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c index 066400ed8841..2b7c3fc965e6 100644 --- a/drivers/dma-buf/dma-fence.c +++ b/drivers/dma-buf/dma-fence.c @@ -307,6 +307,25 @@ bool dma_fence_begin_signalling(void) } EXPORT_SYMBOL(dma_fence_begin_signalling); +/** + * dma_fence_assert_in_signalling_section - check fence signalling annotations + * + * Since dma_fence_begin_signalling() and dma_fence_end_signalling() are built + * using lockdep annotations they have limitations on how freely they can be + * nested. Specifically, they cannot be on both inside and outside of locked + * sections, which in practice means the annotations often have to be pushed out + * to the top level callers. + * + * To ensure low-level functions are only called with the correction + * annotations, this function can be used to check for that. + */ +void dma_fence_assert_in_signalling_section(void) +{ + if (!in_atomic()) + lockdep_assert(lock_is_held(&dma_fence_lockdep_map)); +} +EXPORT_SYMBOL(dma_fence_assert_in_signalling_section); + /** * dma_fence_end_signalling - end a critical DMA fence signalling section * @cookie: opaque cookie from dma_fence_begin_signalling() diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h index 775cdc0b4f24..7179a5692f72 100644 --- a/include/linux/dma-fence.h +++ b/include/linux/dma-fence.h @@ -356,6 +356,7 @@ dma_fence_get_rcu_safe(struct dma_fence __rcu **fencep) #ifdef CONFIG_LOCKDEP bool dma_fence_begin_signalling(void); +void dma_fence_assert_in_signalling_section(void); void dma_fence_end_signalling(bool cookie); void __dma_fence_might_wait(void); #else @@ -363,6 +364,7 @@ static inline bool dma_fence_begin_signalling(void) { return true; } +static inline void dma_fence_assert_in_signalling_section(void) {} static inline void dma_fence_end_signalling(bool cookie) {} static inline void __dma_fence_might_wait(void) {} #endif -- 2.34.1

2 years, 10 months

[PATCH] dma-buf: check the return value of kstrdup()

by xkernel.wang＠foxmail.com

From: Xiaoke Wang <xkernel.wang(a)foxmail.com> kstrdup() is a memory allocation function which can return NULL when some internaly memory errors happen. It is better to check the return value of it to prevent further wrong memory access. Signed-off-by: Xiaoke Wang <xkernel.wang(a)foxmail.com> --- drivers/dma-buf/selftest.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/dma-buf/selftest.c b/drivers/dma-buf/selftest.c index c60b694..2c29e2a 100644 --- a/drivers/dma-buf/selftest.c +++ b/drivers/dma-buf/selftest.c @@ -50,6 +50,9 @@ static bool apply_subtest_filter(const char *caller, const char *name) bool result = true; filter = kstrdup(__st_filter, GFP_KERNEL); + if (!filter) + return false; + for (sep = filter; (tok = strsep(&sep, ","));) { bool allow = true; char *sl; --

2 years, 10 months

[PATCH] drm/amdgpu: check vm bo eviction valuable at last

by Qiang Yu

Workstation application ANSA/META get this error dmesg: [drm:amdgpu_gem_va_ioctl [amdgpu]] *ERROR* Couldn't update BO_VA (-16) This is caused by: 1. create a 256MB buffer in invisible VRAM 2. CPU map the buffer and access it causes vm_fault and try to move it to visible VRAM 3. force visible VRAM space and traverse all VRAM bos to check if evicting this bo is valuable 4. when checking a VM bo (in invisible VRAM), amdgpu_vm_evictable() will set amdgpu_vm->evicting, but latter due to not in visible VRAM, won't really evict it so not add it to amdgpu_vm->evicted 5. before next CS to clear the amdgpu_vm->evicting, user VM ops ioctl will pass amdgpu_vm_ready() (check amdgpu_vm->evicted) but fail in amdgpu_vm_bo_update_mapping() (check amdgpu_vm->evicting) and get this error log This error won't affect functionality as next CS will finish the waiting VM ops. But we'd better make the amdgpu_vm->evicting correctly reflact the vm status and clear the error log. Signed-off-by: Qiang Yu <qiang.yu(a)amd.com> --- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 85 ++++++++++++++----------- 1 file changed, 47 insertions(+), 38 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index 5a32ee66d8c8..88a27911054f 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -1306,45 +1306,11 @@ uint64_t amdgpu_ttm_tt_pte_flags(struct amdgpu_device *adev, struct ttm_tt *ttm, return flags; } -/* - * amdgpu_ttm_bo_eviction_valuable - Check to see if we can evict a buffer - * object. - * - * Return true if eviction is sensible. Called by ttm_mem_evict_first() on - * behalf of ttm_bo_mem_force_space() which tries to evict buffer objects until - * it can find space for a new object and by ttm_bo_force_list_clean() which is - * used to clean out a memory space. - */ -static bool amdgpu_ttm_bo_eviction_valuable(struct ttm_buffer_object *bo, - const struct ttm_place *place) +static bool amdgpu_ttm_mem_eviction_valuable(struct ttm_buffer_object *bo, + const struct ttm_place *place) { unsigned long num_pages = bo->resource->num_pages; struct amdgpu_res_cursor cursor; - struct dma_resv_list *flist; - struct dma_fence *f; - int i; - - /* Swapout? */ - if (bo->resource->mem_type == TTM_PL_SYSTEM) - return true; - - if (bo->type == ttm_bo_type_kernel && - !amdgpu_vm_evictable(ttm_to_amdgpu_bo(bo))) - return false; - - /* If bo is a KFD BO, check if the bo belongs to the current process. - * If true, then return false as any KFD process needs all its BOs to - * be resident to run successfully - */ - flist = dma_resv_shared_list(bo->base.resv); - if (flist) { - for (i = 0; i < flist->shared_count; ++i) { - f = rcu_dereference_protected(flist->shared[i], - dma_resv_held(bo->base.resv)); - if (amdkfd_fence_check_mm(f, current->mm)) - return false; - } - } switch (bo->resource->mem_type) { case AMDGPU_PL_PREEMPT: @@ -1377,10 +1343,53 @@ static bool amdgpu_ttm_bo_eviction_valuable(struct ttm_buffer_object *bo, return false; default: - break; + return ttm_bo_eviction_valuable(bo, place); } +} - return ttm_bo_eviction_valuable(bo, place); +/* + * amdgpu_ttm_bo_eviction_valuable - Check to see if we can evict a buffer + * object. + * + * Return true if eviction is sensible. Called by ttm_mem_evict_first() on + * behalf of ttm_bo_mem_force_space() which tries to evict buffer objects until + * it can find space for a new object and by ttm_bo_force_list_clean() which is + * used to clean out a memory space. + */ +static bool amdgpu_ttm_bo_eviction_valuable(struct ttm_buffer_object *bo, + const struct ttm_place *place) +{ + struct dma_resv_list *flist; + struct dma_fence *f; + int i; + + /* Swapout? */ + if (bo->resource->mem_type == TTM_PL_SYSTEM) + return true; + + /* If bo is a KFD BO, check if the bo belongs to the current process. + * If true, then return false as any KFD process needs all its BOs to + * be resident to run successfully + */ + flist = dma_resv_shared_list(bo->base.resv); + if (flist) { + for (i = 0; i < flist->shared_count; ++i) { + f = rcu_dereference_protected(flist->shared[i], + dma_resv_held(bo->base.resv)); + if (amdkfd_fence_check_mm(f, current->mm)) + return false; + } + } + + /* Check by different mem type. */ + if (!amdgpu_ttm_mem_eviction_valuable(bo, place)) + return false; + + /* VM bo should be checked at last because it will mark VM evicting. */ + if (bo->type == ttm_bo_type_kernel) + return amdgpu_vm_evictable(ttm_to_amdgpu_bo(bo)); + + return true; } static void amdgpu_ttm_vram_mm_access(struct amdgpu_device *adev, loff_t pos, -- 2.25.1

2 years, 10 months

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig February 2022