On Wed, Oct 14, 2015 at 10:50:40PM +0800, Xiao Guangrong wrote: > On 10/14/2015 05:40 PM, Stefan Hajnoczi wrote: > >On Sun, Oct 11, 2015 at 11:52:59AM +0800, Xiao Guangrong wrote: > >> static void dsm_write(void *opaque, hwaddr addr, > >> uint64_t val, unsigned size) > >> { > >>+ NVDIMMState *state = opaque; > >>+ MemoryRegion *dsm_ram_mr; > >>+ dsm_in *in; > >>+ dsm_out *out; > >>+ uint32_t revision, function, handle; > >>+ > >> if (val != NOTIFY_VALUE) { > >> fprintf(stderr, "BUG: unexepected notify value 0x%" PRIx64, val); > >> } > >>+ > >>+ dsm_ram_mr = memory_region_find(&state->mr, state->page_size, > >>+ state->page_size).mr; > >>+ memory_region_unref(dsm_ram_mr); > >>+ in = memory_region_get_ram_ptr(dsm_ram_mr); > > > >This looks suspicious. Shouldn't the memory_region_unref(dsm_ram_mr) > >happen after we're done using it? > > This region is keep-alive during QEMU's running, it is okay. The same > style is applied to other codes, for example: line 208 in > hw/s390x/sclp.c. In sclp.c (assign_storage()), the memory region is never used after memory_region_unref() is called. In unassign_storage(), sclp.c owns an additional reference, grabbed by assign_storage(). -- Eduardo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html