[RFC] plugins: add [pre|post]fork helpers to linux-user [!TESTED]

Commit Message

Alex Bennée Oct. 4, 2022, 11:33 a.m. UTC

Special care needs to be taken in ensuring locks are in a consistent
state across fork events. Add helpers so the plugin system can ensure
that.

Signed-off-by: Alex Bennée <[e-mail]>
Fixes: https://gitlab.com/qemu-project/qemu/-/issues/358
---
 include/qemu/plugin.h | 24 ++++++++++++++++++++++++
 linux-user/main.c     |  2 ++
 plugins/core.c        | 20 ++++++++++++++++++++
 3 files changed, 46 insertions(+)

Comments

Daniel P. Berrangé Oct. 4, 2022, 11:39 a.m. UTC | #1

On Tue, Oct 04, 2022 at 12:33:30PM +0100, Alex Bennée wrote:
> Special care needs to be taken in ensuring locks are in a consistent
> state across fork events. Add helpers so the plugin system can ensure
> that.
> 
> Signed-off-by: Alex Bennée <[e-mail]>
> Fixes: https://gitlab.com/qemu-project/qemu/-/issues/358
> ---
>  include/qemu/plugin.h | 24 ++++++++++++++++++++++++
>  linux-user/main.c     |  2 ++
>  plugins/core.c        | 20 ++++++++++++++++++++
>  3 files changed, 46 insertions(+)

Reviewed-by: Daniel P. Berrangé <[e-mail]>
Tested-by: Daniel P. Berrangé <[e-mail]>

I confirm it fixes the plugin hangs from #358.

Much much much less frequently, I can still hit the g_slice hangs
described in

 https://gitlab.com/qemu-project/qemu/-/issues/285


With regards,
Daniel

Patch

diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
index 145f8a221a..810c6b4588 100644
--- a/include/qemu/plugin.h
+++ b/include/qemu/plugin.h
@@ -224,6 +224,23 @@  void qemu_plugin_disable_mem_helpers(CPUState *cpu);
  */
 void qemu_plugin_user_exit(void);
 
+/**
+ * qemu_plugin_user_prefork_lock(): take plugin lock before forking
+ *
+ * This is a user-mode only helper to take the internal plugin lock
+ * before a fork event. This is ensure a consistent lock state
+ */
+void qemu_plugin_user_prefork_lock(void);
+
+/**
+ * qemu_plugin_user_postfork(): reset the plugin lock
+ * @is_child: is this thread the child
+ *
+ * This user-mode only helper resets the lock state after a fork so we
+ * can continue using the plugin interface.
+ */
+void qemu_plugin_user_postfork(bool is_child);
+
 #else /* !CONFIG_PLUGIN */
 
 static inline void qemu_plugin_add_opts(void)
@@ -287,6 +304,13 @@  static inline void qemu_plugin_disable_mem_helpers(CPUState *cpu)
 
 static inline void qemu_plugin_user_exit(void)
 { }
+
+static inline void qemu_plugin_prefork_lock(void)
+{ }
+
+static inline qemu_plugin_user_postfork(bool is_child)
+{ }
+
 #endif /* !CONFIG_PLUGIN */
 
 #endif /* QEMU_PLUGIN_H */
diff --git a/linux-user/main.c b/linux-user/main.c
index 88fccfe261..a17fed045b 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -142,10 +142,12 @@  void fork_start(void)
     start_exclusive();
     mmap_fork_start();
     cpu_list_lock();
+    qemu_plugin_user_prefork_lock();
 }
 
 void fork_end(int child)
 {
+    qemu_plugin_user_postfork(child);
     mmap_fork_end(child);
     if (child) {
         CPUState *cpu, *next_cpu;
diff --git a/plugins/core.c b/plugins/core.c
index c3ae284994..ccb770a485 100644
--- a/plugins/core.c
+++ b/plugins/core.c
@@ -526,6 +526,26 @@  void qemu_plugin_user_exit(void)
     qemu_plugin_atexit_cb();
 }
 
+/*
+ * Helpers for *-user to ensure locks are sane across fork() events.
+ */
+
+void qemu_plugin_user_prefork_lock(void)
+{
+    qemu_rec_mutex_lock(&plugin.lock);
+}
+
+void qemu_plugin_user_postfork(bool is_child)
+{
+    if (is_child) {
+        /* should we just reset via plugin_init? */
+        qemu_rec_mutex_init(&plugin.lock);
+    } else {
+        qemu_rec_mutex_unlock(&plugin.lock);
+    }
+}
+
+
 /*
  * Call this function after longjmp'ing to the main loop. It's possible that the
  * last instruction of a TB might have used helpers, and therefore the