From: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
To: Daniel Vetter <daniel.vetter@ffwll.ch>,
DRI Development <dri-devel@lists.freedesktop.org>
Cc: Lokesh Vutla <lokeshvutla@ti.com>,
Daniel Vetter <daniel.vetter@intel.com>
Subject: Re: [PATCH 11/11] drm/tidss: Annotate dma-fence critical section in commit path
Date: Thu, 27 May 2021 15:15:34 +0300 [thread overview]
Message-ID: <7c07a183-cd1b-ab75-527c-cdf37cd4273c@ideasonboard.com> (raw)
In-Reply-To: <20210121152959.1725404-12-daniel.vetter@ffwll.ch>
Hi Daniel,
On 21/01/2021 17:29, Daniel Vetter wrote:
> Ends right after hw_done(), totally standard case.
>
> Acked-by: Jyri Sarha <jsarha@ti.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Jyri Sarha <jsarha@ti.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
> drivers/gpu/drm/tidss/tidss_kms.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/tidss/tidss_kms.c b/drivers/gpu/drm/tidss/tidss_kms.c
> index 09485c7f0d6f..95f8e0f78e32 100644
> --- a/drivers/gpu/drm/tidss/tidss_kms.c
> +++ b/drivers/gpu/drm/tidss/tidss_kms.c
> @@ -4,6 +4,8 @@
> * Author: Tomi Valkeinen <tomi.valkeinen@ti.com>
> */
>
> +#include <linux/dma-fence.h>
> +
> #include <drm/drm_atomic.h>
> #include <drm/drm_atomic_helper.h>
> #include <drm/drm_bridge.h>
> @@ -26,6 +28,7 @@ static void tidss_atomic_commit_tail(struct drm_atomic_state *old_state)
> {
> struct drm_device *ddev = old_state->dev;
> struct tidss_device *tidss = to_tidss(ddev);
> + bool fence_cookie = dma_fence_begin_signalling();
>
> dev_dbg(ddev->dev, "%s\n", __func__);
>
> @@ -36,6 +39,7 @@ static void tidss_atomic_commit_tail(struct drm_atomic_state *old_state)
> drm_atomic_helper_commit_modeset_enables(ddev, old_state);
>
> drm_atomic_helper_commit_hw_done(old_state);
> + dma_fence_end_signalling(fence_cookie);
> drm_atomic_helper_wait_for_flip_done(ddev, old_state);
>
> drm_atomic_helper_cleanup_planes(ddev, old_state);
>
I bisected v5.13 rc lockdep warnings to this patch. I see this with tidss (lockdep
report below) and omapdrm (probably caused by "drm/omapdrm: Annotate dma-fence critical
section in commit path"). The report on omapdrm is very similar, with fs_reclaim and
dma_fence_map, but hdmi bridge instead of mhdp bridge.
I'm unfamiliar with this piece of drm code, do you have any hints on what the problem
might be and where I should be looking at?
Tomi
[ 20.769286] ======================================================
[ 20.776918] WARNING: possible circular locking dependency detected
[ 20.783082] 5.11.0-rc2-00688-g4d56a4f08391-dirty #18 Not tainted
[ 20.789072] ------------------------------------------------------
[ 20.795232] kmstest/397 is trying to acquire lock:
[ 20.800008] ffff800011637878 (fs_reclaim){+.+.}-{0:0}, at: fs_reclaim_acquire+0x40/0xa4
[ 20.808014]
[ 20.808014] but task is already holding lock:
[ 20.813828] ffff000804fb41a8 (&mhdp->link_mutex){+.+.}-{3:3}, at: cdns_mhdp_atomic_enable+0x54/0x2c0 [cdns_mhdp8546]
[ 20.824343]
[ 20.824343] which lock already depends on the new lock.
[ 20.824343]
[ 20.832497]
[ 20.832497] the existing dependency chain (in reverse order) is:
[ 20.839958]
[ 20.839958] -> #2 (&mhdp->link_mutex){+.+.}-{3:3}:
[ 20.846214] lock_acquire.part.0+0x178/0x380
[ 20.850999] lock_acquire+0x6c/0x90
[ 20.854996] __mutex_lock+0x9c/0x540
[ 20.859083] mutex_lock_nested+0x44/0x70
[ 20.863513] cdns_mhdp_atomic_enable+0x54/0x2c0 [cdns_mhdp8546]
[ 20.869938] drm_atomic_bridge_chain_enable+0x60/0xd4 [drm]
[ 20.876093] drm_atomic_helper_commit_modeset_enables+0x148/0x260 [drm_kms_helper]
[ 20.884207] tidss_atomic_commit_tail+0x64/0xd0 [tidss]
[ 20.889947] commit_tail+0xac/0x190 [drm_kms_helper]
[ 20.895445] drm_atomic_helper_commit+0x194/0x3a0 [drm_kms_helper]
[ 20.902155] drm_atomic_commit+0x58/0x70 [drm]
[ 20.907165] drm_mode_atomic_ioctl+0x9f0/0xbbc [drm]
[ 20.912693] drm_ioctl_kernel+0xd0/0x124 [drm]
[ 20.917701] drm_ioctl+0x238/0x460 [drm]
[ 20.922189] __arm64_sys_ioctl+0xb4/0x100
[ 20.926707] el0_svc_common.constprop.0+0x80/0x1e0
[ 20.932007] do_el0_svc+0x30/0xa0
[ 20.935831] el0_svc+0x20/0x30
[ 20.939394] el0_sync_handler+0x1a8/0x1b0
[ 20.943910] el0_sync+0x174/0x180
[ 20.947734]
[ 20.947734] -> #1 (dma_fence_map){++++}-{0:0}:
[ 20.953644] lock_acquire.part.0+0x178/0x380
[ 20.958422] lock_acquire+0x6c/0x90
[ 20.962418] __dma_fence_might_wait+0x60/0xdc
[ 20.967285] dma_resv_lockdep+0x1dc/0x2b4
[ 20.971803] do_one_initcall+0x90/0x460
[ 20.976145] kernel_init_freeable+0x2c0/0x32c
[ 20.981012] kernel_init+0x20/0x128
[ 20.985007] ret_from_fork+0x10/0x3c
[ 20.989091]
[ 20.989091] -> #0 (fs_reclaim){+.+.}-{0:0}:
[ 20.994740] check_noncircular+0x164/0x180
[ 20.999344] __lock_acquire+0x13dc/0x1be4
[ 21.003861] lock_acquire.part.0+0x178/0x380
[ 21.008639] lock_acquire+0x6c/0x90
[ 21.012635] fs_reclaim_acquire+0x7c/0xa4
[ 21.017152] kmem_cache_alloc_trace+0x7c/0x3c0
[ 21.022103] drm_mode_duplicate+0x34/0x70 [drm]
[ 21.027199] cdns_mhdp_atomic_enable+0x1c4/0x2c0 [cdns_mhdp8546]
[ 21.033711] drm_atomic_bridge_chain_enable+0x60/0xd4 [drm]
[ 21.039845] drm_atomic_helper_commit_modeset_enables+0x148/0x260 [drm_kms_helper]
[ 21.047946] tidss_atomic_commit_tail+0x64/0xd0 [tidss]
[ 21.053681] commit_tail+0xac/0x190 [drm_kms_helper]
[ 21.059179] drm_atomic_helper_commit+0x194/0x3a0 [drm_kms_helper]
[ 21.065889] drm_atomic_commit+0x58/0x70 [drm]
[ 21.070897] drm_mode_atomic_ioctl+0x9f0/0xbbc [drm]
[ 21.076423] drm_ioctl_kernel+0xd0/0x124 [drm]
[ 21.081430] drm_ioctl+0x238/0x460 [drm]
[ 21.085917] __arm64_sys_ioctl+0xb4/0x100
[ 21.090434] el0_svc_common.constprop.0+0x80/0x1e0
[ 21.095730] do_el0_svc+0x30/0xa0
[ 21.099554] el0_svc+0x20/0x30
[ 21.103117] el0_sync_handler+0x1a8/0x1b0
[ 21.107632] el0_sync+0x174/0x180
[ 21.111454]
[ 21.111454] other info that might help us debug this:
[ 21.111454]
[ 21.119435] Chain exists of:
[ 21.119435] fs_reclaim --> dma_fence_map --> &mhdp->link_mutex
[ 21.119435]
[ 21.129768] Possible unsafe locking scenario:
[ 21.129768]
[ 21.135670] CPU0 CPU1
[ 21.140184] ---- ----
[ 21.144698] lock(&mhdp->link_mutex);
[ 21.148435] lock(dma_fence_map);
[ 21.154340] lock(&mhdp->link_mutex);
[ 21.160591] lock(fs_reclaim);
[ 21.163721]
[ 21.163721] *** DEADLOCK ***
[ 21.163721]
[ 21.169623] 4 locks held by kmstest/397:
[ 21.173532] #0: ffff8000145efc28 (crtc_ww_class_acquire){+.+.}-{0:0}, at: drm_modeset_acquire_init+0x48/0x70 [drm]
[ 21.184011] #1: ffff000805dc7888 (crtc_ww_class_mutex){+.+.}-{3:3}, at: drm_modeset_lock+0x138/0x170 [drm]
[ 21.193795] #2: ffff8000116d53f8 (dma_fence_map){++++}-{0:0}, at: tidss_atomic_commit_tail+0x2c/0xd0 [tidss]
[ 21.203701] #3: ffff000804fb41a8 (&mhdp->link_mutex){+.+.}-{3:3}, at: cdns_mhdp_atomic_enable+0x54/0x2c0 [cdns_mhdp8546]
[ 21.214642]
[ 21.214642] stack backtrace:
[ 21.218984] CPU: 1 PID: 397 Comm: kmstest Not tainted 5.11.0-rc2-00688-g4d56a4f08391-dirty #18
[ 21.227575] Hardware name: Texas Instruments K3 J721E SoC (DT)
[ 21.233390] Call trace:
[ 21.235825] dump_backtrace+0x0/0x1f0
[ 21.239476] show_stack+0x24/0x80
[ 21.242781] dump_stack+0xec/0x154
[ 21.246170] print_circular_bug+0x1f8/0x200
[ 21.250341] check_noncircular+0x164/0x180
[ 21.254426] __lock_acquire+0x13dc/0x1be4
[ 21.258422] lock_acquire.part.0+0x178/0x380
[ 21.262679] lock_acquire+0x6c/0x90
[ 21.266155] fs_reclaim_acquire+0x7c/0xa4
[ 21.270152] kmem_cache_alloc_trace+0x7c/0x3c0
[ 21.274582] drm_mode_duplicate+0x34/0x70 [drm]
[ 21.279159] cdns_mhdp_atomic_enable+0x1c4/0x2c0 [cdns_mhdp8546]
[ 21.285151] drm_atomic_bridge_chain_enable+0x60/0xd4 [drm]
[ 21.290764] drm_atomic_helper_commit_modeset_enables+0x148/0x260 [drm_kms_helper]
[ 21.298343] tidss_atomic_commit_tail+0x64/0xd0 [tidss]
[ 21.303559] commit_tail+0xac/0x190 [drm_kms_helper]
[ 21.308535] drm_atomic_helper_commit+0x194/0x3a0 [drm_kms_helper]
[ 21.314725] drm_atomic_commit+0x58/0x70 [drm]
[ 21.319214] drm_mode_atomic_ioctl+0x9f0/0xbbc [drm]
[ 21.324220] drm_ioctl_kernel+0xd0/0x124 [drm]
[ 21.328708] drm_ioctl+0x238/0x460 [drm]
[ 21.332676] __arm64_sys_ioctl+0xb4/0x100
[ 21.336672] el0_svc_common.constprop.0+0x80/0x1e0
[ 21.341449] do_el0_svc+0x30/0xa0
[ 21.344753] el0_svc+0x20/0x30
[ 21.347795] el0_sync_handler+0x1a8/0x1b0
[ 21.351790] el0_sync+0x174/0x180
next prev parent reply other threads:[~2021-05-27 12:15 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-21 15:29 [PATCH 00/11] dma_fence critical sections annotations for atomic Daniel Vetter
2021-01-21 15:29 ` [PATCH 01/11] drm/atomic-helper: Add dma-fence annotations Daniel Vetter
2021-01-21 15:29 ` [PATCH 02/11] drm/vkms: Annotate vblank timer Daniel Vetter
2021-01-21 15:29 ` [PATCH 03/11] drm/vblank: Annotate with dma-fence signalling section Daniel Vetter
2021-01-21 15:29 ` [PATCH 04/11] drm/komeda: Annotate dma-fence critical section in commit path Daniel Vetter
2021-01-21 15:29 ` [PATCH 05/11] drm/malidp: " Daniel Vetter
2021-01-21 15:29 ` [PATCH 06/11] drm/atmel: Use drm_atomic_helper_commit Daniel Vetter
2021-01-21 15:29 ` [PATCH 07/11] drm/imx: Annotate dma-fence critical section in commit path Daniel Vetter
2021-01-21 15:29 ` [PATCH 08/11] drm/omapdrm: " Daniel Vetter
2021-01-21 15:29 ` [PATCH 09/11] drm/rcar-du: " Daniel Vetter
2021-01-21 15:29 ` [PATCH 10/11] drm/tegra: " Daniel Vetter
2021-02-05 18:09 ` Thierry Reding
2021-01-21 15:29 ` [PATCH 11/11] drm/tidss: " Daniel Vetter
2021-05-27 12:15 ` Tomi Valkeinen [this message]
2021-06-22 16:31 ` Daniel Vetter
2021-02-23 11:54 ` [PATCH 00/11] dma_fence critical sections annotations for atomic Daniel Vetter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7c07a183-cd1b-ab75-527c-cdf37cd4273c@ideasonboard.com \
--to=tomi.valkeinen@ideasonboard.com \
--cc=daniel.vetter@ffwll.ch \
--cc=daniel.vetter@intel.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=lokeshvutla@ti.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox