Re: [PATCH v4 2/2] rcu: add uninit destructor for rcu

[Stefan Hajnoczi]

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