From: Geert Uytterhoeven <geert@linux-m68k.org>
To: Saravana Kannan <saravanak@google.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
"Rafael J. Wysocki" <rafael@kernel.org>,
Marek Szyprowski <m.szyprowski@samsung.com>,
Marc Zyngier <maz@kernel.org>,
Android Kernel Team <kernel-team@android.com>,
LKML <linux-kernel@vger.kernel.org>
Subject: Re: [TEST PATCH v1] driver: core: Make fw_devlink=on more forgiving
Date: Thu, 21 Jan 2021 11:37:49 +0100 [thread overview]
Message-ID: <CAMuHMdVs6UM-jvQh8kgp9x9n_rBt36ToOik44oVwKD_kkqkCVg@mail.gmail.com> (raw)
In-Reply-To: <CAGETcx9+JbVAX4uXGn5hYSd-1VK8oZeLxhGAKZksyR31xpeQhg@mail.gmail.com>
Hi Saravana,
On Thu, Jan 21, 2021 at 9:28 AM Saravana Kannan <saravanak@google.com> wrote:
> On Thu, Jan 21, 2021 at 12:22 AM Saravana Kannan <saravanak@google.com> wrote:
> > This patch is for test purposes only and pretty experimental. Code might
> > not be optimized, clean, formatted properly, etc.
> >
> > Please review it only for functional bugs like locking bugs, wrong
> > logic, etc.
> >
> > It's basically trying to figure out which devices will never probe and
> > ignore them. Might not always work.
> >
> > Marek, Geert, Marc,
> >
> > Can you please try this patch INSTEAD of the other workarounds we found?
Thanks for your patch!
> Oh and can you please also try with the CONFIG_MODULES enabled vs
> disabled? Or have it disabled but fix the patch so the condition
> always evaluates to true.
With CONFIG_MODULES=y, it fails in the same way as before (no "Deleting
{fwnode,dev} link" messages seen), with a new lockdep warning:
+======================================================
+WARNING: possible circular locking dependency detected
+5.11.0-rc2-salvator-x-00009-gdf1dd3208a90 #941 Not tainted
+------------------------------------------------------
+swapper/0/1 is trying to acquire lock:
+ffffffc0110da488 (fwnode_link_lock){+.+.}-{4:4}, at: fw_devlink_unblock_probe+0
x50/0x158
+
+but task is already holding lock:
+ffffffc0110da988 (deferred_probe_mutex){+.+.}-{4:4}, at: deferred_probe_initcal
l+0xe4/0x12c
+
+which lock already depends on the new lock.
+
+
+the existing dependency chain (in reverse order) is:
+
+-> #2 (deferred_probe_mutex){+.+.}-{4:4}:
+ lock_acquire+0x344/0x390
+ __mutex_lock+0xc0/0x37c
+ mutex_lock_nested+0x34/0x48
+ driver_deferred_probe_add+0x2c/0x88
+ device_links_driver_bound+0x11c/0x1ac
+ driver_bound+0x64/0xac
+ really_probe+0x304/0x338
+ driver_probe_device+0x98/0xa8
+ device_driver_attach+0x40/0x68
+ __driver_attach+0xa8/0xac
+ bus_for_each_dev+0x6c/0xb8
+ driver_attach+0x20/0x28
+ bus_add_driver+0x16c/0x1b0
+ driver_register+0xac/0xe4
+ __platform_driver_probe+0x88/0xe0
+ cpg_mssr_init+0x20/0x28
+ do_one_initcall+0xf0/0x280
+ kernel_init_freeable+0x1e0/0x1e4
+ kernel_init+0x10/0x108
+ ret_from_fork+0x10/0x18
+
+-> #1 (device_links_lock){+.+.}-{4:4}:
+ lock_acquire+0x344/0x390
+ __mutex_lock+0xc0/0x37c
+ mutex_lock_nested+0x34/0x48
+ device_links_write_lock+0x18/0x20
+ device_link_add+0xfc/0x3e4
+ fw_devlink_create_devlink+0x40/0xec
+ device_add+0x640/0x6ac
+ of_device_add+0x38/0x40
+ of_platform_device_create_pdata+0xb0/0xcc
+ of_platform_bus_create+0x2b8/0x364
+ of_platform_bus_create+0x300/0x364
+ of_platform_populate+0x7c/0xd8
+ of_platform_default_populate+0x20/0x28
+ of_platform_default_populate_init+0x80/0xb8
+ do_one_initcall+0xf0/0x280
+ kernel_init_freeable+0x1e0/0x1e4
+ kernel_init+0x10/0x108
+ ret_from_fork+0x10/0x18
+
+-> #0 (fwnode_link_lock){+.+.}-{4:4}:
+ check_noncircular+0x74/0xa4
+ __lock_acquire+0xdd0/0x10a8
+ lock_acquire+0x344/0x390
+ __mutex_lock+0xc0/0x37c
+ mutex_lock_nested+0x34/0x48
+ fw_devlink_unblock_probe+0x50/0x158
+ deferred_probe_initcall+0x11c/0x12c
+ do_one_initcall+0xf0/0x280
+ kernel_init_freeable+0x1e0/0x1e4
+ kernel_init+0x10/0x108
+ ret_from_fork+0x10/0x18
+
+other info that might help us debug this:
+
+Chain exists of:
+ fwnode_link_lock --> device_links_lock --> deferred_probe_mutex
+
+ Possible unsafe locking scenario:
+
+ CPU0 CPU1
+ ---- ----
+ lock(deferred_probe_mutex);
+ lock(device_links_lock);
+ lock(deferred_probe_mutex);
+ lock(fwnode_link_lock);
+
+ *** DEADLOCK ***
+
+1 lock held by swapper/0/1:
+ #0: ffffffc0110da988 (deferred_probe_mutex){+.+.}-{4:4}, at:
deferred_probe_initcall+0xe4/0x12c
+
+stack backtrace:
+CPU: 2 PID: 1 Comm: swapper/0 Not tainted
5.11.0-rc2-salvator-x-00009-gdf1dd3208a90 #941
+Hardware name: Renesas Salvator-X 2nd version board based on r8a77951 (DT)
+Call trace:
+ dump_backtrace+0x0/0x188
+ show_stack+0x14/0x28
+ dump_stack+0xf0/0x140
+ print_circular_bug.isra.0+0x1b0/0x1e8
+ check_noncircular+0x74/0xa4
+ __lock_acquire+0xdd0/0x10a8
+ lock_acquire+0x344/0x390
+ __mutex_lock+0xc0/0x37c
+ mutex_lock_nested+0x34/0x48
+ fw_devlink_unblock_probe+0x50/0x158
+ deferred_probe_initcall+0x11c/0x12c
+ do_one_initcall+0xf0/0x280
+ kernel_init_freeable+0x1e0/0x1e4
+ kernel_init+0x10/0x108
+ ret_from_fork+0x10/0x18
With CONFIG_MODULES disabled, it deletes one link, and hangs:
+platform e61c0000.interrupt-controller: Deleting dev link to
e6180000.system-controller
+INFO: task swapper/0:1 blocked for more than 120 seconds.
+ Not tainted 5.11.0-rc2-salvator-x-00009-gdf1dd3208a90-dirty #944
+"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
+task:swapper/0 state:D stack: 0 pid: 1 ppid: 0 flags:0x00000008
+Call trace:
+ __switch_to+0xa8/0x10c
+ __schedule+0x54c/0x728
+ schedule+0x7c/0xc0
+ schedule_preempt_disabled+0x10/0x1c
+ __mutex_lock+0x1e8/0x37c
+ mutex_lock_nested+0x34/0x48
+ driver_deferred_probe_del+0x28/0x8c
+ device_del+0x198/0x30c
+ device_unregister+0x14/0x28
+ __device_link_del+0x4c/0x5c
+ device_link_drop_managed+0x44/0x50
+ fw_devlink_unblock_probe+0x1e8/0x230
+ deferred_probe_initcall+0x11c/0x12c
+ do_one_initcall+0xf0/0x240
+ kernel_init_freeable+0x1e0/0x1e4
+ kernel_init+0x10/0x108
+ ret_from_fork+0x10/0x18
+INFO: lockdep is turned off.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
next prev parent reply other threads:[~2021-01-21 10:43 UTC|newest]
Thread overview: 89+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-18 3:16 [PATCH v1 0/5] Enable fw_devlink=on by default Saravana Kannan
2020-12-18 3:16 ` [PATCH v1 1/5] driver core: Add debug logs for device link related probe deferrals Saravana Kannan
2020-12-18 3:17 ` [PATCH v1 2/5] driver core: Add device link support for INFERRED flag Saravana Kannan
2020-12-18 3:17 ` [PATCH v1 3/5] driver core: Have fw_devlink use DL_FLAG_INFERRED Saravana Kannan
2020-12-18 3:17 ` [PATCH v1 4/5] driver core: Handle cycles in device links created by fw_devlink Saravana Kannan
2020-12-18 6:39 ` kernel test robot
2020-12-18 6:39 ` [RFC PATCH] driver core: fw_devlink_relax_cycle() can be static kernel test robot
2020-12-18 6:48 ` [PATCH v1 4/5] driver core: Handle cycles in device links created by fw_devlink kernel test robot
2020-12-18 7:12 ` kernel test robot
2020-12-18 3:17 ` [PATCH v1 5/5] driver core: Set fw_devlink=on by default Saravana Kannan
[not found] ` <CGME20210111111245eucas1p15acde7ecc2ca7f7782beb8ed74c72022@eucas1p1.samsung.com>
2021-01-11 11:12 ` Marek Szyprowski
[not found] ` <CGME20210111141814eucas1p1f388df07b789693a999042b27f0d8c2a@eucas1p1.samsung.com>
2021-01-11 14:18 ` Marek Szyprowski
2021-01-11 21:47 ` Saravana Kannan
2021-01-12 7:11 ` Marek Szyprowski
2021-01-12 20:51 ` Saravana Kannan
2021-01-13 7:04 ` Marek Szyprowski
2021-01-13 19:23 ` Saravana Kannan
2021-01-14 7:36 ` Marek Szyprowski
2021-01-14 18:08 ` Saravana Kannan
2021-01-18 17:43 ` Geert Uytterhoeven
2021-01-17 23:01 ` Michael Walle
2021-01-18 21:01 ` Saravana Kannan
2021-01-19 10:41 ` Michael Walle
2021-01-20 0:00 ` Saravana Kannan
2021-01-18 17:39 ` Geert Uytterhoeven
2021-01-18 17:59 ` Marc Zyngier
2021-01-18 19:16 ` Geert Uytterhoeven
2021-01-18 19:30 ` Marc Zyngier
2021-01-18 21:18 ` Saravana Kannan
2021-01-19 9:05 ` Geert Uytterhoeven
2021-01-19 18:08 ` Saravana Kannan
2021-01-19 21:50 ` Saravana Kannan
2021-01-20 9:40 ` Geert Uytterhoeven
2021-01-20 14:26 ` Geert Uytterhoeven
2021-01-20 17:22 ` Saravana Kannan
2021-01-21 16:04 ` Geert Uytterhoeven
2021-01-25 23:30 ` Saravana Kannan
2021-01-26 8:25 ` Geert Uytterhoeven
2021-01-20 9:11 ` Geert Uytterhoeven
2021-01-21 8:22 ` [TEST PATCH v1] driver: core: Make fw_devlink=on more forgiving Saravana Kannan
2021-01-21 8:27 ` Saravana Kannan
2021-01-21 10:37 ` Geert Uytterhoeven [this message]
2021-01-22 1:07 ` Saravana Kannan
2021-01-21 10:33 ` Marek Szyprowski
2021-01-25 17:05 ` [PATCH v1 5/5] driver core: Set fw_devlink=on by default Tudor.Ambarus
2021-01-25 18:16 ` Saravana Kannan
2021-01-28 10:59 ` Tudor.Ambarus
2021-01-28 17:04 ` Saravana Kannan
2021-02-10 5:54 ` Guenter Roeck
2021-02-10 8:20 ` Saravana Kannan
2021-02-10 15:10 ` Guenter Roeck
2021-02-10 20:52 ` Saravana Kannan
2021-02-10 21:21 ` Guenter Roeck
2021-02-17 2:39 ` Saravana Kannan
2021-02-17 3:05 ` Guenter Roeck
2021-02-17 3:13 ` Saravana Kannan
2020-12-18 21:11 ` [PATCH v1 0/5] Enable " Saravana Kannan
2020-12-21 8:18 ` Jisheng Zhang
[not found] ` <CAHp75VfqL1QuvjCZ7p23e_2qhY3DUgVNaS--Uk1mEoEHsD8GBA@mail.gmail.com>
2021-01-14 16:49 ` Saravana Kannan
2020-12-21 9:48 ` Rafael J. Wysocki
2021-01-07 20:05 ` Greg Kroah-Hartman
2021-01-07 21:53 ` Saravana Kannan
2021-01-13 11:11 ` Marc Zyngier
2021-01-13 15:27 ` Jon Hunter
2021-01-13 21:29 ` Saravana Kannan
2021-01-14 11:34 ` Jon Hunter
2021-01-14 16:40 ` Saravana Kannan
2021-01-14 16:47 ` Jon Hunter
2021-01-14 16:52 ` Saravana Kannan
2021-01-14 18:55 ` Jon Hunter
2021-01-14 21:50 ` Saravana Kannan
2021-01-15 16:12 ` Jon Hunter
2021-01-15 17:44 ` Saravana Kannan
2021-01-13 20:56 ` Saravana Kannan
2021-01-13 11:30 ` Jon Hunter
2021-01-13 21:26 ` Saravana Kannan
2021-01-14 16:11 ` Jon Hunter
2021-01-14 16:47 ` Saravana Kannan
2021-01-14 16:56 ` Jon Hunter
2021-01-28 15:03 ` Jon Hunter
2021-01-28 17:27 ` Saravana Kannan
2021-02-11 0:02 ` Saravana Kannan
2021-02-11 15:03 ` Rafael J. Wysocki
2021-02-11 17:14 ` Saravana Kannan
2021-02-11 17:48 ` Rafael J. Wysocki
2021-02-12 3:04 ` Saravana Kannan
2021-01-13 11:44 ` Nicolas Saenz Julienne
2021-01-13 11:48 ` Marc Zyngier
2021-01-13 21:27 ` Saravana Kannan
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=CAMuHMdVs6UM-jvQh8kgp9x9n_rBt36ToOik44oVwKD_kkqkCVg@mail.gmail.com \
--to=geert@linux-m68k.org \
--cc=gregkh@linuxfoundation.org \
--cc=kernel-team@android.com \
--cc=linux-kernel@vger.kernel.org \
--cc=m.szyprowski@samsung.com \
--cc=maz@kernel.org \
--cc=rafael@kernel.org \
--cc=saravanak@google.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;
as well as URLs for NNTP newsgroup(s).