[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 17:14:44 +0100 |
On Wed, Sep 09, 2020 at 05:05:16PM +0800, 罗勇刚(Yonggang Luo) wrote:
> On Wed, Sep 9, 2020 at 4:41 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> > 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
> >
> How about not gurad with #ifdef _WIN32?
> So we don't got silent differencies between posix and win32?
> and qemu_log_close only called in function cpu_abort()
> if (qemu_log_separate()) {
> FILE *logfile = qemu_log_lock();
> qemu_log("qemu: fatal: ");
> qemu_log_vprintf(fmt, ap2);
> qemu_log("\n");
> log_cpu_state(cpu, CPU_DUMP_FPU | CPU_DUMP_CCOP);
> qemu_log_flush();
> qemu_log_unlock(logfile);
> qemu_log_close();
> }
>
> So that on't affect the performance
Yes, that sounds good.
Stefan
signature.asc
Description: PGP signature