Re: [PATCH v8 2/6] memory: make global_dirty_tracking a bitmask

[Hyman]

在 2021/6/18 23:46, Peter Xu 写道:
> On Fri, Jun 18, 2021 at 11:32:03PM +0800, huangy81@chinatelecom.cn wrote:
> > diff --git a/include/exec/memory.h b/include/exec/memory.h
> > index b114f54..dd2404f 100644
> > --- a/include/exec/memory.h
> > +++ b/include/exec/memory.h
> > @@ -55,7 +55,17 @@ static inline void fuzz_dma_read_cb(size_t addr,
> >   }
> >   #endif
> >   
> > -extern bool global_dirty_log;
> > +/* Possible bits for global_dirty_log */
>
> s/log/tracking/
>
> [...]
>
> > -static void memory_global_dirty_log_do_stop(void)
> > +static void memory_global_dirty_log_do_stop(unsigned int flags)
> >   {
> > -    global_dirty_log = false;
> > +    assert(flags && !(flags & (~GLOBAL_DIRTY_MASK)));
> > +    assert((global_dirty_tracking & flags) == flags);
> > +    global_dirty_tracking &= ~flags;
> > +
> > +    trace_global_dirty_changed(global_dirty_tracking);
> >   
> >       /* Refresh DIRTY_MEMORY_MIGRATION bit.  */
> >       memory_region_transaction_begin();
> > @@ -2691,8 +2699,9 @@ static void memory_global_dirty_log_do_stop(void)
> >   static void memory_vm_change_state_handler(void *opaque, bool running,
> >                                              RunState state)
> >   {
> > +    unsigned int *flags = (unsigned int *)opaque;
>
> [1]
>
> >       if (running) {
> > -        memory_global_dirty_log_do_stop();
> > +        memory_global_dirty_log_do_stop(*flags);
> >   
> >           if (vmstate_change) {
> >               qemu_del_vm_change_state_handler(vmstate_change);
> > @@ -2701,18 +2710,19 @@ static void memory_vm_change_state_handler(void 
> > *opaque, bool running,
> >       }
> >   }
> >   
> > -void memory_global_dirty_log_stop(void)
> > +void memory_global_dirty_log_stop(unsigned int flags)
> >   {
> >       if (!runstate_is_running()) {
> >           if (vmstate_change) {
> >               return;
> >           }
> >           vmstate_change = qemu_add_vm_change_state_handler(
> > -                                memory_vm_change_state_handler, NULL);
> > +                                memory_vm_change_state_handler,
> > +                                (void *)&flags);
>
> If to drop malloc/free, we need to cast it with (void *)flags.  &flags is the
> address of the local var, which will lost its meaning after the function
> returns..
get it, the callback may almost execute after 
memory_global_dirty_log_stop returns, when it excutes, it cannot get the 
right value of flags. my fault!
> Then at [1] it should be "unsigned int flags = (unsigned int)opaque;".