[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 2/2] rcu: add uninit destructor for rcu
From: |
Stefan Hajnoczi |
Subject: |
Re: [PATCH v4 2/2] rcu: add uninit destructor for rcu |
Date: |
Wed, 9 Sep 2020 09:41:07 +0100 |
On Tue, Sep 08, 2020 at 11:10:52PM +0800, Yonggang Luo wrote:
> This is necessary if the pending rcu calls are closing and removing
> temp files. This also provide a function
> void rcu_wait_finished(void);
> to fixes test-logging.c test failure on msys2/mingw.
> On windows if the file doesn't closed, you can not remove it.
>
> Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> ---
> include/qemu/rcu.h | 5 +++++
> tests/test-logging.c | 2 ++
> util/rcu.c | 37 ++++++++++++++++++++++++++++++++++++-
> 3 files changed, 43 insertions(+), 1 deletion(-)
Can the new drain_call_rcu() function be used? Maxim recently posted the
following patch:
https://patchew.org/QEMU/20200831150124.206267-1-mlevitsk@redhat.com/20200831150124.206267-3-mlevitsk@redhat.com/
Whether drain_call_rcu() or rcu_wait_finished() is used, please include
a comment in the code that documents why the wait is necessary. For
example, "qemu_log_close() uses RCU for its FILE pointer but Windows
cannot remove open files, so we need to wait for RCU here".
Another option is to wait for RCU inside qemu_log_close() so that
callers don't need to worry about this implementation detail:
#ifdef _WIN32
/* Windows cannot remove open files so we need to wait for RCU here */
drain_call_rcu();
#endif
> diff --git a/include/qemu/rcu.h b/include/qemu/rcu.h
> index 570aa603eb..dd0a92c1d0 100644
> --- a/include/qemu/rcu.h
> +++ b/include/qemu/rcu.h
> @@ -124,6 +124,11 @@ extern void rcu_unregister_thread(void);
> extern void rcu_enable_atfork(void);
> extern void rcu_disable_atfork(void);
>
> +/*
> + * Wait all rcu call executed and exit the rcu thread.
> + */
> +extern void rcu_wait_finished(void);
> +
> struct rcu_head;
> typedef void RCUCBFunc(struct rcu_head *head);
>
> diff --git a/tests/test-logging.c b/tests/test-logging.c
> index 957f6c08cd..7a5b59f4a5 100644
> --- a/tests/test-logging.c
> +++ b/tests/test-logging.c
> @@ -210,6 +210,8 @@ int main(int argc, char **argv)
> tmp_path, test_logfile_lock);
>
> rc = g_test_run();
> + qemu_log_close();
> + rcu_wait_finished();
>
> rmdir_full(tmp_path);
> g_free(tmp_path);
> diff --git a/util/rcu.c b/util/rcu.c
> index 60a37f72c3..43367988b9 100644
> --- a/util/rcu.c
> +++ b/util/rcu.c
> @@ -308,10 +308,20 @@ void rcu_unregister_thread(void)
> qemu_mutex_unlock(&rcu_registry_lock);
> }
>
> +typedef struct QemuRcuMessage {
> + struct rcu_head rcu;
> + void *message;
> +} QemuRcuMessage;
> +
> +static int rcu_thread_exit_called = 0;
> +static int rcu_thread_exited = 0;
> +static QemuRcuMessage rcu_thread_message;
> +
> static void rcu_init_complete(void)
> {
> QemuThread thread;
> -
> + atomic_mb_set(&rcu_thread_exit_called, 0);
> + atomic_mb_set(&rcu_thread_exited, 0);
> qemu_mutex_init(&rcu_registry_lock);
> qemu_mutex_init(&rcu_sync_lock);
> qemu_event_init(&rcu_gp_event, true);
> @@ -327,6 +337,26 @@ static void rcu_init_complete(void)
> rcu_register_thread();
> }
>
> +static void rcu_thread_exit(QemuRcuMessage *param)
> +{
> + atomic_mb_set((int*)param->message, 1);
> + qemu_thread_exit(NULL);
> +}
> +
> +void rcu_wait_finished(void)
> +{
> + if (atomic_xchg(&rcu_thread_exit_called, 1) == 0)
> + {
> + rcu_thread_message.message = &rcu_thread_exited;
> + call_rcu(&rcu_thread_message, rcu_thread_exit, rcu);
> + }
> +
> + while (atomic_mb_read(&rcu_thread_exited) == 0)
> + {
> + g_usleep(10000);
> + }
> +}
> +
> static int atfork_depth = 1;
>
> void rcu_enable_atfork(void)
> @@ -379,3 +409,8 @@ static void __attribute__((__constructor__))
> rcu_init(void)
> #endif
> rcu_init_complete();
> }
> +
> +static void __attribute__((__destructor__)) rcu_uninit(void)
> +{
> + rcu_wait_finished();
> +}
> --
> 2.28.0.windows.1
>
signature.asc
Description: PGP signature