Linaro-mm-sig

linaro-mm-sig@lists.linaro.org

26 participants
3083 discussions

[PATCH 17/65] drm/scheduler: use dma-fence annotations in main thread

by Daniel Vetter

If the scheduler rt thread gets stuck on a mutex that we're holding while waiting for gpu workloads to complete, we have a problem. Add dma-fence annotations so that lockdep can check this for us. I've tried to quite carefully review this, and I think it's at the right spot. But obviosly no expert on drm scheduler. Cc: linux-media(a)vger.kernel.org Cc: linaro-mm-sig(a)lists.linaro.org Cc: linux-rdma(a)vger.kernel.org Cc: amd-gfx(a)lists.freedesktop.org Cc: intel-gfx(a)lists.freedesktop.org Cc: Chris Wilson <chris(a)chris-wilson.co.uk> Cc: Maarten Lankhorst <maarten.lankhorst(a)linux.intel.com> Cc: Christian König <christian.koenig(a)amd.com> Signed-off-by: Daniel Vetter <daniel.vetter(a)intel.com> --- drivers/gpu/drm/scheduler/sched_main.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c index 9a0d77a68018..f69abc4e70d3 100644 --- a/drivers/gpu/drm/scheduler/sched_main.c +++ b/drivers/gpu/drm/scheduler/sched_main.c @@ -764,9 +764,12 @@ static int drm_sched_main(void *param) { struct drm_gpu_scheduler *sched = (struct drm_gpu_scheduler *)param; int r; + bool fence_cookie; sched_set_fifo_low(current); + fence_cookie = dma_fence_begin_signalling(); + while (!kthread_should_stop()) { struct drm_sched_entity *entity = NULL; struct drm_sched_fence *s_fence; @@ -824,6 +827,9 @@ static int drm_sched_main(void *param) wake_up(&sched->job_scheduled); } + + dma_fence_end_signalling(fence_cookie); + return 0; } -- 2.28.0

4 years, 12 months

[PATCH 08/65] drm/amdgpu: add dma-fence annotations to atomic commit path

by Daniel Vetter

I need a canary in a ttm-based atomic driver to make sure the dma_fence_begin/end_signalling annotations actually work. Cc: linux-media(a)vger.kernel.org Cc: linaro-mm-sig(a)lists.linaro.org Cc: linux-rdma(a)vger.kernel.org Cc: amd-gfx(a)lists.freedesktop.org Cc: intel-gfx(a)lists.freedesktop.org Cc: Chris Wilson <chris(a)chris-wilson.co.uk> Cc: Maarten Lankhorst <maarten.lankhorst(a)linux.intel.com> Cc: Christian König <christian.koenig(a)amd.com> Signed-off-by: Daniel Vetter <daniel.vetter(a)intel.com> --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index bb1bc7f5d149..b05fecf06f25 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -57,6 +57,7 @@ #include "ivsrcid/ivsrcid_vislands30.h" +#include <linux/module.h> #include <linux/module.h> #include <linux/moduleparam.h> #include <linux/version.h> @@ -7492,6 +7493,9 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state) struct dm_crtc_state *dm_old_crtc_state, *dm_new_crtc_state; int crtc_disable_count = 0; bool mode_set_reset_required = false; + bool fence_cookie; + + fence_cookie = dma_fence_begin_signalling(); drm_atomic_helper_update_legacy_modeset_state(dev, state); drm_atomic_helper_calc_timestamping_constants(state); @@ -7816,6 +7820,8 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state) /* Signal HW programming completion */ drm_atomic_helper_commit_hw_done(state); + dma_fence_end_signalling(fence_cookie); + if (wait_for_vblank) drm_atomic_helper_wait_for_flip_done(dev, state); -- 2.28.0

4 years, 12 months

[PATCH 07/65] drm/vblank: Annotate with dma-fence signalling section

by Daniel Vetter

This is rather overkill since currently all drivers call this from hardirq (or at least timers). But maybe in the future we're going to have thread irq handlers and what not, doesn't hurt to be prepared. Plus this is an easy start for sprinkling these fence annotations into shared code. Cc: linux-media(a)vger.kernel.org Cc: linaro-mm-sig(a)lists.linaro.org Cc: linux-rdma(a)vger.kernel.org Cc: amd-gfx(a)lists.freedesktop.org Cc: intel-gfx(a)lists.freedesktop.org Cc: Chris Wilson <chris(a)chris-wilson.co.uk> Cc: Maarten Lankhorst <maarten.lankhorst(a)linux.intel.com> Cc: Christian König <christian.koenig(a)amd.com> Signed-off-by: Daniel Vetter <daniel.vetter(a)intel.com> --- drivers/gpu/drm/drm_vblank.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c index f135b79593dd..ba7e741764aa 100644 --- a/drivers/gpu/drm/drm_vblank.c +++ b/drivers/gpu/drm/drm_vblank.c @@ -24,6 +24,7 @@ * OTHER DEALINGS IN THE SOFTWARE. */ +#include <linux/dma-fence.h> #include <linux/export.h> #include <linux/kthread.h> #include <linux/moduleparam.h> @@ -1913,7 +1914,7 @@ bool drm_handle_vblank(struct drm_device *dev, unsigned int pipe) { struct drm_vblank_crtc *vblank = &dev->vblank[pipe]; unsigned long irqflags; - bool disable_irq; + bool disable_irq, fence_cookie; if (drm_WARN_ON_ONCE(dev, !drm_dev_has_vblank(dev))) return false; @@ -1921,6 +1922,8 @@ bool drm_handle_vblank(struct drm_device *dev, unsigned int pipe) if (drm_WARN_ON(dev, pipe >= dev->num_crtcs)) return false; + fence_cookie = dma_fence_begin_signalling(); + spin_lock_irqsave(&dev->event_lock, irqflags); /* Need timestamp lock to prevent concurrent execution with @@ -1933,6 +1936,7 @@ bool drm_handle_vblank(struct drm_device *dev, unsigned int pipe) if (!vblank->enabled) { spin_unlock(&dev->vblank_time_lock); spin_unlock_irqrestore(&dev->event_lock, irqflags); + dma_fence_end_signalling(fence_cookie); return false; } @@ -1959,6 +1963,8 @@ bool drm_handle_vblank(struct drm_device *dev, unsigned int pipe) if (disable_irq) vblank_disable_fn(&vblank->disable_timer); + dma_fence_end_signalling(fence_cookie); + return true; } EXPORT_SYMBOL(drm_handle_vblank); -- 2.28.0

4 years, 12 months

[PATCH 06/65] drm/vkms: Annotate vblank timer

by Daniel Vetter

This is needed to signal the fences from page flips, annotate it accordingly. We need to annotate entire timer callback since if we get stuck anywhere in there, then the timer stops, and hence fences stop. Just annotating the top part that does the vblank handling isn't enough. Tested-by: Melissa Wen <melissa.srw(a)gmail.com> Reviewed-by: Rodrigo Siqueira <rodrigosiqueiramelo(a)gmail.com> Cc: linux-media(a)vger.kernel.org Cc: linaro-mm-sig(a)lists.linaro.org Cc: linux-rdma(a)vger.kernel.org Cc: amd-gfx(a)lists.freedesktop.org Cc: intel-gfx(a)lists.freedesktop.org Cc: Chris Wilson <chris(a)chris-wilson.co.uk> Cc: Maarten Lankhorst <maarten.lankhorst(a)linux.intel.com> Cc: Christian König <christian.koenig(a)amd.com> Signed-off-by: Daniel Vetter <daniel.vetter(a)intel.com> Cc: Rodrigo Siqueira <rodrigosiqueiramelo(a)gmail.com> Cc: Haneen Mohammed <hamohammed.sa(a)gmail.com> Cc: Daniel Vetter <daniel(a)ffwll.ch> --- drivers/gpu/drm/vkms/vkms_crtc.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c index e43e4e1b268a..8124d8f2ee15 100644 --- a/drivers/gpu/drm/vkms/vkms_crtc.c +++ b/drivers/gpu/drm/vkms/vkms_crtc.c @@ -1,5 +1,7 @@ // SPDX-License-Identifier: GPL-2.0+ +#include <linux/dma-fence.h> + #include <drm/drm_atomic.h> #include <drm/drm_atomic_helper.h> #include <drm/drm_probe_helper.h> @@ -14,7 +16,9 @@ static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer) struct drm_crtc *crtc = &output->crtc; struct vkms_crtc_state *state; u64 ret_overrun; - bool ret; + bool ret, fence_cookie; + + fence_cookie = dma_fence_begin_signalling(); ret_overrun = hrtimer_forward_now(&output->vblank_hrtimer, output->period_ns); @@ -49,6 +53,8 @@ static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer) DRM_DEBUG_DRIVER("Composer worker already queued\n"); } + dma_fence_end_signalling(fence_cookie); + return HRTIMER_RESTART; } -- 2.28.0

4 years, 12 months

[PATCH 05/65] drm/atomic-helper: Add dma-fence annotations

by Daniel Vetter

This is a bit disappointing since we need to split the annotations over all the different parts. I was considering just leaking the critical section into the ->atomic_commit_tail callback of each driver. But that would mean we need to pass the fence_cookie into each driver (there's a total of 13 implementations of this hook right now), so bad flag day. And also a bit leaky abstraction. Hence just do it function-by-function. Cc: linux-media(a)vger.kernel.org Cc: linaro-mm-sig(a)lists.linaro.org Cc: linux-rdma(a)vger.kernel.org Cc: amd-gfx(a)lists.freedesktop.org Cc: intel-gfx(a)lists.freedesktop.org Cc: Chris Wilson <chris(a)chris-wilson.co.uk> Cc: Maarten Lankhorst <maarten.lankhorst(a)linux.intel.com> Cc: Christian König <christian.koenig(a)amd.com> Signed-off-by: Daniel Vetter <daniel.vetter(a)intel.com> --- drivers/gpu/drm/drm_atomic_helper.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 549a31e6042c..23013209d4bf 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -1567,6 +1567,7 @@ EXPORT_SYMBOL(drm_atomic_helper_wait_for_flip_done); void drm_atomic_helper_commit_tail(struct drm_atomic_state *old_state) { struct drm_device *dev = old_state->dev; + bool fence_cookie = dma_fence_begin_signalling(); drm_atomic_helper_commit_modeset_disables(dev, old_state); @@ -1578,6 +1579,8 @@ void drm_atomic_helper_commit_tail(struct drm_atomic_state *old_state) drm_atomic_helper_commit_hw_done(old_state); + dma_fence_end_signalling(fence_cookie); + drm_atomic_helper_wait_for_vblanks(dev, old_state); drm_atomic_helper_cleanup_planes(dev, old_state); @@ -1597,6 +1600,7 @@ EXPORT_SYMBOL(drm_atomic_helper_commit_tail); void drm_atomic_helper_commit_tail_rpm(struct drm_atomic_state *old_state) { struct drm_device *dev = old_state->dev; + bool fence_cookie = dma_fence_begin_signalling(); drm_atomic_helper_commit_modeset_disables(dev, old_state); @@ -1609,6 +1613,8 @@ void drm_atomic_helper_commit_tail_rpm(struct drm_atomic_state *old_state) drm_atomic_helper_commit_hw_done(old_state); + dma_fence_end_signalling(fence_cookie); + drm_atomic_helper_wait_for_vblanks(dev, old_state); drm_atomic_helper_cleanup_planes(dev, old_state); @@ -1624,6 +1630,9 @@ static void commit_tail(struct drm_atomic_state *old_state) ktime_t start; s64 commit_time_ms; unsigned int i, new_self_refresh_mask = 0; + bool fence_cookie; + + fence_cookie = dma_fence_begin_signalling(); funcs = dev->mode_config.helper_private; @@ -1652,6 +1661,8 @@ static void commit_tail(struct drm_atomic_state *old_state) if (new_crtc_state->self_refresh_active) new_self_refresh_mask |= BIT(i); + dma_fence_end_signalling(fence_cookie); + if (funcs && funcs->atomic_commit_tail) funcs->atomic_commit_tail(old_state); else @@ -1810,6 +1821,7 @@ int drm_atomic_helper_commit(struct drm_device *dev, bool nonblock) { int ret; + bool fence_cookie; if (state->async_update) { ret = drm_atomic_helper_prepare_planes(dev, state); @@ -1832,6 +1844,8 @@ int drm_atomic_helper_commit(struct drm_device *dev, if (ret) return ret; + fence_cookie = dma_fence_begin_signalling(); + if (!nonblock) { ret = drm_atomic_helper_wait_for_fences(dev, state, true); if (ret) @@ -1869,6 +1883,7 @@ int drm_atomic_helper_commit(struct drm_device *dev, */ drm_atomic_state_get(state); + dma_fence_end_signalling(fence_cookie); if (nonblock) queue_work(system_unbound_wq, &state->commit_work); else @@ -1877,6 +1892,7 @@ int drm_atomic_helper_commit(struct drm_device *dev, return 0; err: + dma_fence_end_signalling(fence_cookie); drm_atomic_helper_cleanup_planes(dev, state); return ret; } -- 2.28.0

4 years, 12 months

[PATCH v3 00/23] drm/msm: de-struct_mutex-ification

by Rob Clark

From: Rob Clark <robdclark(a)chromium.org> This doesn't remove *all* the struct_mutex, but it covers the worst of it, ie. shrinker/madvise/free/retire. The submit path still uses struct_mutex, but it still needs *something* serialize a portion of the submit path, and lock_stat mostly just shows the lock contention there being with other submits. And there are a few other bits of struct_mutex usage in less critical paths (debugfs, etc). But this seems like a reasonable step in the right direction. v2: teach lockdep about shrinker locking patters (danvet) and convert to obj->resv locking (danvet) v3: fix get_vaddr locking for legacy userspace (relocs), devcoredump, and rd/hangrd Rob Clark (23): drm/msm: Fix a couple incorrect usages of get_vaddr_active() drm/msm/gem: Add obj->lock wrappers drm/msm/gem: Rename internal get_iova_locked helper drm/msm/gem: Move prototypes to msm_gem.h drm/msm/gem: Add some _locked() helpers drm/msm/gem: Move locking in shrinker path drm/msm/submit: Move copy_from_user ahead of locking bos drm/msm: Do rpm get sooner in the submit path drm/msm/gem: Switch over to obj->resv for locking drm/msm: Use correct drm_gem_object_put() in fail case drm/msm: Drop chatty trace drm/msm: Move update_fences() drm/msm: Add priv->mm_lock to protect active/inactive lists drm/msm: Document and rename preempt_lock drm/msm: Protect ring->submits with it's own lock drm/msm: Refcount submits drm/msm: Remove obj->gpu drm/msm: Drop struct_mutex from the retire path drm/msm: Drop struct_mutex in free_object() path drm/msm: Remove msm_gem_free_work drm/msm: Drop struct_mutex in madvise path drm/msm: Drop struct_mutex in shrinker path drm/msm: Don't implicit-sync if only a single ring drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 6 +- drivers/gpu/drm/msm/adreno/a5xx_preempt.c | 12 +- drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 6 +- drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c | 1 + drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 1 + drivers/gpu/drm/msm/dsi/dsi_host.c | 1 + drivers/gpu/drm/msm/msm_debugfs.c | 7 + drivers/gpu/drm/msm/msm_drv.c | 21 +- drivers/gpu/drm/msm/msm_drv.h | 73 +----- drivers/gpu/drm/msm/msm_fbdev.c | 1 + drivers/gpu/drm/msm/msm_gem.c | 266 +++++++++++----------- drivers/gpu/drm/msm/msm_gem.h | 133 +++++++++-- drivers/gpu/drm/msm/msm_gem_shrinker.c | 81 ++----- drivers/gpu/drm/msm/msm_gem_submit.c | 158 ++++++++----- drivers/gpu/drm/msm/msm_gpu.c | 110 +++++---- drivers/gpu/drm/msm/msm_gpu.h | 5 +- drivers/gpu/drm/msm/msm_rd.c | 2 +- drivers/gpu/drm/msm/msm_ringbuffer.c | 3 +- drivers/gpu/drm/msm/msm_ringbuffer.h | 13 +- 19 files changed, 495 insertions(+), 405 deletions(-) -- 2.26.2

4 years, 12 months

Re: [Linaro-mm-sig] WARNING in dma_map_page_attrs

by Christoph Hellwig

I don't think the merge commit makes sense here. But what we see here is that dma_map_page is called on the rxe device, without that device having a DMA mask. For now this needs a workaround in rxe, but for 5.11 I'll send a patch to remove dma-virt and just handle this case inside of the rdma core. On Wed, Oct 21, 2020 at 12:03:19PM -0700, syzbot wrote: > Hello, > > syzbot found the following issue on: > > HEAD commit: c4d6fe73 Merge tag 'xarray-5.9' of git://git.infradead.org.. > git tree: upstream > console output: https://syzkaller.appspot.com/x/log.txt?x=14862ff0500000 > kernel config: https://syzkaller.appspot.com/x/.config?x=7d790573d3e379c4 > dashboard link: https://syzkaller.appspot.com/bug?extid=34dc2fea3478e659af01 > compiler: gcc (GCC) 10.1.0-syz 20200507 > > 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+34dc2fea3478e659af01(a)syzkaller.appspotmail.com > > infiniband syz1: set active > infiniband syz1: added vcan0 > ------------[ cut here ]------------ > WARNING: CPU: 1 PID: 9851 at kernel/dma/mapping.c:149 dma_map_page_attrs+0x493/0x700 kernel/dma/mapping.c:149 > Modules linked in: > CPU: 1 PID: 9851 Comm: syz-executor.1 Not tainted 5.9.0-syzkaller #0 > Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.12.0-59-gc9ba5276e321-prebuilt.qemu.org 04/01/2014 > RIP: 0010:dma_map_page_attrs+0x493/0x700 kernel/dma/mapping.c:149 > Code: 80 3c 10 00 0f 85 ed 01 00 00 48 8b 1d 36 c3 fa 0c e9 2d fc ff ff 48 89 c3 e9 d1 fd ff ff e8 04 12 12 00 0f 0b e8 fd 11 12 00 <0f> 0b 49 c7 c4 ff ff ff ff e9 d5 fd ff ff e8 ea 11 12 00 48 8d 7b > RSP: 0018:ffffc90001546c68 EFLAGS: 00010246 > RAX: 0000000000040000 RBX: ffffffff894d0040 RCX: ffffc9000dbe4000 > RDX: 0000000000040000 RSI: ffffffff815d3b03 RDI: ffff88806a988b00 > RBP: ffff8880236cc400 R08: 0000000000000002 R09: 0000000000000000 > R10: 0000000000000002 R11: 0000000000000000 R12: ffffea00008db300 > R13: ffff88806a9886e8 R14: 00000000000004b8 R15: 0000000000000002 > FS: 00007f678fae2700(0000) GS:ffff88802ce00000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 00007f299a39b190 CR3: 0000000069f31000 CR4: 0000000000350ee0 > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > Call Trace: > dma_map_single_attrs include/linux/dma-mapping.h:279 [inline] > ib_dma_map_single include/rdma/ib_verbs.h:3967 [inline] > ib_mad_post_receive_mads+0x23f/0xd60 drivers/infiniband/core/mad.c:2715 > ib_mad_port_start drivers/infiniband/core/mad.c:2862 [inline] > ib_mad_port_open drivers/infiniband/core/mad.c:3016 [inline] > ib_mad_init_device+0x72b/0x1400 drivers/infiniband/core/mad.c:3092 > add_client_context+0x405/0x5e0 drivers/infiniband/core/device.c:680 > enable_device_and_get+0x1d5/0x3c0 drivers/infiniband/core/device.c:1301 > ib_register_device drivers/infiniband/core/device.c:1376 [inline] > ib_register_device+0x7a7/0xa40 drivers/infiniband/core/device.c:1335 > rxe_register_device+0x46d/0x570 drivers/infiniband/sw/rxe/rxe_verbs.c:1182 > rxe_add+0x12fe/0x16d0 drivers/infiniband/sw/rxe/rxe.c:247 > rxe_net_add+0x8c/0xe0 drivers/infiniband/sw/rxe/rxe_net.c:507 > rxe_newlink drivers/infiniband/sw/rxe/rxe.c:269 [inline] > rxe_newlink+0xb7/0xe0 drivers/infiniband/sw/rxe/rxe.c:250 > nldev_newlink+0x30e/0x540 drivers/infiniband/core/nldev.c:1555 > rdma_nl_rcv_msg+0x367/0x690 drivers/infiniband/core/netlink.c:195 > rdma_nl_rcv_skb drivers/infiniband/core/netlink.c:239 [inline] > rdma_nl_rcv+0x2f2/0x440 drivers/infiniband/core/netlink.c:259 > netlink_unicast_kernel net/netlink/af_netlink.c:1304 [inline] > netlink_unicast+0x533/0x7d0 net/netlink/af_netlink.c:1330 > netlink_sendmsg+0x856/0xd90 net/netlink/af_netlink.c:1919 > sock_sendmsg_nosec net/socket.c:651 [inline] > sock_sendmsg+0xcf/0x120 net/socket.c:671 > ____sys_sendmsg+0x6e8/0x810 net/socket.c:2353 > ___sys_sendmsg+0xf3/0x170 net/socket.c:2407 > __sys_sendmsg+0xe5/0x1b0 net/socket.c:2440 > do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46 > entry_SYSCALL_64_after_hwframe+0x44/0xa9 > RIP: 0033:0x45d9f9 > Code: bd b1 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 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 0f 83 8b b1 fb ff c3 66 2e 0f 1f 84 00 00 00 00 > RSP: 002b:00007f678fae1c88 EFLAGS: 00000246 ORIG_RAX: 000000000000002e > RAX: ffffffffffffffda RBX: 000000000071f480 RCX: 000000000045d9f9 > RDX: 0000000000000000 RSI: 0000000020000200 RDI: 0000000000000003 > RBP: 00000000004aab13 R08: 0000000000000000 R09: 0000000000000000 > R10: 0000000000000000 R11: 0000000000000246 R12: 000000000075bf00 > R13: 00007ffc6f9b8bbf R14: 00007f678fac2000 R15: 0000000000000003 > > > --- > 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. ---end quoted text---

5 years

Re: [Linaro-mm-sig] [PATCH] staging: ion: remove from the tree

by Greg Kroah-Hartman

On Thu, Aug 27, 2020 at 09:31:27AM -0400, Laura Abbott wrote: > On 8/27/20 8:36 AM, Greg Kroah-Hartman wrote: > > The ION android code has long been marked to be removed, now that we > > dma-buf support merged into the real part of the kernel. > > > > It was thought that we could wait to remove the ion kernel at a later > > time, but as the out-of-tree Android fork of the ion code has diverged > > quite a bit, and any Android device using the ion interface uses that > > forked version and not this in-tree version, the in-tree copy of the > > code is abandonded and not used by anyone. > > > > Combine this abandoned codebase with the need to make changes to it in > > order to keep the kernel building properly, which then causes merge > > issues when merging those changes into the out-of-tree Android code, and > > you end up with two different groups of people (the in-kernel-tree > > developers, and the Android kernel developers) who are both annoyed at > > the current situation. Because of this problem, just drop the in-kernel > > copy of the ion code now, as it's not used, and is only causing problems > > for everyone involved. > > > > Cc: "Arve Hjønnevåg" <arve(a)android.com> > > Cc: "Christian König" <christian.koenig(a)amd.com> > > Cc: Christian Brauner <christian(a)brauner.io> > > Cc: Christoph Hellwig <hch(a)infradead.org> > > Cc: Hridya Valsaraju <hridya(a)google.com> > > Cc: Joel Fernandes <joel(a)joelfernandes.org> > > Cc: John Stultz <john.stultz(a)linaro.org> > > Cc: Laura Abbott <laura(a)labbott.name> > > Cc: Martijn Coenen <maco(a)android.com> > > Cc: Shuah Khan <shuah(a)kernel.org> > > Cc: Sumit Semwal <sumit.semwal(a)linaro.org> > > Cc: Suren Baghdasaryan <surenb(a)google.com> > > Cc: Todd Kjos <tkjos(a)android.com> > > Cc: devel(a)driverdev.osuosl.org > > Cc: dri-devel(a)lists.freedesktop.org > > Cc: linaro-mm-sig(a)lists.linaro.org > > Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> > > We discussed this at the Android MC on Monday and the plan was to > remove it after the next LTS release. As 5.10 will be the next LTS release, I have now merged it to my "testing" branch to go into 5.11-rc1. thanks, greg k-h

5 years

Re: [Linaro-mm-sig] [PATCH v4 5/5] dma-buf: Clarify that dma-buf sg lists are page aligned

by Daniel Vetter

On Wed, Oct 14, 2020 at 09:16:01AM -0700, Jianxin Xiong wrote: > The dma-buf API have been used under the assumption that the sg lists > returned from dma_buf_map_attachment() are fully page aligned. Lots of > stuff can break otherwise all over the place. Clarify this in the > documentation and add a check when DMA API debug is enabled. > > Signed-off-by: Jianxin Xiong <jianxin.xiong(a)intel.com> lgtm, thanks for creating this and giving it a spin. I'll queue this up in drm-misc-next for 5.11, should show up in linux-next after the merge window is closed. Cheers, Daniel > --- > drivers/dma-buf/dma-buf.c | 21 +++++++++++++++++++++ > include/linux/dma-buf.h | 3 ++- > 2 files changed, 23 insertions(+), 1 deletion(-) > > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c > index 844967f..7309c83 100644 > --- a/drivers/dma-buf/dma-buf.c > +++ b/drivers/dma-buf/dma-buf.c > @@ -851,6 +851,9 @@ void dma_buf_unpin(struct dma_buf_attachment *attach) > * Returns sg_table containing the scatterlist to be returned; returns ERR_PTR > * on error. May return -EINTR if it is interrupted by a signal. > * > + * On success, the DMA addresses and lengths in the returned scatterlist are > + * PAGE_SIZE aligned. > + * > * A mapping must be unmapped by using dma_buf_unmap_attachment(). Note that > * the underlying backing storage is pinned for as long as a mapping exists, > * therefore users/importers should not hold onto a mapping for undue amounts of > @@ -904,6 +907,24 @@ struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach, > attach->dir = direction; > } > > +#ifdef CONFIG_DMA_API_DEBUG > + { > + struct scatterlist *sg; > + u64 addr; > + int len; > + int i; > + > + for_each_sgtable_dma_sg(sg_table, sg, i) { > + addr = sg_dma_address(sg); > + len = sg_dma_len(sg); > + if (!PAGE_ALIGNED(addr) || !PAGE_ALIGNED(len)) { > + pr_debug("%s: addr %llx or len %x is not page aligned!\n", > + __func__, addr, len); > + } > + } > + } > +#endif /* CONFIG_DMA_API_DEBUG */ > + > return sg_table; > } > EXPORT_SYMBOL_GPL(dma_buf_map_attachment); > diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h > index a2ca294e..4a5fa70 100644 > --- a/include/linux/dma-buf.h > +++ b/include/linux/dma-buf.h > @@ -145,7 +145,8 @@ struct dma_buf_ops { > * > * A &sg_table scatter list of or the backing storage of the DMA buffer, > * already mapped into the device address space of the &device attached > - * with the provided &dma_buf_attachment. > + * with the provided &dma_buf_attachment. The addresses and lengths in > + * the scatter list are PAGE_SIZE aligned. > * > * On failure, returns a negative error value wrapped into a pointer. > * May also return -EINTR when a signal was received while being > -- > 1.8.3.1 > > _______________________________________________ > dri-devel mailing list > dri-devel(a)lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch

5 years

[PATCH 1/6] mm: mmap: fix fput in error path

by Christian König

Patch "495c10cc1c0c CHROMIUM: dma-buf: restore args..." adds a workaround for a bug in mmap_region. As the comment states ->mmap() callback can change vma->vm_file and so we might call fput() on the wrong file. Revert the workaround and proper fix this in mmap_region. Signed-off-by: Christian König <christian.koenig(a)amd.com> --- drivers/dma-buf/dma-buf.c | 22 +++++----------------- mm/mmap.c | 2 +- 2 files changed, 6 insertions(+), 18 deletions(-) diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index a6ba4d598f0e..edd57402a48a 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -1143,9 +1143,6 @@ EXPORT_SYMBOL_GPL(dma_buf_end_cpu_access); int dma_buf_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma, unsigned long pgoff) { - struct file *oldfile; - int ret; - if (WARN_ON(!dmabuf || !vma)) return -EINVAL; @@ -1163,22 +1160,13 @@ int dma_buf_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma, return -EINVAL; /* readjust the vma */ - get_file(dmabuf->file); - oldfile = vma->vm_file; - vma->vm_file = dmabuf->file; - vma->vm_pgoff = pgoff; + if (vma->vm_file) + fput(vma->vm_file); - ret = dmabuf->ops->mmap(dmabuf, vma); - if (ret) { - /* restore old parameters on failure */ - vma->vm_file = oldfile; - fput(dmabuf->file); - } else { - if (oldfile) - fput(oldfile); - } - return ret; + vma->vm_file = get_file(dmabuf->file); + vma->vm_pgoff = pgoff; + return dmabuf->ops->mmap(dmabuf, vma); } EXPORT_SYMBOL_GPL(dma_buf_mmap); diff --git a/mm/mmap.c b/mm/mmap.c index 40248d84ad5f..3a2670d73355 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -1852,8 +1852,8 @@ unsigned long mmap_region(struct file *file, unsigned long addr, return addr; unmap_and_free_vma: + fput(vma->vm_file); vma->vm_file = NULL; - fput(file); /* Undo any partial mapping done by a device driver. */ unmap_region(mm, vma, prev, vma->vm_start, vma->vm_end); -- 2.17.1

5 years

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig