netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jiri Pirko <jiri@resnulli.us>
To: "Kubalewski, Arkadiusz" <arkadiusz.kubalewski@intel.com>
Cc: Vadim Fedorenko <vadfed@meta.com>,
	Jakub Kicinski <kuba@kernel.org>,
	Jonathan Lemon <jonathan.lemon@gmail.com>,
	Paolo Abeni <pabeni@redhat.com>,
	"Olech, Milena" <milena.olech@intel.com>,
	"Michalik, Michal" <michal.michalik@intel.com>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	Vadim Fedorenko <vadim.fedorenko@linux.dev>,
	poros <poros@redhat.com>, mschmidt <mschmidt@redhat.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"linux-clk@vger.kernel.org" <linux-clk@vger.kernel.org>
Subject: Re: [RFC PATCH v7 2/8] dpll: Add DPLL framework base functions
Date: Tue, 13 Jun 2023 15:49:45 +0200	[thread overview]
Message-ID: <ZIhz+QlNMDOeeIUD@nanopsycho> (raw)
In-Reply-To: <DM6PR11MB4657C1D51F8FEB83C219B1509B51A@DM6PR11MB4657.namprd11.prod.outlook.com>

Fri, Jun 09, 2023 at 02:53:27PM CEST, arkadiusz.kubalewski@intel.com wrote:
>>From: Jiri Pirko <jiri@resnulli.us>
>>Sent: Tuesday, May 9, 2023 3:40 PM
>>
>>Fri, Apr 28, 2023 at 02:20:03AM CEST, vadfed@meta.com wrote:
>>>From: Vadim Fedorenko <vadim.fedorenko@linux.dev>
>>>
>>
>>[...]
>>
>>>+int dpll_pre_dumpit(struct netlink_callback *cb)
>>>+{
>>>+	mutex_lock(&dpll_xa_lock);
>>
>>Did you test this?
>>
>>I'm gettting following deadlock warning:
>>
>>[  280.899789] ======================================================
>>[  280.900458] WARNING: possible circular locking dependency detected
>>[  280.901126] 6.3.0jiri+ #4 Tainted: G             L
>>[  280.901702] ------------------------------------------------------
>>[  280.902378] python3/1058 is trying to acquire lock:
>>[  280.902934] ffff88811571ae88 (nlk_cb_mutex-GENERIC){+.+.}-{3:3}, at:
>>netlink_dump+0x4a/0x400
>>[  280.903869]
>>               but task is already holding lock:
>>[  280.904559] ffffffff827d1c68 (dpll_xa_lock){+.+.}-{3:3}, at:
>>dpll_pin_pre_dumpit+0x13/0x20
>>[  280.905464]
>>               which lock already depends on the new lock.
>>
>>[  280.906414]
>>               the existing dependency chain (in reverse order) is:
>>[  280.907141]
>>               -> #1 (dpll_xa_lock){+.+.}-{3:3}:
>>[  280.907711]        __mutex_lock+0x91/0xbb0
>>[  280.908116]        dpll_pin_pre_dumpit+0x13/0x20
>>[  280.908553]        genl_start+0xc6/0x150
>>[  280.908940]        __netlink_dump_start+0x158/0x230
>>[  280.909399]        genl_family_rcv_msg_dumpit+0xf9/0x110
>>[  280.909894]        genl_rcv_msg+0x115/0x290
>>[  280.910302]        netlink_rcv_skb+0x54/0x100
>>[  280.910726]        genl_rcv+0x24/0x40
>>[  280.911106]        netlink_unicast+0x182/0x260
>>[  280.911547]        netlink_sendmsg+0x242/0x4b0
>>[  280.911984]        sock_sendmsg+0x38/0x60
>>[  280.912384]        __sys_sendto+0xeb/0x130
>>[  280.912797]        __x64_sys_sendto+0x20/0x30
>>[  280.913227]        do_syscall_64+0x3c/0x80
>>[  280.913639]        entry_SYSCALL_64_after_hwframe+0x46/0xb0
>>[  280.914156]
>>               -> #0 (nlk_cb_mutex-GENERIC){+.+.}-{3:3}:
>>[  280.914809]        __lock_acquire+0x1165/0x26b0
>>[  280.915254]        lock_acquire+0xce/0x2b0
>>[  280.915665]        __mutex_lock+0x91/0xbb0
>>[  280.916080]        netlink_dump+0x4a/0x400
>>[  280.916488]        __netlink_dump_start+0x188/0x230
>>[  280.916953]        genl_family_rcv_msg_dumpit+0xf9/0x110
>>[  280.917448]        genl_rcv_msg+0x115/0x290
>>[  280.917863]        netlink_rcv_skb+0x54/0x100
>>[  280.918301]        genl_rcv+0x24/0x40
>>[  280.918686]        netlink_unicast+0x182/0x260
>>[  280.919129]        netlink_sendmsg+0x242/0x4b0
>>[  280.919569]        sock_sendmsg+0x38/0x60
>>[  280.919969]        __sys_sendto+0xeb/0x130
>>[  280.920377]        __x64_sys_sendto+0x20/0x30
>>[  280.920808]        do_syscall_64+0x3c/0x80
>>[  280.921220]        entry_SYSCALL_64_after_hwframe+0x46/0xb0
>>[  280.921730]
>>               other info that might help us debug this:
>>
>>[  280.922513]  Possible unsafe locking scenario:
>>
>>[  280.923095]        CPU0                    CPU1
>>[  280.923541]        ----                    ----
>>[  280.923976]   lock(dpll_xa_lock);
>>[  280.924329]                                lock(nlk_cb_mutex-GENERIC);
>>[  280.924916]                                lock(dpll_xa_lock);
>>[  280.925454]   lock(nlk_cb_mutex-GENERIC);
>>[  280.925858]
>>                *** DEADLOCK ***
>>
>>[  280.926488] 2 locks held by python3/1058:
>>[  280.926891]  #0: ffffffff827e2430 (cb_lock){++++}-{3:3}, at:
>>genl_rcv+0x15/0x40
>>[  280.927585]  #1: ffffffff827d1c68 (dpll_xa_lock){+.+.}-{3:3}, at:
>>dpll_pin_pre_dumpit+0x13/0x20
>>[  280.928385]
>>               stack backtrace:
>>[  280.928853] CPU: 8 PID: 1058 Comm: python3 Tainted: G             L
>>6.3.0jiri+ #4
>>[  280.929586] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS
>>rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
>>[  280.930558] Call Trace:
>>[  280.930849]  <TASK>
>>[  280.931117]  dump_stack_lvl+0x58/0xb0
>>[  280.931500]  check_noncircular+0x11b/0x130
>>[  280.931916]  ? kernel_text_address+0x109/0x110
>>[  280.932353]  __lock_acquire+0x1165/0x26b0
>>[  280.932759]  lock_acquire+0xce/0x2b0
>>[  280.933130]  ? netlink_dump+0x4a/0x400
>>[  280.933517]  __mutex_lock+0x91/0xbb0
>>[  280.933885]  ? netlink_dump+0x4a/0x400
>>[  280.934269]  ? netlink_dump+0x4a/0x400
>>[  280.934662]  ? netlink_dump+0x4a/0x400
>>[  280.935054]  netlink_dump+0x4a/0x400
>>[  280.935426]  __netlink_dump_start+0x188/0x230
>>[  280.935857]  genl_family_rcv_msg_dumpit+0xf9/0x110
>>[  280.936321]  ? genl_family_rcv_msg_attrs_parse.constprop.0+0xe0/0xe0
>>[  280.936887]  ? dpll_nl_pin_get_doit+0x100/0x100
>>[  280.937324]  ? genl_lock_dumpit+0x50/0x50
>>[  280.937729]  genl_rcv_msg+0x115/0x290
>>[  280.938109]  ? dpll_pin_post_doit+0x20/0x20
>>[  280.938526]  ? dpll_nl_pin_get_doit+0x100/0x100
>>[  280.938966]  ? dpll_pin_pre_dumpit+0x20/0x20
>>[  280.939390]  ? genl_family_rcv_msg_doit.isra.0+0x110/0x110
>>[  280.939904]  netlink_rcv_skb+0x54/0x100
>>[  280.940296]  genl_rcv+0x24/0x40
>>[  280.940636]  netlink_unicast+0x182/0x260
>>[  280.941034]  netlink_sendmsg+0x242/0x4b0
>>[  280.941439]  sock_sendmsg+0x38/0x60
>>[  280.941804]  ? sockfd_lookup_light+0x12/0x70
>>[  280.942230]  __sys_sendto+0xeb/0x130
>>[  280.942616]  ? mntput_no_expire+0x7e/0x490
>>[  280.943038]  ? proc_nr_files+0x30/0x30
>>[  280.943425]  __x64_sys_sendto+0x20/0x30
>>[  280.943817]  do_syscall_64+0x3c/0x80
>>[  280.944194]  entry_SYSCALL_64_after_hwframe+0x46/0xb0
>>[  280.944674] RIP: 0033:0x7f252fd132b0
>>[  280.945042] Code: c0 ff ff ff ff eb b8 0f 1f 00 f3 0f 1e fa 41 89 ca 64
>>8b 04 25 18 00 00 00 85 c0 75 1d 45 31 c9 45 31 c0 b8 2c 00 00 00 0f 05
>><48> 3d 00 f0 ff ff 77 68 c3 0f 1f 80 00 00 00 00 41 54 48 83 ec 20
>>[  280.946622] RSP: 002b:00007ffdbd9335d8 EFLAGS: 00000246 ORIG_RAX:
>>000000000000002c
>>[  280.947328] RAX: ffffffffffffffda RBX: 00007ffdbd933688 RCX:
>>00007f252fd132b0
>>[  280.947962] RDX: 0000000000000014 RSI: 00007f252ede65d0 RDI:
>>0000000000000003
>>[  280.948594] RBP: 00007f252f806da0 R08: 0000000000000000 R09:
>>0000000000000000
>>[  280.949229] R10: 0000000000000000 R11: 0000000000000246 R12:
>>0000000000000000
>>[  280.949858] R13: ffffffffc4653600 R14: 0000000000000001 R15:
>>00007f252f74d147
>>[  280.950494]  </TASK>
>>
>>Problem is that in __netlink_dump_start() you take dpll_xa_lock
>>(in control->start(cb)) while holding nlk->cb_mutex, then you unlock
>>the nlk->cb_mutex and take it again in netlink_dump().
>>I hear "Chiquitita" from the distance :)
>>
>>[...]
>
>Well I tested it, but haven't seen such outcome, do you have any script
>for reproducing this behavior?

Any dump will do. For example:
sudo ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/dpll.yaml --dump device-get

This is still present in RFCv8.



>
>Thank you,
>Arkadiusz

  reply	other threads:[~2023-06-13 13:49 UTC|newest]

Thread overview: 79+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-28  0:20 [RFC PATCH v7 0/8] Create common DPLL configuration API Vadim Fedorenko
2023-04-28  0:20 ` [RFC PATCH v7 1/8] dpll: spec: Add Netlink spec in YAML Vadim Fedorenko
2023-05-04 12:02   ` Jiri Pirko
2023-05-04 21:24     ` Jakub Kicinski
2023-05-05 10:29       ` Jiri Pirko
2023-05-11  7:44         ` Kubalewski, Arkadiusz
2023-05-11  8:00           ` Jiri Pirko
2023-05-11 14:55             ` Jakub Kicinski
2023-05-11  7:40       ` Kubalewski, Arkadiusz
2023-05-11  7:59         ` Jiri Pirko
2023-05-11 20:51           ` Kubalewski, Arkadiusz
2023-05-15  9:30             ` Jiri Pirko
2023-05-16 12:05               ` Kubalewski, Arkadiusz
2023-05-16 14:33                 ` Jiri Pirko
2023-05-18 13:24                   ` Kubalewski, Arkadiusz
2023-05-18 14:02                     ` Jiri Pirko
2023-05-11 15:20         ` Jakub Kicinski
2023-05-11 20:53           ` Kubalewski, Arkadiusz
2023-05-11 23:29             ` Jakub Kicinski
2023-05-16 12:15               ` Kubalewski, Arkadiusz
2023-05-11  7:38     ` Kubalewski, Arkadiusz
2023-05-11  8:14       ` Jiri Pirko
2023-05-11 20:53         ` Kubalewski, Arkadiusz
2023-05-11 15:26       ` Jakub Kicinski
2023-05-11 20:54         ` Kubalewski, Arkadiusz
2023-04-28  0:20 ` [RFC PATCH v7 2/8] dpll: Add DPLL framework base functions Vadim Fedorenko
2023-05-02 15:38   ` Jiri Pirko
2023-06-06 18:47     ` Kubalewski, Arkadiusz
2023-05-03  8:09   ` Jiri Pirko
2023-06-06 18:50     ` Kubalewski, Arkadiusz
2023-05-04 11:18   ` Jiri Pirko
2023-05-04 20:27     ` Jakub Kicinski
2023-06-06 18:55       ` Kubalewski, Arkadiusz
2023-05-04 21:21   ` Jakub Kicinski
2023-06-09 12:54     ` Kubalewski, Arkadiusz
2023-05-09 13:40   ` Jiri Pirko
2023-06-09 12:53     ` Kubalewski, Arkadiusz
2023-06-13 13:49       ` Jiri Pirko [this message]
2023-05-22 14:45   ` Paolo Abeni
2023-06-09 12:51     ` Kubalewski, Arkadiusz
2023-04-28  0:20 ` [RFC PATCH v7 3/8] dpll: documentation on DPLL subsystem interface Vadim Fedorenko
2023-05-04 19:04   ` Jakub Kicinski
2023-05-05 13:16     ` Vadim Fedorenko
2023-05-05 15:29       ` Jakub Kicinski
2023-05-11 10:23         ` Kubalewski, Arkadiusz
2023-04-28  0:20 ` [RFC PATCH v7 4/8] ice: add admin commands to access cgu configuration Vadim Fedorenko
2023-04-28  0:20 ` [RFC PATCH v7 5/8] ice: implement dpll interface to control cgu Vadim Fedorenko
2023-05-03 12:18   ` Jiri Pirko
2023-05-15 22:07     ` Kubalewski, Arkadiusz
2023-05-16  6:26       ` Jiri Pirko
2023-05-18 16:06         ` Kubalewski, Arkadiusz
2023-05-19  6:15           ` Jiri Pirko
2023-05-25  9:01             ` Kubalewski, Arkadiusz
2023-05-19  6:47         ` Paolo Abeni
2023-05-25  9:05           ` Kubalewski, Arkadiusz
2023-05-15 17:12   ` Jiri Pirko
2023-05-16  9:22     ` Kubalewski, Arkadiusz
2023-05-16 11:46       ` Jiri Pirko
2023-05-18 16:07         ` Kubalewski, Arkadiusz
2023-05-19  6:15           ` Jiri Pirko
2023-04-28  0:20 ` [RFC PATCH v7 6/8] ptp_ocp: implement DPLL ops Vadim Fedorenko
2023-05-04  9:27   ` Jiri Pirko
2023-05-05 13:43     ` Vadim Fedorenko
2023-05-06 12:42       ` Jiri Pirko
2023-04-28  0:20 ` [RFC PATCH v7 7/8] netdev: expose DPLL pin handle for netdevice Vadim Fedorenko
2023-04-28  2:36   ` Stephen Hemminger
2023-04-28 10:00     ` Vadim Fedorenko
2023-05-04 20:31   ` Jakub Kicinski
2023-05-05 10:32     ` Jiri Pirko
2023-04-28  0:20 ` [RFC PATCH v7 8/8] mlx5: Implement SyncE support using DPLL infrastructure Vadim Fedorenko
2023-05-02  8:55 ` [RFC PATCH v7 0/8] Create common DPLL configuration API Jiri Pirko
2023-05-02 13:04 ` Jiri Pirko
2023-05-25 12:52   ` Kubalewski, Arkadiusz
2023-05-11  7:52 ` Jiri Pirko
2023-05-25 13:01   ` Kubalewski, Arkadiusz
2023-05-17 10:18 ` Jiri Pirko
2023-05-26 10:14   ` Kubalewski, Arkadiusz
2023-05-26 10:39     ` Jiri Pirko
2023-06-05 10:07       ` Kubalewski, Arkadiusz

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=ZIhz+QlNMDOeeIUD@nanopsycho \
    --to=jiri@resnulli.us \
    --cc=arkadiusz.kubalewski@intel.com \
    --cc=jonathan.lemon@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=michal.michalik@intel.com \
    --cc=milena.olech@intel.com \
    --cc=mschmidt@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=poros@redhat.com \
    --cc=vadfed@meta.com \
    --cc=vadim.fedorenko@linux.dev \
    /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).