* [PATCH] eal/linux: register mp hotplug callback after memory init
@ 2023-05-31 6:55 Zhihong Wang
2023-06-01 12:26 ` Burakov, Anatoly
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Zhihong Wang @ 2023-05-31 6:55 UTC (permalink / raw)
To: dev, anatoly.burakov, qi.z.zhang; +Cc: Zhihong Wang
Secondary would crash if it tries to handle mp requests before memory
init, since globals such as eth_dev_shared_data_lock are not accessible
to it at this moment.
---
lib/eal/linux/eal.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index ae323cd492..a74d564597 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -1058,12 +1058,6 @@ rte_eal_init(int argc, char **argv)
}
}
- /* register multi-process action callbacks for hotplug */
- if (eal_mp_dev_hotplug_init() < 0) {
- rte_eal_init_alert("failed to register mp callback for hotplug");
- return -1;
- }
-
if (rte_bus_scan()) {
rte_eal_init_alert("Cannot scan the buses for devices");
rte_errno = ENODEV;
@@ -1221,6 +1215,12 @@ rte_eal_init(int argc, char **argv)
return -1;
}
+ /* register multi-process action callbacks for hotplug after memory init */
+ if (eal_mp_dev_hotplug_init() < 0) {
+ rte_eal_init_alert("failed to register mp callback for hotplug");
+ return -1;
+ }
+
if (rte_eal_tailqs_init() < 0) {
rte_eal_init_alert("Cannot init tail queues for objects");
rte_errno = EFAULT;
--
2.11.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] eal/linux: register mp hotplug callback after memory init
2023-05-31 6:55 [PATCH] eal/linux: register mp hotplug callback after memory init Zhihong Wang
@ 2023-06-01 12:26 ` Burakov, Anatoly
2023-06-02 3:33 ` [External] " 王志宏
2023-06-02 13:10 ` Burakov, Anatoly
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Burakov, Anatoly @ 2023-06-01 12:26 UTC (permalink / raw)
To: Zhihong Wang, dev, qi.z.zhang
On 5/31/2023 7:55 AM, Zhihong Wang wrote:
> Secondary would crash if it tries to handle mp requests before memory
> init, since globals such as eth_dev_shared_data_lock are not accessible
> to it at this moment.
> ---
Please correct me if I'm wrong, but if init is not completed, none of
the IPC stuff is initialized either, and any hotplug requests would not
trigger any callbacks in the first place?
--
Thanks,
Anatoly
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [External] Re: [PATCH] eal/linux: register mp hotplug callback after memory init
2023-06-01 12:26 ` Burakov, Anatoly
@ 2023-06-02 3:33 ` 王志宏
2023-06-02 13:10 ` Burakov, Anatoly
0 siblings, 1 reply; 9+ messages in thread
From: 王志宏 @ 2023-06-02 3:33 UTC (permalink / raw)
To: Burakov, Anatoly; +Cc: dev, qi.z.zhang
[-- Attachment #1: Type: text/plain, Size: 1535 bytes --]
Apologize that I have to go directly to function names to explain :)
- rte_eal_intr_init creates eal_intr_thread_main which starts
eal_intr_handle_interrupts
- rte_mp_channel_init creates mp_handle which processes messages
registered by rte_mp_action_register
- then, eal_mp_dev_hotplug_init calls rte_mp_action_register to register
handle_primary_request for EAL_DEV_MP_ACTION_REQUEST
At this point the whole messaging mechanism starts to function: When
primary attaches/detaches devices, it sends EAL_DEV_MP_ACTION_REQUEST, and
handle_primary_request invokes __handle_primary_request, which
calls local_dev_probe/remove. In the end it goes to for
example rte_eth_dev_attach_secondary.
Now, if secondary is somewhere after eal_mp_dev_hotplug_init but before
memory init done, it will crash due to memory access violation.
Thanks
Zhihong
From: "Burakov, Anatoly"<anatoly.burakov@intel.com>
Date: Thu, Jun 1, 2023, 20:26
Subject: [External] Re: [PATCH] eal/linux: register mp hotplug callback
after memory init
To: "Zhihong Wang"<wangzhihong.wzh@bytedance.com>, <dev@dpdk.org>, <
qi.z.zhang@intel.com>
On 5/31/2023 7:55 AM, Zhihong Wang wrote:
> Secondary would crash if it tries to handle mp requests before memory
> init, since globals such as eth_dev_shared_data_lock are not accessible
> to it at this moment.
> ---
Please correct me if I'm wrong, but if init is not completed, none of
the IPC stuff is initialized either, and any hotplug requests would not
trigger any callbacks in the first place?
--
Thanks,
Anatoly
[-- Attachment #2: Type: text/html, Size: 5764 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [External] Re: [PATCH] eal/linux: register mp hotplug callback after memory init
2023-06-02 3:33 ` [External] " 王志宏
@ 2023-06-02 13:10 ` Burakov, Anatoly
0 siblings, 0 replies; 9+ messages in thread
From: Burakov, Anatoly @ 2023-06-02 13:10 UTC (permalink / raw)
To: 王志宏; +Cc: dev, qi.z.zhang
On 6/2/2023 4:33 AM, 王志宏 wrote:
> Apologize that I have to go directly to function names to explain :)
> - rte_eal_intr_init creates eal_intr_thread_main which starts
> eal_intr_handle_interrupts
> - rte_mp_channel_init creates mp_handle which processes messages
> registered by rte_mp_action_register
> - then, eal_mp_dev_hotplug_init calls rte_mp_action_register to
> register handle_primary_request for EAL_DEV_MP_ACTION_REQUEST
>
> At this point the whole messaging mechanism starts to function: When
> primary attaches/detaches devices, it sends EAL_DEV_MP_ACTION_REQUEST,
> and handle_primary_request invokes __handle_primary_request, which
> calls local_dev_probe/remove. In the end it goes to for
> example rte_eth_dev_attach_secondary.
>
> Now, if secondary is somewhere after eal_mp_dev_hotplug_init but before
> memory init done, it will crash due to memory access violation.
>
> Thanks
> Zhihong
Hi,
I went into the code for IPC and you're right: we're only ignoring
requests from *unregistered* callbacks, but if they were already
registered, we handle them, which would cause your issue.
--
Thanks,
Anatoly
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] eal/linux: register mp hotplug callback after memory init
2023-05-31 6:55 [PATCH] eal/linux: register mp hotplug callback after memory init Zhihong Wang
2023-06-01 12:26 ` Burakov, Anatoly
@ 2023-06-02 13:10 ` Burakov, Anatoly
2023-06-07 20:21 ` David Marchand
2023-06-08 7:25 ` [PATCH v2] " Zhihong Wang
3 siblings, 0 replies; 9+ messages in thread
From: Burakov, Anatoly @ 2023-06-02 13:10 UTC (permalink / raw)
To: Zhihong Wang, dev, qi.z.zhang
On 5/31/2023 7:55 AM, Zhihong Wang wrote:
> Secondary would crash if it tries to handle mp requests before memory
> init, since globals such as eth_dev_shared_data_lock are not accessible
> to it at this moment.
> ---
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
--
Thanks,
Anatoly
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] eal/linux: register mp hotplug callback after memory init
2023-05-31 6:55 [PATCH] eal/linux: register mp hotplug callback after memory init Zhihong Wang
2023-06-01 12:26 ` Burakov, Anatoly
2023-06-02 13:10 ` Burakov, Anatoly
@ 2023-06-07 20:21 ` David Marchand
2023-06-08 7:29 ` [External] " 王志宏
2023-06-08 7:25 ` [PATCH v2] " Zhihong Wang
3 siblings, 1 reply; 9+ messages in thread
From: David Marchand @ 2023-06-07 20:21 UTC (permalink / raw)
To: Zhihong Wang; +Cc: dev, anatoly.burakov, qi.z.zhang
Hello Zhihong,
On Wed, May 31, 2023 at 8:55 AM Zhihong Wang
<wangzhihong.wzh@bytedance.com> wrote:
>
> Secondary would crash if it tries to handle mp requests before memory
> init, since globals such as eth_dev_shared_data_lock are not accessible
> to it at this moment.
Can you please resend this patch with a SoB ?
Thanks.
--
David Marchand
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [External] Re: [PATCH] eal/linux: register mp hotplug callback after memory init
2023-06-07 20:21 ` David Marchand
@ 2023-06-08 7:29 ` 王志宏
0 siblings, 0 replies; 9+ messages in thread
From: 王志宏 @ 2023-06-08 7:29 UTC (permalink / raw)
To: David Marchand; +Cc: dev, anatoly.burakov, qi.z.zhang
[-- Attachment #1: Type: text/plain, Size: 682 bytes --]
Done. Thanks :)
From: "David Marchand"<david.marchand@redhat.com>
Date: Thu, Jun 8, 2023, 04:21
Subject: [External] Re: [PATCH] eal/linux: register mp hotplug callback
after memory init
To: "Zhihong Wang"<wangzhihong.wzh@bytedance.com>
Cc: <dev@dpdk.org>, <anatoly.burakov@intel.com>, <qi.z.zhang@intel.com>
Hello Zhihong,
On Wed, May 31, 2023 at 8:55 AM Zhihong Wang
<wangzhihong.wzh@bytedance.com> wrote:
>
> Secondary would crash if it tries to handle mp requests before memory
> init, since globals such as eth_dev_shared_data_lock are not accessible
> to it at this moment.
Can you please resend this patch with a SoB ?
Thanks.
--
David Marchand
[-- Attachment #2: Type: text/html, Size: 4807 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2] eal/linux: register mp hotplug callback after memory init
2023-05-31 6:55 [PATCH] eal/linux: register mp hotplug callback after memory init Zhihong Wang
` (2 preceding siblings ...)
2023-06-07 20:21 ` David Marchand
@ 2023-06-08 7:25 ` Zhihong Wang
2023-06-08 8:08 ` David Marchand
3 siblings, 1 reply; 9+ messages in thread
From: Zhihong Wang @ 2023-06-08 7:25 UTC (permalink / raw)
To: dev, anatoly.burakov, david.marchand, qi.z.zhang; +Cc: Zhihong Wang
Secondary would crash if it tries to handle mp requests before memory
init, since globals such as eth_dev_shared_data_lock are not accessible
to it at this moment.
v2: add signed-off-by
Signed-off-by: Zhihong Wang <wangzhihong.wzh@bytedance.com>
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
lib/eal/linux/eal.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index ae323cd492..a74d564597 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -1058,12 +1058,6 @@ rte_eal_init(int argc, char **argv)
}
}
- /* register multi-process action callbacks for hotplug */
- if (eal_mp_dev_hotplug_init() < 0) {
- rte_eal_init_alert("failed to register mp callback for hotplug");
- return -1;
- }
-
if (rte_bus_scan()) {
rte_eal_init_alert("Cannot scan the buses for devices");
rte_errno = ENODEV;
@@ -1221,6 +1215,12 @@ rte_eal_init(int argc, char **argv)
return -1;
}
+ /* register multi-process action callbacks for hotplug after memory init */
+ if (eal_mp_dev_hotplug_init() < 0) {
+ rte_eal_init_alert("failed to register mp callback for hotplug");
+ return -1;
+ }
+
if (rte_eal_tailqs_init() < 0) {
rte_eal_init_alert("Cannot init tail queues for objects");
rte_errno = EFAULT;
--
2.11.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] eal/linux: register mp hotplug callback after memory init
2023-06-08 7:25 ` [PATCH v2] " Zhihong Wang
@ 2023-06-08 8:08 ` David Marchand
0 siblings, 0 replies; 9+ messages in thread
From: David Marchand @ 2023-06-08 8:08 UTC (permalink / raw)
To: Zhihong Wang; +Cc: dev, anatoly.burakov, qi.z.zhang
On Thu, Jun 8, 2023 at 9:26 AM Zhihong Wang
<wangzhihong.wzh@bytedance.com> wrote:
>
> Secondary would crash if it tries to handle mp requests before memory
> init, since globals such as eth_dev_shared_data_lock are not accessible
> to it at this moment.
>
> Signed-off-by: Zhihong Wang <wangzhihong.wzh@bytedance.com>
> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
Forgot to mention that we need a Fixes: tag.
I added it and applied, thanks.
--
David Marchand
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-06-08 8:08 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-31 6:55 [PATCH] eal/linux: register mp hotplug callback after memory init Zhihong Wang
2023-06-01 12:26 ` Burakov, Anatoly
2023-06-02 3:33 ` [External] " 王志宏
2023-06-02 13:10 ` Burakov, Anatoly
2023-06-02 13:10 ` Burakov, Anatoly
2023-06-07 20:21 ` David Marchand
2023-06-08 7:29 ` [External] " 王志宏
2023-06-08 7:25 ` [PATCH v2] " Zhihong Wang
2023-06-08 8:08 ` David Marchand
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).